Skip to content

LineMaterial: Fix world-unit extrusion with orthographic cameras. - #34540

Open
diarmidmackenzie wants to merge 9 commits into
mrdoob:devfrom
diarmidmackenzie:fix-line-material-orthographic-world-units
Open

LineMaterial: Fix world-unit extrusion with orthographic cameras.#34540
diarmidmackenzie wants to merge 9 commits into
mrdoob:devfrom
diarmidmackenzie:fix-line-material-orthographic-world-units

Conversation

@diarmidmackenzie

Copy link
Copy Markdown
Contributor

World-unit lines rendered with an orthographic camera become narrower away from the image centre and thin strokes can disappear. The vertex shader builds its extrusion basis from the camera-space segment midpoint, which assumes perspective viewing rays.

Use camera-space -z for the orthographic extrusion basis, selected using the existing projection-matrix classification. The perspective expression is preserved, and the same material can switch between camera types.

This moves the extrusion fix described in diarmidmackenzie/aframe-components#56 into LineMaterial itself.

Adds a QUnit rendering regression covering centred and off-axis horizontal, vertical and diagonal segments, an asymmetric orthographic frustum, and camera switching. The fixture uses an uninterrupted dash to isolate vertex extrusion from the solid-line fragment endcap calculation.

Validation:

  • Headless Chrome: all 8 rendering assertions pass; substituting the original vertex expression causes 5 assertions to fail. Off-axis horizontal/vertical coverage changes from 0 to the expected 160 pixels; diagonal coverage changes from 80 to 240 pixels.
  • npm run lint passes.
  • ESLint on all changed JavaScript files and git diff --check pass.

The rendering test was run through a temporary Puppeteer harness loading UnitTestsAddons.html?filter=LineMaterial; the standard runner splits its testPage argument at every = and drops the filter value. The full unit suite was not run.

@Mugen87 Mugen87 added this to the r187 milestone Sep 11, 2026
@github-actions

github-actions Bot commented Sep 11, 2026

Copy link
Copy Markdown

📦 Bundle size

Full ESM build, minified and gzipped.

Before After Diff
Core 389.51
102.17
389.51
102.17
+0 B
+0 B
WebGL 379.11
88.92
379.11
88.92
+0 B
+0 B
WebGPU 711.65
195.81
711.9
195.87
+245 B
+66 B
WebGPU Nodes 709.64
195.49
709.89
195.56
+245 B
+66 B

🌳 Bundle size after tree-shaking

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

Before After Diff
WebGL 523.73
126.08
523.73
126.08
+0 B
+0 B
WebGPU 765.6
205.37
765.6
205.37
+0 B
+0 B
WebGPU Nodes 711.69
192.04
711.69
192.04
+0 B
+0 B

@diarmidmackenzie
diarmidmackenzie marked this pull request as draft September 11, 2026 10:42
@Mugen87

Mugen87 commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator

I've also patched the TSL version of the line material.

The unit tests is problematic because of the intermediate renderer creation. I've removed it for now since the test failed and throwed context creation errors.

@diarmidmackenzie
diarmidmackenzie marked this pull request as ready for review September 11, 2026 10:47
@diarmidmackenzie

Copy link
Copy Markdown
Contributor Author

Thanks for looking at this. Are you happy to merge without tests, or do we need to come up with another plan for testing?

diarmidmackenzie added a commit to diarmidmackenzie/aframe-components that referenced this pull request Sep 11, 2026
…variant

Replaces the hand-written override with the exact line from
mrdoob/three.js#34540 (milestone r187), and drops everything that existed
only to support a local variant of it.

Byte-identical to upstream is the point, not the line count. It makes this
visibly the same fix rather than a lookalike, and it makes removal free:
resolveVertexShader() now recognises three states -- already fixed, stock, or
neither -- so when the bundled three ships r187 it finds the fixed line
present and returns the shader untouched. Deleting this block at that point
changes no pixel and needs no coordination with a super-three bump.

Dropped along the way:

- The bespoke D4 degenerate-cross guard. It fires exactly when the segment
  runs along the view axis, which is exactly when the extruded quad has zero
  screen area -- measured, it cannot change a rendered pixel either way. All
  it bought was a defined zero-area quad instead of NaN vertex positions, at
  the cost of diverging from upstream on a case that cannot render. If that
  NaN is worth closing it belongs in the three.js PR, not in a vendored copy.
- check-shader-target.mjs and its `npm run dist` gate. Its premise was that
  the throw fired at first LineMaterial construction, so a broken bump would
  survive a build and a smoke load. Hoisting the resolution to module scope
  already fixed that -- it now throws when the bundle loads, on any page --
  so the script was a second copy of the target strings guarding a hole that
  was already closed, and it needed its own anti-drift check to stay honest.
  This departs from the plan's D2; restoring it is one file.

Re-measured after the change: orthographic 400/400/400/400/650 against
400/400/400/400/650 predicted from geometry, and all five perspective
fixtures still byte-identical. Asserted against the running material built
from this branch's bundle: the compiled shader carries the fixed line and no
longer carries the stock one.

Net: 258 insertions -> 86, over one fewer file.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013TR1kzRUyHfCWmFXYgpi15
@Mugen87

Mugen87 commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator

Sorry, but I'm afraid the PR is not ready to merge. If you add an orthographic camera to webgl_lines_fat and compare the dev and PR version, you will notice that the PR makes the line rendering worse. You can clearly see this at the start of the first red line segment:

Dev:

image

PR:

image

Open both screenshots in separate tabs and switch back and forth. You will see that your change cuts off parts of the segment at the right.

The root cause is a mismatch in your version. You have updated the vertex shader but the fragment shader still seems to work with a perspective ray.

@github-actions

github-actions Bot commented Sep 11, 2026

Copy link
Copy Markdown

🖼️ E2E screenshot tests

✅ All examples render correctly again (run).

@diarmidmackenzie

Copy link
Copy Markdown
Contributor Author

Thanks for spotting that problem. Fragment shader now also updated in the PR.

I've added a camera picker (perspective / orthographic) to webgl_lines_fat as well in case that's a helpful addition, please discard if it's not wanted.

I've also created a demo here showing original, my 1st fix, and the latest fix, for a couple of different lines, at a variable offset from an orthograhic camera.
https://tangy-ties-smash.krabbel.fun/

@diarmidmackenzie
diarmidmackenzie marked this pull request as ready for review September 11, 2026 17:39
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.

2 participants