Repository navigation
Upgrade three.js to r186, draw label backgrounds above splats - #777
Merged
Merged
Conversation
three 0.185.1 -> 0.186.0. @types/three stays at 0.185.4 (no 0.186 types published yet); tsc is clean against it. r186 fixes the reversed-depth render-list sort that r185 broke (mrdoob/three.js#33945): `WebGLRenderList.push()` now negates the projected z instead of reversing the sorted lists, so `renderOrder` works again without our pre-inverted comparators. ReversedDepthSort.ts loses the comparators and the r185 revision gate, and keeps only the camera priming: three still initializes `camera.reversedDepth` lazily in `setProgram()`, after `sort()`, so without priming a fresh camera's first frame (every get_render() capture) sorts transparents with the un-negated z. The gate on `capabilities.reversedDepthBuffer` stays, since priming without EXT_clip_control would build a reversed projection matrix for a conventional depth buffer. Checked the r186 migration notes against the codebase: `Object3D` now has its own `dispose()`, so CascadeHelper chains to it; `Source` is only a deprecated alias for `TextureSource` and we use neither; `Sky`, `SimplifyModifier`, `LightProbeGrid`, and `toTrianglesDrawMode` are unused. The WebGLGeometries dispose contract that bufferGeometrySync.ts relies on is unchanged (the r186 "delete disposed geometry's own attributes" change is WebGPU-only), and the ShaderChunks that the vendored InstancedMesh2 overrides only changed in comments. Verified: tsc, eslint, prettier, vitest (724 tests), vite build, and the label render-order e2e suite pass on r186.
The previous commit kept a slimmed ReversedDepthSort.ts that primed `camera.reversedDepth` before three's render-list sort, on the theory that an unprimed first frame would sort transparents with the wrong sign of the projected z. That theory was wrong: three keeps the flag and the projection matrix consistent on the same camera, so an unprimed camera sorts with a standard projection (z grows with distance) and no negation, and a primed one sorts with a reversed projection and r186's negation. Feeding both states through three's WebGLRenderLists gives the same far-to-near order. The priming only ever mattered for the r185-specific pre-inverted comparators, which are already gone. Delete the module, its test, and the onCreated hook. Captures of a scene with overlapping transparents, labels, splats, shadows, and an environment map are pixel-identical with and without it (device pixel ratio pinned to rule out the adaptive DPR).
The label background quad had renderOrder 9999, below splats at 10000, so a label placed inside a splat cloud kept its text but lost its white backing: the cloud painted over the quad and the glyphs sat directly on the splats. Labels are annotations, so the whole label should composite over splats. Move the background to 10001 and the glyphs to 10002. Pin the contract with an e2e test that puts a label inside a dense black splat cloud and counts bright pixels around it on both the live canvas and a get_render() capture. Against the old order the count is zero on the live canvas; with the fix both surfaces clear the threshold.
The "labels above splats" invariant was three prose comments agreeing with a bare 10000 literal in GaussianSplats.tsx. Move the tiers into one renderOrders.ts module and derive the label orders from the splat order, so changing one cannot silently reintroduce #767. In tests/e2e/test_label_render_order.py, the new background test had copied the file's screenshot, capture, compositing, and polling code with small variations, and its live and capture regions disagreed by 10 px. Factor them into shared helpers: one polling assertion parameterized by metric and threshold that stops measuring a surface once it has passed (a capture is a full round trip and an off-screen render), one screenshot decoder, one capture compositor that crops before converting, and regions centered on the label and sized from the page's own viewport. That last point also fixes a latent mismatch: connect_client opens its context at Playwright's default 1280x720, not conftest's 960x600, so the old hardcoded core region missed the label on the live canvas. Captures are now requested at the live viewport size too, so screen-space label sizing matches across surfaces. The quad metric counts only once the black cloud actually covers the region: before the splats stream in the region is page white, which must not read as the quad being drawn on top. Verified that the test passes with the new order and fails on the live canvas with zero quad pixels when the background is forced back below the splats.
The e2e refactor folded _live_dark_pixels into a lambda, but test_label_cjk.py imports it from this module, so every e2e shard failed at collection while all of its tests passed. Bring back _live_dark_pixels and _capture_dark_pixels as named helpers over the shared screenshot and capture code, and use them in the glyph assertion too.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Removes hack from #768.