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

Fix K-bits range for meshopt octahedral filter #2463

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lilleyse
Copy link
Contributor

Looking at meshopt_encodeFilterOct as a reference, the bit range should be 1 <= K <= 16, not 4 <= K <= 16.

@zeux does this change look correct?

@zeux
Copy link
Contributor

zeux commented Dec 17, 2024

While 1 is the smallest possible K from the encoding perspective, unsure if the spec should state that as the bounds - a 1-bit signed normalized integer has two values, -1 and 0, so that's not super useful for normal encoding :) My recollection for why the spec stated K=4 is the minimum was that I felt that this is the lowest value assets could be reasonably encoded in (at a great quality loss!), but maybe this is too conservative. When normals are used for specular reflections, Suzanne from glTF-Sample-Assets looks broken on K=3:

K=4
image

K=3
image

... but BrainStem model is perhaps okay for some contexts with K=3:

image

... or K=2

image

A 2-bit signed normalized integer can represent values -1, 0, 1 in [-1, 1] range. So if we want to update the spec then minimum K should probably start with 2; K=1 is not useful.

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