Merge branch '0.58_maintenance' into 0.59_maintenance

Changes.md

@@ -4,13 +4,15 @@
 Fixes
 -----
 
+- Viewer : Fixed bug that caused the Inspector to edit the wrong node when SetFilters were in use.
 - Widget : Fixed incorrect `ButtonEvent` coordinate origin for mouse signals under certain widget configurations.
 
 API
 ---
 
 - M33fVectorDataPlug : Added new plug type for specifying arrays of 3x3 matrices.
 - PlugAlgo : `extraDataFromPlug()` now supports M33fPlug and M33fVectorDataPlug.
+- FilterPlug : Added `match` method to evaluate the filter for the specified `ScenePlug`.
 
 0.59.0.0
 ========
@@ -170,6 +172,19 @@ Build
   - OpenSSL 1.1.1h
   - See https://github.com/GafferHQ/dependencies/releases/tag/2.1.1 for full details.
 
+0.58.6.x (relative to 0.58.6.0)
+========
+
+Fixes
+-----
+
+- Viewer : Fixed bug that caused the Inspector to edit the wrong node when SetFilters were in use.
+
+API
+---
+
+- FilterPlug : Added `match` method to evaluate the filter for the specified `ScenePlug`.
+
 0.58.6.0 (relative to 0.58.5.2)
 ========
 

include/GafferScene/Filter.h

@@ -74,7 +74,7 @@ class GAFFERSCENE_API Filter : public Gaffer::ComputeNode
 		/// > where `input` is a child of a ScenePlug that will later be provided to `computeMatch()`.
 		void affects( const Gaffer::Plug *input, AffectedPlugsContainer &outputs ) const override;
 
-		/// \deprecated Use FilterPlug::SceneScope instead.
+		/// \deprecated Use FilterPlug::SceneScope or FilterPlug::match instead.
 		static void setInputScene( Gaffer::Context *context, const ScenePlug *scenePlug );
 		/// \deprecated
 		static const ScenePlug *getInputScene( const Gaffer::Context *context );

include/GafferScene/FilterPlug.h

@@ -88,6 +88,10 @@ class GAFFERSCENE_API FilterPlug : public Gaffer::IntPlug
 		/// usage, see `FilteredSceneProcessor::affects()`.
 		void sceneAffects( const Gaffer::Plug *scenePlugChild, Gaffer::DependencyNode::AffectedPlugsContainer &outputs ) const;
 
+		/// Evaluates the filter for the specified scene plug. Should be used in preference to
+		/// singular calls to getValue(), as it ensures a suitable SceneScope before evaluating the filter.
+		unsigned match( const ScenePlug *scene ) const;
+
 		/// Name of a context variable used to provide the input
 		/// scene to the filter
 		static const IECore::InternedString inputSceneContextName;

python/GafferSceneTest/FilterPlugTest.py

@@ -38,6 +38,8 @@
 import GafferScene
 import GafferSceneTest
 
+import IECore
+
 class FilterPlugTest( GafferSceneTest.SceneTestCase ) :
 
 	def testAcceptsInput( self ) :
@@ -62,5 +64,35 @@ def testAcceptsInput( self ) :
 		dot.setup( Gaffer.IntPlug() )
 		self.assertFalse( filterPlug1.acceptsInput( dot["out"] ) )
 
+	def testMatch( self ) :
+
+		p = GafferScene.FilterPlug()
+
+		with self.assertRaises( Exception ) :
+			p.match( None )
+
+		c = GafferScene.Cube()
+		c["sets"].setValue( "cubeSet" )
+
+		ctx = Gaffer.Context()
+		ctx[ "scene:path" ] = IECore.InternedStringVectorData( [ "cube" ] )
+		with ctx :
+
+			f = GafferScene.SetFilter()
+			p.setInput( f["out"] )
+
+			f["setExpression"].setValue( "cubeSet" )
+			self.assertEqual( p.match( c["out"] ), IECore.PathMatcher.Result.ExactMatch )
+			f["setExpression"].setValue( "otherSet" )
+			self.assertEqual( p.match( c["out"] ), IECore.PathMatcher.Result.NoMatch )
+
+			f = GafferScene.PathFilter()
+			p.setInput( f["out"] )
+
+			f["paths"].setValue( IECore.StringVectorData( [ "/cube" ] ) )
+			self.assertEqual( p.match( c["out"] ), IECore.PathMatcher.Result.ExactMatch )
+			f["paths"].setValue( IECore.StringVectorData( [ "/other" ] ) )
+			self.assertEqual( p.match( c["out"] ), IECore.PathMatcher.Result.NoMatch )
+
 if __name__ == "__main__":
 	unittest.main()

python/GafferSceneUI/_SceneViewInspector.py

@@ -470,7 +470,7 @@ def __editFromSceneNode( self, attributeHistory ) :
 			if not node["enabled"].getValue() :
 				return None
 
-			if "filter" in node and not ( node["filter"].getValue() & IECore.PathMatcher.Result.ExactMatch ) :
+			if "filter" in node and not ( node["filter"].match( attributeHistory.scene ) & IECore.PathMatcher.Result.ExactMatch ) :
 				return None
 
 			if isinstance( node, ( GafferScene.Light, GafferScene.LightFilter ) ) :

src/GafferScene/FilterPlug.cpp

@@ -155,6 +155,12 @@ void FilterPlug::sceneAffects( const Gaffer::Plug *scenePlugChild, Gaffer::Depen
 	}
 }
 
+unsigned FilterPlug::match( const ScenePlug *scene ) const
+{
+	FilterPlug::SceneScope scope( Context::current(), scene );
+	return getValue();
+}
+
 FilterPlug::SceneScope::SceneScope( const Gaffer::Context *context, const ScenePlug *scenePlug )
 	:	EditableScope( context )
 {

src/GafferScene/SceneAlgo.cpp

@@ -415,19 +415,6 @@ SceneAlgo::History::Ptr historyWalk( const CapturedProcess *process, InternedStr
 	return result;
 }
 
-/// \todo It's error prone to have to use SceneScope like this. Consider
-/// improvements to the FilterPlug so that you're forced to pass a scene
-/// somehow. The use of the context to provide the input scene is questionable
-/// anyway, and we don't tend to cache evaluations for `Filter.out`. So perhaps
-/// Filters shouldn't even be ComputeNodes, and FilterPlug shouldn't be an
-/// IntPlug, and instead we should pass a scene directly to some sort of
-/// `FilterPlug::match( const ScenePlug *scene )` method?
-int filterResult( const FilterPlug *filter, const ScenePlug *scene )
-{
-	FilterPlug::SceneScope scope( Context::current(), scene );
-	return filter->getValue();
-}
-
 void addGenericAttributePredecessors( const SceneAlgo::History::Predecessors &source, SceneAlgo::AttributeHistory *destination )
 {
 	for( auto &h : source )
@@ -443,7 +430,7 @@ void addCopyAttributesPredecessors( const CopyAttributes *copyAttributes, const
 {
 	const ScenePlug *sourceScene = copyAttributes->inPlug();
 	if(
-		( filterResult( copyAttributes->filterPlug(), copyAttributes->inPlug() ) & PathMatcher::ExactMatch ) &&
+		( copyAttributes->filterPlug()->match( copyAttributes->inPlug() ) & PathMatcher::ExactMatch ) &&
 		StringAlgo::matchMultiple( destination->attributeName, copyAttributes->attributesPlug()->getValue() )
 	)
 	{
@@ -486,7 +473,7 @@ void addShuffleAttributesPredecessors( const ShuffleAttributes *shuffleAttribute
 	// has come from.
 
 	InternedString sourceAttributeName = destination->attributeName;
-	if( filterResult( shuffleAttributes->filterPlug(), shuffleAttributes->inPlug() ) & PathMatcher::ExactMatch )
+	if( shuffleAttributes->filterPlug()->match( shuffleAttributes->inPlug() ) & PathMatcher::ExactMatch )
 	{
 		auto inputAttributes = shuffleAttributes->inPlug()->attributesPlug()->getValue();
 		map<InternedString, InternedString> shuffledNames;
@@ -554,7 +541,7 @@ SceneProcessor *objectTweaksWalk( const SceneAlgo::History *h )
 		if( h->scene == tweaks->outPlug() )
 		{
 			Context::Scope contextScope( h->context.get() );
-			if( filterResult( tweaks->filterPlug(), tweaks->inPlug() ) & PathMatcher::ExactMatch )
+			if( tweaks->filterPlug()->match( tweaks->inPlug() ) & PathMatcher::ExactMatch )
 			{
 				return tweaks;
 			}
@@ -581,7 +568,7 @@ ShaderTweaks *shaderTweaksWalk( const SceneAlgo::AttributeHistory *h )
 			Context::Scope contextScope( h->context.get() );
 			if(
 				StringAlgo::matchMultiple( h->attributeName, tweaks->shaderPlug()->getValue() ) &&
-				( filterResult( tweaks->filterPlug(), tweaks->inPlug() ) & PathMatcher::ExactMatch )
+				( tweaks->filterPlug()->match( tweaks->inPlug() ) & PathMatcher::ExactMatch )
 			)
 			{
 				return tweaks;

src/GafferSceneModule/FilterBinding.cpp

@@ -63,6 +63,13 @@ ScenePlugPtr getInputScene( const Gaffer::Context *context )
 	return const_cast<ScenePlug *>( Filter::getInputScene( context ) );
 }
 
+int match( const FilterPlug &plug, const ScenePlug &scene )
+{
+	IECorePython::ScopedGILRelease r;
+	return plug.match( &scene );
+}
+
+
 } // namespace
 
 void GafferSceneModule::bindFilter()
@@ -95,6 +102,7 @@ void GafferSceneModule::bindFilter()
 				)
 			)
 		)
+		.def( "match", &match )
 	;
 
 	GafferBindings::DependencyNodeClass<PathFilter>();

src/GafferSceneUI/TransformTool.cpp

@@ -98,12 +98,6 @@ M44f signOnlyScaling( const M44f &m )
 	return result;
 }
 
-int filterResult( const FilterPlug *filter, const ScenePlug *scene )
-{
-	FilterPlug::SceneScope scope( Context::current(), scene );
-	return filter->getValue();
-}
-
 // Similar to `plug->source()`, but able to traverse through
 // Spreadsheet outputs to find the appropriate input row.
 V3fPlug *spreadsheetAwareSource( V3fPlug *plug, std::string &failureReason )
@@ -294,7 +288,7 @@ void TransformTool::Selection::initFromSceneNode( const GafferScene::SceneAlgo::
 	{
 		if(
 			history->scene == constraint->outPlug() &&
-			( filterResult( constraint->filterPlug(), constraint->inPlug() ) & PathMatcher::ExactMatch )
+			( constraint->filterPlug()->match( constraint->inPlug() ) & PathMatcher::ExactMatch )
 		)
 		{
 			m_aimConstraint = true;
@@ -326,7 +320,7 @@ void TransformTool::Selection::initFromSceneNode( const GafferScene::SceneAlgo::
 	{
 		if(
 			history->scene == transform->outPlug() &&
-			( filterResult( transform->filterPlug(), transform->inPlug() ) & PathMatcher::ExactMatch )
+			( transform->filterPlug()->match( transform->inPlug() ) & PathMatcher::ExactMatch )
 		)
 		{
 			transformPlug = const_cast<TransformPlug *>( transform->transformPlug() );