Skip to content

Fix impostor clip zoom - #1063

Open
Aenardhil wants to merge 3 commits into
nglviewer:masterfrom
Aenardhil:fix-impostor-clip-zoom
Open

Fix impostor clip zoom#1063
Aenardhil wants to merge 3 commits into
nglviewer:masterfrom
Aenardhil:fix-impostor-clip-zoom

Conversation

@Aenardhil

@Aenardhil Aenardhil commented Jul 23, 2026

Copy link
Copy Markdown

I've needed to show molecule in very high zoom so when I changed clipDist to 0 and zoomed, the bond was disapearing halfway to the end of the bond.

Marc Blanch Pujol and others added 2 commits July 23, 2026 18:44
Cylinder/hyperball impostors pinned box vertices near the far plane
(gl_Position.z = 0.99 / 1.0), which clipped corners still in front of
the camera on close zoom. Pin to 0.0 (requires only w > 0) instead.
examples/scripts/test/clip-dist-zoom.js loads a small structure with
licorice/ball+stick, sets clipDist to 0 and shows a HUD; scroll-zoom
onto a bond to verify it no longer vanishes. Press 'c' to toggle
clipDist between 0 and 10.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@fredludlow

Copy link
Copy Markdown
Collaborator

Thanks, I don't have a huge amount to work on NGL now but this does look sensible. Does it behave correctly in orthographic mode too as well as perspective?

@Aenardhil

Copy link
Copy Markdown
Author

Yes — it's correct in orthographic too. In ortho gl_Position.w is always 1, so the clip test is just -1 ≤ z ≤ 1 and the per-corner w never shrinks toward zero the way it does in perspective — so the vanishing the fix addresses can't occur there in the first place. Both the old 0.99 and the new 0.0 sit safely inside the range, so ortho behavior is unchanged. I added an o key to the example scene to toggle perspective/orthographic so it's easy to confirm both modes.

I've updated the PR to have an example that let you validate functionality pressing "o" key that toggles between orthographic and prespective view. And I don't see any visually problem when the camera is really close.

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