fix(testrender): Fix discontinuities in principal directions - #2154
Conversation
Signed-off-by: Alexey Smolenchuk <alexey.smolenchuk@gmail.com>
|
This makes sense. I don't think its critical to preserve the orientation of the previous method (I assume that either way you will have a few tests to update). FWIW this improved ortho basis method does still have a discontinuity around the z=0 plane, but I agree that its probably nicer than the clunky method that was in there before. |
|
Ok, I'll remove negation then, and see how many tests it will break. |
update link Signed-off-by: Alexey Smolenchuk <alexey.smolenchuk@gmail.com>
|
LGTM. Ok with you now, @fpsunflower? The "sanitizer" CI failure is unrelated, happening on all branches, and I'm looking into it separately. |
|
Should we be suspicious that no test outputs needed updates? |
fpsunflower
left a comment
fpsunflower
left a comment
There was a problem hiding this comment.
LGTM
Can't argue with the tests passing :) I don't think we have any tests that are directly dependent on the particular orientation of dPdx or dPdy.
lgritz
changed the title
Add the citation in case the pdf link url changes Signed-off-by: Larry Gritz <lg@larrygritz.com>
Add the citation in case the URL changes Signed-off-by: Larry Gritz <lg@larrygritz.com>
lgritz
left a comment
lgritz
left a comment
There was a problem hiding this comment.
LGTM
I added one line to the comments giving the paper citation.
Just giving the link seems a little brittle, if it should move.
6039cdc
into
AcademySoftwareFoundation:main
…SoftwareFoundation#2154) Switch to a more robust method of producing orthonormal tangents given a normal. Based on https://research.pixar.com/docs/2017.Others.DBCHKLV.pdf Duff, et al. "Building an Orthonormal Basis, Revisited", JCGT 6(1) 2017. --------- Signed-off-by: Alexey Smolenchuk <alexey.smolenchuk@gmail.com>
3 participants
Description
This is just a cosmetic improvement.
It's much nicer to have continuous values when working with derivatives.
E.g. here is OLD result of
Dx(P)Checklist:
and if I used AI coding assistants, I have an
Assisted-by: TOOL / MODELline in the pull request description above.
behavior.
PR, by pushing the changes to my fork and seeing that the automated CI
passed there. (Exceptions: If most tests pass and you can't figure out why
the remaining ones fail, it's ok to submit the PR and ask for help. Or if
any failures seem entirely unrelated to your change; sometimes things break
on the GitHub runners.)
fixed any problems reported by the clang-format CI test.