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

Algebraic Sphere gradient is normalized #118

Closed
ThibaultLejemble opened this issue Nov 21, 2023 · 1 comment · Fixed by #127
Closed

Algebraic Sphere gradient is normalized #118

ThibaultLejemble opened this issue Nov 21, 2023 · 1 comment · Fixed by #127
Assignees

Comments

@ThibaultLejemble
Copy link
Collaborator

I think this is an error

PONCA_MULTIARCH inline VectorType primitiveGradient () const { return m_ul.normalized(); }

https://github.com/poncateam/ponca/blob/f8e54a568fe54b9ed78eb612b9ec97101c79df80/Ponca/src/Fitting/algebraicSphere.h#L252C8-L252C8

The gradient of a primitive should be different than the normal vector and should not be normalized?

But without the normalization, the test gls_paraboloid_der is broken.
If the gradient should not be normalized, then I'll check what is the issue with this test.

@ThibaultLejemble ThibaultLejemble self-assigned this Nov 21, 2023
@nmellado
Copy link
Contributor

True, there is an issue here: primitiveGradient was initially designed as being an accessor to the local surface normal (thus the normalization). But the name tells something else, and as you said, there is no reason to have the scalar field normalized.

My suggestion is to clarify the API and separate normal and gradient. This is somehow related to the local frame considered here #79.

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 a pull request may close this issue.

2 participants