CurvesPrimitive : Add WrapMode enum#1526
Open
johnhaddon wants to merge 16 commits intoImageEngine:mainfrom
Open
CurvesPrimitive : Add WrapMode enum#1526johnhaddon wants to merge 16 commits intoImageEngine:mainfrom
johnhaddon wants to merge 16 commits intoImageEngine:mainfrom
Conversation
And make globals const.
This generalises the periodic/nonperiodic concept and adds a Pinned value to line up with USD's pinned wrap mode.
I don't know how I ever coped editing them the way they were formatted before.
We have no need of modifying it.
This is currently taking the cheap and cheerful approach of converting to NonPeriodic on construction, which does mean that the client has to be careful to use `CurvesPrimitiveEvaluator::primitive()` for retrieving primitive variables rather than the original primitive. An alternative here would be to throw if the curves are pinned, and require the client to do the conversion externally. That's currently what MeshPrimitiveEvaluator does for non-triangle meshes, so there is some precedent.
These are nice and simple, because Varying and Vertex have the same variable size. This also revealed a bug in our handling of linear curves, where the same applies.
Member
Author
|
Debug build is failing an assertion : Seems it might be reasonable to just remove the assertion? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This extends the pre-existing periodic/nonperiodic boolean to support the additional concept of a
Pinnedwrap mode as defined by UsdGeomBasisCurves. This defines implicit "phantom vertices" to ensure that the rendered/evaluated curve interpolates all the way to its endpoints without having to manually double up points. It is so much nicer to work with, and I have no idea why we haven't all been doing this all along.There are knock-on effects to a bunch of curve-consuming code, and there I've focussed most on those I know to be relevant to Gaffer. I'm sure performance could be improved in places, but I hope this is in a reasonable place to merge as a first pass, so I can get onto some more pressing priorities.
I'll open a companion PR for Gaffer shortly, so you'll be able to test this all in practice.