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

Draw pick normal using 32 bits per color channel instead of default 8 #1172

Merged
merged 2 commits into from
Oct 12, 2023

Conversation

Kurtil
Copy link
Contributor

@Kurtil Kurtil commented Oct 12, 2023

Increase the pick normal precision fix this issue : #523

Before: (section plane not exactly parallel to the clicked surface)

normal8

After: (section plane exactly parallel to the clicked surface)

normal32

Implementation:

It uses another framebuffer instead of the previously used pick frame buffer for picking normals. The new framebuffer uses a RGBA32I color renderable texture.

It uses more VRAM but once the 1 x 1 picking viewport PR will be merged, the difference won't be relevant.

Noticeable difference:

Because of the new normal encoding, the missing normal issue does not appear the same way. Before, the normalised missing normal could be closed to [zero, zero, zero] or [NaN, NaN, NaN] OS dependent, but now it is only [NaN, NaN, NaN]...

@ghost
Copy link

ghost commented Oct 12, 2023

👇 Click on the image for a new way to code review

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map legend

@xeolabs xeolabs added this to the 2.4.0 milestone Oct 12, 2023
@xeolabs xeolabs added the bug Something isn't working label Oct 12, 2023
@xeolabs xeolabs merged commit 8e7beca into xeokit:master Oct 12, 2023
2 checks passed
@xeolabs xeolabs added Precision improvement and removed bug Something isn't working labels Oct 12, 2023
@xeolabs xeolabs self-requested a review October 12, 2023 15:50
@Kurtil Kurtil deleted the core/32bitNormals branch October 13, 2023 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants