Added SceneCache Transform SOP

[andrewkaufman]

This node applies transformations from a SceneCache file directly on the points and primitives at the SOP level. In "Name" mode, it applies a different transformation per named shape, using the name as a relative path to the SceneCache file and root. In "Root" mode, it applies a single transformation from the SceneCache file to the entire input.

Note that the loop to collect the named primitive ranges is duplicated from SOP_SceneCacheSource. I've added a todo about finding a common place to put them. I wasn't sure if I preferred another sort of DetailSplitter, or a common base class for these 2 nodes (SOP_SceneCacheNode). Or just too leave it as is until we have a 3rd use case...

[johnhaddon]

Would this also work for space == Path?

[andrewkaufman]

I've added a new commit which I believe addresses the issues with updateState. It now gets called for each object in the cook until it finds one that is animated. The tests all pass still, and I confirmed in an interactive session that the behaviour is what I'd expect for both animated and non-animated scc locations.

[johnhaddon]

I don't know if it's just me, but I'm finding this very hard to follow. I think a lot of it comes down to the large number of arguments passed through getTransform() and transformByName(). The scene and file parameters seem redundant to me - is file just passed because sometimes it is needed for worldTransform(), and scene is passed because sometimes we get the transform ourselves? I see that now the path is traversed in updateState() as well, which means that to get the transform we're deferring to worldTransform() but then to update the state we're doing our own traversal anyway. It seems like we're doing even more redundant work than previously, and I think that's one of the reason's the code is hard to follow - that and the fact that the transforms are calculated in totally different code to where the animatedness is calculated.

If you have time to spend on this I would suggest not using worldTransform() at all, and always doing a scene traversal between point a) and point b) to get the transform needed, figuring out whether or not it is animated at exactly the same time and in the same function. If I'm not misunderstanding, every combo of root, path and space just gives a series of transforms (just one, or from the root down to the name, or from / down to the name) to be concatenated - it feels to me that this could be refactored taking advantage of that, leaving something much easier to understand and maintain, and which would be doing less computation too. Does that make sense or am I just being dim?

[andrewkaufman]

I've added a new commit which basically undoes the middle commit entirely, as well as reworks things to do the relativeTransform approach that you're suggesting. I wasn't sure if it's easier to review this way or if I rebase it all into one commit. Do you have a preference?

[johnhaddon]

This variable needs to be just a SceneInterface rather than a SampledSceneInterface - otherwise we'll skip reading transforms from non-sampled scenes. Not an issue right now when all our SceneInterfaces are sampled, but would become one if we did some crazy read-from-gaffer-graph thing...

[johnhaddon]

Thanks - reads much better now. I've commented on a couple of issues which would be triggered by non-sampled SceneInterfaces if we had them, and one issue which I think would be triggered by a mixture of non-animated and animated names - hopefully both pretty easy to deal with.

I was just reading by looking at the final changes, so I think a quick rebase to squash it all back into one commit would be handy...

[andrewkaufman]

I made the changes about non-sampled scenes, and rebased it all into one commit.

[johnhaddon]

Coolcool - thanks...