SceneAlgo : ObjectProcessor object history fix
#5519
Conversation
ericmehl
force-pushed
the
sceneAlgo-object-history-internal-plug
branch
from
ce6e312 to
e4410a8
Compare
src/GafferScene/SceneAlgo.cpp
Outdated
| ( plug->parent<ScenePlug>() && plug->getName() == m_scenePlugChildName ) || | ||
| ( (Gaffer::TypeId)plug->typeId() == Gaffer::TypeId::ObjectPlugTypeId && plug->getName() == g_processedObjectPlugName ) |
There was a problem hiding this comment.
Could we put this conditional in a little inline shouldCapture( const Plug * ) function, and then use !shouldCapture() in processStarted()? The logic seems subtle enough and likely enough to get out of sync that it might be worth not transposing the individual elements to the negative in processStarted().
I wonder if we should also check that plug->parent<ObjectProcessor>() in the __processedObject case?
There was a problem hiding this comment.
Definitely an improvement, fixed and squashed in to cd91b2d
Changes.md
Outdated
| @@ -22,6 +22,7 @@ Fixes | |||
| - Fixed a bug preventing anything except strings from being copied and pasted. | |||
| - Fixed likely cause of crash when resizing Spreadsheet column width (#5296). | |||
| - Reference : Fixed rare reloading error. | |||
| - SceneAlgo : Fixed computation of `ScenePlug.object` in networks with nodes derived from `ObjectProcessor`. These include : `CameraTweaks`, `ClosestPointSampler`, `CollectPrimitiveVariables`, `CopyPrimitiveVariables`, `CurveSampler`, `DeleteCurves`, `DeleteFaces`, `DeletePoints`, `MapOffset`, `MapProjection`, `MeshDistortion`, `MeshNormals`, `MeshSegments`, `MeshTangents`, `MeshToPoints`, `MeshType`, `Orientation`, `PointsType`, `PrimitiveSampler`, `PrimitiveVariables`, `ReverseWinding`, `ShufflePrimitiveVariables` and `UVSampler`. | |||
There was a problem hiding this comment.
We need to mention the issue number here, and also put "Fixes #..." in the commit message.
There was a problem hiding this comment.
Fixed and squashed into cd91b2d
ericmehl
force-pushed
the
sceneAlgo-object-history-internal-plug
branch
from
e4410a8 to
823b997
Compare
|
Great! Those two items are addressed and squashed down into cd91b2d. Ready for another hopefully quick look and squash. |
ericmehl
force-pushed
the
sceneAlgo-object-history-internal-plug
branch
from
823b997 to
cd91b2d
Compare
This fixes #5406 where the scene history was not taking into consideration the internal
processedObjectPlugused byObjectProcessor.I'm not sure about the hard-coded handling of
ObjectProcessor, perhaps some form of registry or new plug method would be better? I looked at the other nodes that are using an internal plug and I think this is the only one that is affected, so maybe a registry isn't warranted just yet?Checklist