diff --git a/python/GafferUITest/SpreadsheetUITest.py b/python/GafferUITest/SpreadsheetUITest.py index ff6878d53f4..821719145b1 100644 --- a/python/GafferUITest/SpreadsheetUITest.py +++ b/python/GafferUITest/SpreadsheetUITest.py @@ -884,5 +884,35 @@ def testRowMetadataNotPromotedRedundantly( self ) : for plug in Gaffer.Plug.RecursiveRange( promoted[1] ) : self.assertEqual( Gaffer.Metadata.registeredValues( plug, Gaffer.Metadata.RegistrationTypes.Instance ), [] ) + def testMetadataAlgoRemovesNonDefaultRowMetadata( self ) : + + spreadsheet = Gaffer.Spreadsheet() + spreadsheet["rows"].addColumn( Gaffer.StringPlug( "column1" ) ) + spreadsheet["rows"].addRows( 1 ) + + # Create Spreadsheet where non-default row has its own metadata that conflicts with the values + # that should be mirrored from the default row. It used to be possible for the user to get into + # this situation because we redundantly promoted metadata onto the non-default rows. + + Gaffer.Metadata.registerValue( spreadsheet["rows"][0]["cells"][0], "spreadsheet:columnWidth", 100 ) + Gaffer.Metadata.registerValue( spreadsheet["rows"][1]["cells"][0], "spreadsheet:columnWidth", 200 ) + Gaffer.Metadata.registerValue( spreadsheet["rows"][0]["cells"][0], "spreadsheet:columnLabel", "Label 1" ) + Gaffer.Metadata.registerValue( spreadsheet["rows"][1]["cells"][0], "spreadsheet:columnLabel", "Label 2" ) + + # Even though the non-default-row values are different to the default row ones, we still want + # `deregisterRedundantValues()` to clean them up, because having different column widths/labels + # on different rows is logically impossible. + + Gaffer.MetadataAlgo.deregisterRedundantValues( spreadsheet["rows"] ) + + for row in spreadsheet["rows"] : + self.assertEqual( Gaffer.Metadata.value( row["cells"][0], "spreadsheet:columnWidth" ), 100 ) + self.assertEqual( Gaffer.Metadata.value( row["cells"][0], "spreadsheet:columnLabel" ), "Label 1" ) + + self.assertEqual( + Gaffer.Metadata.registeredValues( spreadsheet["rows"][1]["cells"][0], Gaffer.Metadata.RegistrationTypes.Instance ), + [] + ) + if __name__ == "__main__": unittest.main() diff --git a/src/Gaffer/MetadataAlgo.cpp b/src/Gaffer/MetadataAlgo.cpp index 9e901bd8c7e..1536064970d 100644 --- a/src/Gaffer/MetadataAlgo.cpp +++ b/src/Gaffer/MetadataAlgo.cpp @@ -42,6 +42,7 @@ #include "Gaffer/Plug.h" #include "Gaffer/Reference.h" #include "Gaffer/ScriptNode.h" +#include "Gaffer/Spreadsheet.h" #include "IECore/SimpleTypedData.h" @@ -62,6 +63,8 @@ IECore::InternedString g_connectionColorKey( "connectionGadget:color" ); IECore::InternedString g_noduleColorKey( "nodule:color" ); const std::string g_annotationPrefix( "annotation:" ); const InternedString g_annotations( "annotations" ); +const InternedString g_spreadsheetColumnWidth( "spreadsheet:columnWidth" ); +const InternedString g_spreadsheetColumnLabel( "spreadsheet:columnLabel" ); void copy( const Gaffer::GraphComponent *src , Gaffer::GraphComponent *dst , IECore::InternedString key , bool overwrite ) { @@ -769,6 +772,23 @@ void deregisterRedundantValues( GraphComponent *graphComponent ) // back to an identical value. Gaffer::Metadata::deregisterValue( graphComponent, key ); } + + // Special-case for badly promoted spreadsheet metadata of old. + if( key == g_spreadsheetColumnWidth || key == g_spreadsheetColumnLabel ) + { + if( auto row = graphComponent->ancestor() ) + { + if( auto rows = row->parent() ) + { + if( row != rows->defaultRow() ) + { + // Override on non-default row doesn't agree with the value + // that should be mirrored from the default row. Remove it. + Gaffer::Metadata::deregisterValue( graphComponent, key ); + } + } + } + } } for( auto &child : GraphComponent::Range( *graphComponent ) )