CropWindowTool : Consider read-only state when finding target plug #3239
Conversation
themissingcow
force-pushed
the
cropToolReadOnly
branch
from
07ae709
to
113e77f
Compare
src/GafferSceneUI/CropWindowTool.cpp
Outdated
| return; | ||
| } | ||
|
|
||
| if( node != m_cropWindowPlug->node() ) |
There was a problem hiding this comment.
Any node in the parent hierarchy for m_cropWindowPlug could cause us to become read only, because read-onliness is inherited. So we can't early-out like this - we'd need to use !node->isAncestorOf() instead. There is an argument for just assuming nothing about how read-onliness is specified/inherited though, and doing as the TransformTool does - just risk doing a little more work once in a while.
There was a problem hiding this comment.
Ahh great, thanks. Fixed in 558040a
src/GafferSceneUI/CropWindowTool.cpp
Outdated
| @@ -459,6 +464,49 @@ void CropWindowTool::plugDirtied( const Gaffer::Plug *plug ) | |||
| } | |||
| } | |||
|
|
|||
| void CropWindowTool::nodeMetadataChanged( IECore::InternedString key, const Gaffer::Node* node ) | |||
| { | |||
| if( !m_cropWindowPlug || m_overlayDirty ) | |||
There was a problem hiding this comment.
Does the dirtiness of the overlay really mean we can early-out here? A viewport change can dirty the overlay without setting m_needCropWindowPlugSearch.
src/GafferSceneUI/CropWindowTool.cpp
Outdated
|
|
||
| void CropWindowTool::plugMetadataChanged( IECore::InternedString key, const Gaffer::Plug* plug ) | ||
| { | ||
| if( !m_cropWindowPlug || m_overlayDirty ) |
src/GafferSceneUI/CropWindowTool.cpp
Outdated
| m_overlay->setCaption( m_cropWindowPlug->relativeName( m_cropWindowPlug->ancestor<ScriptNode>() ) ); | ||
| const std::string plugName = m_cropWindowPlug->relativeName( m_cropWindowPlug->ancestor<ScriptNode>() ); | ||
|
|
||
| // If even after the second search, we could still be read-only |
johnhaddon
added
the
pr-revision
User Facing Changes : - The CropWindowTool now properly considers a plug's read-only state when looking for a crop window to edit. This covers nodes within references as well as individually locked plugs.
themissingcow
force-pushed
the
cropToolReadOnly
branch
from
113e77f
to
558040a
Compare
themissingcow
removed
the
pr-revision
Manual merge of #3239 after squashing commits and fixing a bug.
|
Closing - merged this manually after squashing commits and fixing a bug. |
Fixes #3236
User Facing Changes :
CropWindowToolnow properly considers a plug's read-only state whenlooking for a crop window to edit. This covers nodes within references
as well as individually locked plugs.