Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WebGLRenderer: Sort objects in clip space. #28571

Merged
merged 6 commits into from
Jun 6, 2024

Conversation

eiriklegernaes
Copy link
Contributor

@eiriklegernaes eiriklegernaes commented Jun 6, 2024

Original PR #28474 was merged slightly early; here's the changes that the discussion in that PR landed on.

This PR changes the space in which objects are sorted from NDC to clip space, as well as adds Vector4.setFromMatrixPosition for convenience.

@eiriklegernaes eiriklegernaes changed the title Fix sorting for objects with positive view z Sort objects in clip space Jun 6, 2024
Copy link

github-actions bot commented Jun 6, 2024

📦 Bundle size

Full ESM build, minified and gzipped.

Filesize dev Filesize PR Diff
678.5 kB (168.2 kB) 678.6 kB (168.3 kB) +129 B

🌳 Bundle size after tree-shaking

Minimal build including a renderer, camera, empty scene, and dependencies.

Filesize dev Filesize PR Diff
456.6 kB (110.3 kB) 456.7 kB (110.3 kB) +118 B

@mrdoob mrdoob added this to the r166 milestone Jun 6, 2024
@Mugen87 Mugen87 changed the title Sort objects in clip space WebGLRenderer: Sort objects in clip space. Jun 6, 2024
@eiriklegernaes
Copy link
Contributor Author

The doc-change broke a bunch of visual tests. I see a lot of Error creating WebGL context., another attempt... errors. Are these known to be flaky, or is this my fault somehow?

@Mugen87
Copy link
Collaborator

Mugen87 commented Jun 6, 2024

It's an issue of the E2E environment. We don't know why sometimes WebGL context creation fails. I'll rerun the failed test which is often the best approach to get a successful CI run.

@mrdoob mrdoob merged commit 7a62164 into mrdoob:dev Jun 6, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants