Merge branch '1.4_maintenance'

Changes.md

@@ -63,7 +63,10 @@ Breaking Changes
 Improvements
 ------------
 
-- LightEditor : Values of inherited attributes are now displayed in the Light Editor. These are presented as dimmed "fallback" values.
+- LightEditor :
+  - Values of inherited attributes are now displayed in the Light Editor. These are presented as dimmed "fallback" values. Values are inherited from an ancestor of the inspected location or from attributes created in the scene globals.
+  - Default values are now displayed as dimmed "fallback" values for attributes that don't exist in the scene.
+  - When a fallback value is displayed, the cell's tooltip includes a description of the source of the value.
 - LightEditor, RenderPassEditor : Fallback values shown in the history window are displayed with the same dimmed text colour used for fallback values in editor columns.
 - EditScope : Filtered the EditScope menu to show only nodes that are active in the relevant context.
 
@@ -72,6 +75,7 @@ Fixes
 
 - ImageReader : Fixed crash caused by invalid OpenEXR `multiView` attributes.
 - LightEditor, RenderPassEditor : Added missing icon representing use of the `CreateIfMissing` tweak mode in the history window.
+- Slider : Fixed bug where two undo steps were needed to get back to the original value when dragging.
 
 API
 ---
@@ -734,14 +738,20 @@ Build
   - Removed QtNetworkAuth library.
 - USD : Updated to version 23.11.
 
-1.3.16.x (relative to 1.3.16.6)
+1.3.16.x (relative to 1.3.16.7)
+========
+
+
+
+1.3.16.7 (relative to 1.3.16.6)
 ========
 
 Fixes
 -----
 
 - ImageReader : Fixed crash caused by invalid OpenEXR `multiView` attributes.
 - LightEditor, RenderPassEditor : Added missing icon representing use of the `CreateIfMissing` tweak mode in the history window.
+- Slider : Fixed bug where two undo steps were needed to get back to the original value when dragging.
 
 1.3.16.6 (relative to 1.3.16.5)
 ========

include/GafferSceneUI/Private/AttributeInspector.h

@@ -67,7 +67,7 @@ class GAFFERSCENEUI_API AttributeInspector : public Inspector
 		IECore::ConstObjectPtr value( const GafferScene::SceneAlgo::History *history) const override;
 		Gaffer::ValuePlugPtr source( const GafferScene::SceneAlgo::History *history, std::string &editWarning ) const override;
 		EditFunctionOrFailure editFunction( Gaffer::EditScope *scope, const GafferScene::SceneAlgo::History *history) const override;
-		IECore::ConstObjectPtr fallbackValue( const GafferScene::SceneAlgo::History *history ) const override;
+		IECore::ConstObjectPtr fallbackValue( const GafferScene::SceneAlgo::History *history, std::string &description ) const override;
 
 		bool attributeExists() const;
 

include/GafferSceneUI/Private/Inspector.h

@@ -155,6 +155,12 @@ class GAFFERSCENEUI_API Inspector : public IECore::RefCounted, public Gaffer::Si
 		/// base class?
 		virtual IECore::ConstObjectPtr value( const GafferScene::SceneAlgo::History *history ) const = 0;
 
+		/// Can be implemented by derived classes to provide a fallback value for the inspection,
+		/// used when no value is returned from `value()`. Called with `history->context` as the current
+		/// context. Optionally, `description` may be assigned a description to be shown to the user.
+		/// Typically, this description would be used to disambiguate the source of the fallback value.
+		virtual IECore::ConstObjectPtr fallbackValue( const GafferScene::SceneAlgo::History *history, std::string &description ) const;
+
 		/// Should be implemented by derived classes to return the source for
 		/// the value authored at this point in the history. Optionally,
 		/// `editWarning` may be assigned a warning that will be shown to the
@@ -177,10 +183,6 @@ class GAFFERSCENEUI_API Inspector : public IECore::RefCounted, public Gaffer::Si
 		/// > that edits the processor itself.
 		virtual EditFunctionOrFailure editFunction( Gaffer::EditScope *editScope, const GafferScene::SceneAlgo::History *history ) const;
 
-		/// Can be implemented by derived classes to provide a fallback value for the inspection,
-		/// used when no value is returned from `value()`.
-		virtual IECore::ConstObjectPtr fallbackValue( const GafferScene::SceneAlgo::History *history ) const;
-
 	protected :
 
 		Gaffer::EditScope *targetEditScope() const;
@@ -319,6 +321,9 @@ class GAFFERSCENEUI_API Inspector::Result : public IECore::RefCounted
 
 		/// The relationship between `source()` and `editScope()`.
 		SourceType sourceType() const;
+		/// Returns a user-facing description of the source of the
+		/// fallback value when `SourceType` is `Fallback`.
+		const std::string &fallbackDescription() const;
 
 		/// Editing
 		/// =======
@@ -347,6 +352,7 @@ class GAFFERSCENEUI_API Inspector::Result : public IECore::RefCounted
 		const IECore::ConstObjectPtr m_value;
 		Gaffer::ValuePlugPtr m_source;
 		SourceType m_sourceType;
+		std::string m_fallbackDescription;
 		Gaffer::EditScopePtr m_editScope;
 		bool m_editScopeInHistory;
 

include/GafferSceneUI/Private/OptionInspector.h

@@ -65,7 +65,7 @@ class GAFFERSCENEUI_API OptionInspector : public Inspector
 		IECore::ConstObjectPtr value( const GafferScene::SceneAlgo::History *history ) const override;
 		Gaffer::ValuePlugPtr source( const GafferScene::SceneAlgo::History *history, std::string &editWarning ) const override;
 		EditFunctionOrFailure editFunction( Gaffer::EditScope *scope, const GafferScene::SceneAlgo::History *history ) const override;
-		IECore::ConstObjectPtr fallbackValue( const GafferScene::SceneAlgo::History *history ) const override;
+		IECore::ConstObjectPtr fallbackValue( const GafferScene::SceneAlgo::History *history, std::string &description ) const override;
 
 	private :
 

include/GafferSceneUI/Private/ParameterInspector.h

@@ -69,7 +69,7 @@ class GAFFERSCENEUI_API ParameterInspector : public AttributeInspector
 		IECore::ConstObjectPtr value( const GafferScene::SceneAlgo::History *history ) const override;
 		Gaffer::ValuePlugPtr source( const GafferScene::SceneAlgo::History *history, std::string &editWarning ) const override;
 		EditFunctionOrFailure editFunction( Gaffer::EditScope *editScope, const GafferScene::SceneAlgo::History *history ) const override;
-		IECore::ConstObjectPtr fallbackValue( const GafferScene::SceneAlgo::History *history ) const override;
+		IECore::ConstObjectPtr fallbackValue( const GafferScene::SceneAlgo::History *history, std::string &description ) const override;
 
 		const IECoreScene::ShaderNetwork::Parameter m_parameter;
 

include/GafferSceneUI/Private/SetMembershipInspector.h

@@ -81,7 +81,7 @@ class GAFFERSCENEUI_API SetMembershipInspector : public Inspector
 		/// those are found.
 		Gaffer::ValuePlugPtr source( const GafferScene::SceneAlgo::History *history, std::string &editWarning ) const override;
 		EditFunctionOrFailure editFunction( Gaffer::EditScope *scope, const GafferScene::SceneAlgo::History *history ) const override;
-		IECore::ConstObjectPtr fallbackValue( const GafferScene::SceneAlgo::History *history ) const override;
+		IECore::ConstObjectPtr fallbackValue( const GafferScene::SceneAlgo::History *history, std::string &description ) const override;
 
 	private :
 

python/GafferSceneUITest/AttributeInspectorTest.py

@@ -122,21 +122,44 @@ def testFallbackValue( self ) :
 		group = GafferScene.Group()
 		group["in"][0].setInput( light["out"] )
 
+		# With no "gl:visualiser:scale" attribute at /group/light, the inspection returns
+		# the registered default value with `sourceType` identifying it as a fallback.
+
+		inspection = self.__inspect( group["out"], "/group/light", "gl:visualiser:scale" )
+		self.assertEqual( inspection.value().value, Gaffer.Metadata.value( "attribute:gl:visualiser:scale", "defaultValue" ) )
+		self.assertEqual( inspection.sourceType(), GafferSceneUI.Private.Inspector.Result.SourceType.Fallback )
+		self.assertEqual( inspection.fallbackDescription(), "Default value" )
+
+		globalGlAttributes = GafferScene.OpenGLAttributes()
+		globalGlAttributes["in"].setInput( group["out"] )
+		globalGlAttributes["global"].setValue( True )
+		globalGlAttributes["attributes"]["visualiserScale"]["enabled"].setValue( True )
+		globalGlAttributes["attributes"]["visualiserScale"]["value"].setValue( 4.0 )
+
+		# With no "gl:visualiser:scale" attribute at /group/light, the inspection returns
+		# the inherited global attribute value with `sourceType` identifying it as a fallback.
+
+		inspection = self.__inspect( globalGlAttributes["out"], "/group/light", "gl:visualiser:scale" )
+		self.assertEqual( inspection.value(), IECore.FloatData( 4.0 ) )
+		self.assertEqual( inspection.sourceType(), GafferSceneUI.Private.Inspector.Result.SourceType.Fallback )
+		self.assertEqual( inspection.fallbackDescription(), "Global attribute" )
+
 		groupFilter = GafferScene.PathFilter()
 		groupFilter["paths"].setValue( IECore.StringVectorData( [ "/group" ] ) )
 
 		glAttributes = GafferScene.OpenGLAttributes()
-		glAttributes["in"].setInput( group["out"] )
+		glAttributes["in"].setInput( globalGlAttributes["out"] )
 		glAttributes["filter"].setInput( groupFilter["out"] )
 		glAttributes["attributes"]["visualiserScale"]["enabled"].setValue( True )
 		glAttributes["attributes"]["visualiserScale"]["value"].setValue( 2.0 )
 
 		# With no "gl:visualiser:scale" attribute at /group/light, the inspection returns
-		# the inherited attribute value with `sourceType` identifying it as a fallback.
+		# the inherited attribute value from /group with `sourceType` identifying it as a fallback.
 
 		inspection = self.__inspect( glAttributes["out"], "/group/light", "gl:visualiser:scale" )
 		self.assertEqual( inspection.value(), IECore.FloatData( 2.0 ) )
 		self.assertEqual( inspection.sourceType(), GafferSceneUI.Private.Inspector.Result.SourceType.Fallback )
+		self.assertEqual( inspection.fallbackDescription(), "Inherited from /group" )
 
 		# With a "gl:visualiser:scale" attribute created at the inspected location, it is
 		# returned instead of the inherited fallback.
@@ -686,7 +709,7 @@ def testDisabledAttribute( self ) :
 		self.__assertExpectedResult(
 			self.__inspect( light["out"], "/light", "gl:visualiser:scale", None ),
 			source = light["visualiserAttributes"]["scale"],
-			sourceType = GafferSceneUI.Private.Inspector.Result.SourceType.Other,
+			sourceType = GafferSceneUI.Private.Inspector.Result.SourceType.Fallback,
 			editable = True,
 			edit = light["visualiserAttributes"]["scale"]
 		)
@@ -695,7 +718,7 @@ def testDisabledAttribute( self ) :
 		self.__assertExpectedResult(
 			self.__inspect( group["out"], "/group/light", "gl:visualiser:scale", None ),
 			source = light["visualiserAttributes"]["scale"],
-			sourceType = GafferSceneUI.Private.Inspector.Result.SourceType.Other,
+			sourceType = GafferSceneUI.Private.Inspector.Result.SourceType.Fallback,
 			editable = True,
 			edit = light["visualiserAttributes"]["scale"]
 		)
@@ -711,7 +734,7 @@ def testRegisteredAttribute( self ) :
 		self.__assertExpectedResult(
 			self.__inspect( editScope["out"], "/light", "gl:visualiser:scale", None ),
 			source = light["visualiserAttributes"]["scale"],
-			sourceType = GafferSceneUI.Private.Inspector.Result.SourceType.Other,
+			sourceType = GafferSceneUI.Private.Inspector.Result.SourceType.Fallback,
 			editable = True,
 			edit = light["visualiserAttributes"]["scale"]
 		)
@@ -779,7 +802,7 @@ def testLightFilter( self ) :
 		self.__assertExpectedResult(
 			self.__inspect( editScope["out"], "/lightFilter", "filteredLights" ),
 			source = lightFilter["filteredLights"],
-			sourceType = GafferSceneUI.Private.Inspector.Result.SourceType.Other,
+			sourceType = GafferSceneUI.Private.Inspector.Result.SourceType.Fallback,
 			editable = True,
 			edit = lightFilter["filteredLights"]
 		)

python/GafferSceneUITest/HistoryPathTest.py

@@ -400,7 +400,7 @@ def testAttributeFallbackValues( self ) :
 		self.assertEqual( c[0].property( "name" ), str( c[0][-1] ) )
 		self.assertEqual( c[0].property( "history:node" ), s["openGLAttributes"] )
 		self.assertEqual( c[0].property( "history:value" ), 1536 )
-		self.assertEqual( c[0].property( "history:fallbackValue" ), 1536 )
+		self.assertEqual( c[0].property( "history:fallbackValue" ), 1024 )
 		self.assertEqual( c[0].property( "history:operation" ), Gaffer.TweakPlug.Mode.Create )
 		self.assertEqual( c[0].property( "history:source" ), s["openGLAttributes"]["attributes"]["visualiserMaxTextureResolution"] )
 		self.assertEqual( c[0].property( "history:editWarning" ), "Edits to \"gl:visualiser:maxTextureResolution\" may affect other locations in the scene." )
@@ -416,7 +416,7 @@ def testAttributeFallbackValues( self ) :
 		self.assertEqual( c[0].property( "name" ), str( c[0][-1] ) )
 		self.assertEqual( c[0].property( "history:node" ), s["testLight"] )
 		self.assertEqual( c[0].property( "history:value" ), None )
-		self.assertEqual( c[0].property( "history:fallbackValue" ), None )
+		self.assertEqual( c[0].property( "history:fallbackValue" ), 512 )
 		self.assertEqual( c[0].property( "history:operation" ), Gaffer.TweakPlug.Mode.Create )
 		self.assertEqual( c[0].property( "history:source" ), s["testLight"]["visualiserAttributes"]["maxTextureResolution"] )
 		self.assertEqual( c[0].property( "history:editWarning" ), "" )

python/GafferSceneUITest/OptionInspectorTest.py

@@ -844,6 +844,7 @@ def testDefaultValueMetadata( self ) :
 		inspection = self.__inspect( editScope["out"], "test:enabled", editScope )
 		self.assertEqual( inspection.value(), IECore.BoolData( 1 ) )
 		self.assertEqual( inspection.sourceType(), GafferSceneUI.Private.Inspector.Result.SourceType.Fallback )
+		self.assertEqual( inspection.fallbackDescription(), "Default value" )
 
 		# If the option does exist, then its value is returned as normal
 
@@ -852,13 +853,15 @@ def testDefaultValueMetadata( self ) :
 		inspection = self.__inspect( editScope["out"], "test:enabled", editScope )
 		self.assertEqual( inspection.value(), IECore.BoolData( 0 ) )
 		self.assertEqual( inspection.sourceType(), GafferSceneUI.Private.Inspector.Result.SourceType.Upstream )
+		self.assertEqual( inspection.fallbackDescription(), "" )
 
 		# Disabling the option should revert to the fallback
 
 		optionPlug["enabled"].setValue( False )
 		inspection = self.__inspect( editScope["out"], "test:enabled", editScope )
 		self.assertEqual( inspection.value(), IECore.BoolData( 1 ) )
 		self.assertEqual( inspection.sourceType(), GafferSceneUI.Private.Inspector.Result.SourceType.Fallback )
+		self.assertEqual( inspection.fallbackDescription(), "Default value" )
 
 		# Updates to "defaultValue" are reflected in new inspections
 

python/GafferSceneUITest/SetMembershipInspectorTest.py

@@ -117,15 +117,33 @@ def testFallbackValue( self ) :
 
 		plane = GafferScene.Plane()
 		group = GafferScene.Group()
-		group["sets"].setValue( "planeSet" )
 		group["in"][0].setInput( plane["out"] )
 
-		inspection = self.__inspect( group["out"], "/group/plane", "planeSet" )
+		pathFilter = GafferScene.PathFilter()
+		pathFilter["paths"].setValue( IECore.StringVectorData( [ "/" ] ) )
+
+		setNode = GafferScene.Set()
+		setNode["in"].setInput( group["out"] )
+		setNode["name"].setValue( "planeSet" )
+		setNode["filter"].setInput( pathFilter["out"] )
+
+		inspection = self.__inspect( setNode["out"], "/group/plane", "planeSet" )
+		self.assertEqual( inspection.value().value, True )
+		self.assertEqual(
+			inspection.sourceType(),
+			GafferSceneUI.Private.Inspector.Result.SourceType.Fallback
+		)
+		self.assertEqual( inspection.fallbackDescription(), "Inherited from /" )
+
+		pathFilter["paths"].setValue( IECore.StringVectorData( [ "/", "/group" ] ) )
+
+		inspection = self.__inspect( setNode["out"], "/group/plane", "planeSet" )
 		self.assertEqual( inspection.value().value, True )
 		self.assertEqual(
 			inspection.sourceType(),
 			GafferSceneUI.Private.Inspector.Result.SourceType.Fallback
 		)
+		self.assertEqual( inspection.fallbackDescription(), "Inherited from /group" )
 
 	def testSourceAndEdits( self ) :
 

python/GafferUI/Slider.py

@@ -430,6 +430,11 @@ def __buttonPress( self, widget, event ) :
 	def __dragBegin( self, widget, event ) :
 
 		if event.buttons == GafferUI.ButtonEvent.Buttons.Left and self.getSelectedIndex() is not None :
+			self.__setValueInternal(
+				self.getSelectedIndex(),
+				self.__eventValue( event ),
+				self.ValueChangedReason.DragBegin
+			)
 			return IECore.NullObject.defaultNullObject()
 
 		return None
@@ -451,7 +456,11 @@ def __dragMove( self, widget, event ) :
 
 	def __dragEnd( self, widget, event ) :
 
-		self.__dragMove( widget, event )
+		self.__setValueInternal(
+			self.getSelectedIndex(),
+			self.__eventValue( event ),
+			self.ValueChangedReason.DragEnd
+		)
 
 	def __keyPress( self, widget, event ) :
 

src/GafferSceneUI/AttributeInspector.cpp

@@ -69,6 +69,9 @@ using namespace GafferSceneUI::Private;
 namespace
 {
 
+const std::string g_attributePrefix( "attribute:" );
+const InternedString g_defaultValue( "defaultValue" );
+
 // This uses the same strategy that ValuePlug uses for the hash cache,
 // using `plug->dirtyCount()` to invalidate previous cache entries when
 // a plug is dirtied.
@@ -245,9 +248,44 @@ IECore::ConstObjectPtr AttributeInspector::value( const GafferScene::SceneAlgo::
 	return nullptr;
 }
 
-IECore::ConstObjectPtr AttributeInspector::fallbackValue( const GafferScene::SceneAlgo::History *history ) const
+IECore::ConstObjectPtr AttributeInspector::fallbackValue( const GafferScene::SceneAlgo::History *history, std::string &description ) const
 {
-	return history->scene->fullAttributes( history->context->get<ScenePlug::ScenePath>( ScenePlug::scenePathContextName ) )->member( m_attribute );
+	ScenePlug::PathScope pathScope( Context::current() );
+	ScenePlug::ScenePath currentPath( history->context->get<ScenePlug::ScenePath>( ScenePlug::scenePathContextName ) );
+
+	// No need to check inheritance for immediate children of `/` as we
+	// don't allow attributes to be created at the root of the scene.
+	if( currentPath.size() > 1 )
+	{
+		// We start the inheritance search from the parent in order to return the value that
+		// would be inherited if the inspected attribute did not exist at the original location.
+		currentPath.pop_back();
+
+		while( !currentPath.empty() )
+		{
+			pathScope.setPath( &currentPath );
+			auto a = history->scene->attributesPlug()->getValue();
+			if( const auto attribute = a->member( m_attribute ) )
+			{
+				description = "Inherited from " + ScenePlug::pathToString( currentPath );
+				return attribute;
+			}
+			currentPath.pop_back();
+		}
+	}
+
+	if( const auto globalAttribute = history->scene->globals()->member<Object>( g_attributePrefix + m_attribute.string() ) )
+	{
+		description = "Global attribute";
+		return globalAttribute;
+	}
+	else if( const auto defaultValue = Gaffer::Metadata::value( g_attributePrefix + m_attribute.string(), g_defaultValue ) )
+	{
+		description = "Default value";
+		return defaultValue;
+	}
+
+	return nullptr;
 }
 
 Gaffer::ValuePlugPtr AttributeInspector::source( const GafferScene::SceneAlgo::History *history, std::string &editWarning ) const
@@ -369,7 +407,7 @@ Inspector::EditFunctionOrFailure AttributeInspector::editFunction( Gaffer::EditS
 
 void AttributeInspector::plugDirtied( Gaffer::Plug *plug )
 {
-	if( plug == m_scene->attributesPlug() )
+	if( plug == m_scene->attributesPlug() || plug == m_scene->globalsPlug() )
 	{
 		dirtiedSignal()( this );
 	}