Skip to content

Commit

Permalink
SetEditor : Prefer ScriptNodeAlgo to ContextAlgo (phase 2)
Browse files Browse the repository at this point in the history 
This moves the handling of selection out of SetPath and into the SetEditorEmptySetFilter and SetSelectionColumn, switching to ScriptNodeAlgo at the same time. It felt a bit more natural to have selection be managed in the "presentation layer" rather than in the data (Path) layer itself. But it would also be possible to have kept the original organisation. Can switch back if preferred.
  • Loading branch information
@johnhaddon
johnhaddon committed Sep 10, 2024
1 parent f05e329 commit 1c0dfe1
Show file tree
Hide file tree
Showing 3 changed files with 99 additions and 60 deletions.
9 changes: 4 additions & 5 deletions python/GafferSceneUI/SetEditor.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@
import GafferUI
import GafferSceneUI

from . import ContextAlgo
from . import _GafferSceneUI

class SetEditor( GafferSceneUI.SceneEditor ) :
Expand All @@ -56,11 +55,11 @@ def __init__( self, scriptNode, **kw ) :
GafferSceneUI.SceneEditor.__init__( self, mainColumn, scriptNode, **kw )

searchFilter = _GafferSceneUI._SetEditor.SearchFilter()
emptySetFilter = _GafferSceneUI._SetEditor.EmptySetFilter()
emptySetFilter = _GafferSceneUI._SetEditor.EmptySetFilter( scriptNode )
emptySetFilter.userData()["UI"] = { "label" : "Hide Empty Members", "toolTip" : "Hide sets with no members" }
emptySetFilter.setEnabled( False )

emptySelectionFilter = _GafferSceneUI._SetEditor.EmptySetFilter( propertyName = "setPath:selectedMemberCount" )
emptySelectionFilter = _GafferSceneUI._SetEditor.EmptySetFilter( scriptNode, useSelection = True )
emptySelectionFilter.userData()["UI"] = { "label" : "Hide Empty Selection", "toolTip" : "Hide sets with no selected members or descendants" }
emptySelectionFilter.setEnabled( False )

Expand All @@ -75,7 +74,7 @@ def __init__( self, scriptNode, **kw ) :
GafferUI.BasicPathFilterWidget( emptySelectionFilter )

self.__setMembersColumn = _GafferSceneUI._SetEditor.SetMembersColumn()
self.__selectedSetMembersColumn = _GafferSceneUI._SetEditor.SetSelectionColumn()
self.__selectedSetMembersColumn = _GafferSceneUI._SetEditor.SetSelectionColumn( scriptNode )
self.__includedSetMembersColumn = _GafferSceneUI._SetEditor.VisibleSetInclusionsColumn( scriptNode )
self.__excludedSetMembersColumn = _GafferSceneUI._SetEditor.VisibleSetExclusionsColumn( scriptNode )
self.__pathListing = GafferUI.PathListingWidget(
Expand Down Expand Up @@ -104,7 +103,7 @@ def __repr__( self ) :

def _updateFromContext( self, modifiedItems ) :

if any( not i.startswith( "ui:" ) for i in modifiedItems ) or any( ContextAlgo.affectsSelectedPaths( i ) for i in modifiedItems ) :
if any( not i.startswith( "ui:" ) for i in modifiedItems ) :
self.__updatePathListingPath()

@GafferUI.LazyMethod( deferUntilPlaybackStops = True )
Expand Down
33 changes: 19 additions & 14 deletions python/GafferSceneUITest/SetEditorTest.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ def testSetPathMemberCount( self ) :
path.setFromString( parent )
self.assertEqual( path.property( "setPath:memberCount" ), count )

def testSetPathSelectedMemberCount( self ) :
def testSetSelectionColumn( self ) :

script = Gaffer.ScriptNode()

Expand All @@ -203,6 +203,8 @@ def testSetPathSelectedMemberCount( self ) :
self.assertTrue( path.isValid() )
self.assertFalse( path.isLeaf() )

column = _GafferSceneUI._SetEditor.SetSelectionColumn( script )

for parent, selection, count in [
( "/", [], None ),
( "/", [ "/plane" ], None ),
Expand Down Expand Up @@ -232,9 +234,9 @@ def testSetPathSelectedMemberCount( self ) :

path.setFromString( parent )
GafferSceneUI.ScriptNodeAlgo.setSelectedPaths( script, IECore.PathMatcher( selection ) )
self.assertEqual( path.property( "setPath:selectedMemberCount" ), count )
self.assertEqual( column.cellData( path ).value, count )

def testSetPathSelectedMemberCountWithInheritance( self ) :
def testSetSelectionColumnWithInheritance( self ) :

script = Gaffer.ScriptNode()

Expand Down Expand Up @@ -268,6 +270,8 @@ def testSetPathSelectedMemberCountWithInheritance( self ) :
self.assertTrue( path.isValid() )
self.assertFalse( path.isLeaf() )

column = _GafferSceneUI._SetEditor.SetSelectionColumn( script )

for parent, selection, count in [
( "/", [], None ),
( "/", [ "/group" ], None ),
Expand All @@ -290,7 +294,7 @@ def testSetPathSelectedMemberCountWithInheritance( self ) :

path.setFromString( parent )
GafferSceneUI.ScriptNodeAlgo.setSelectedPaths( script, IECore.PathMatcher( selection ) )
self.assertEqual( path.property( "setPath:selectedMemberCount" ), count )
self.assertEqual( column.cellData( path ).value, count )

def testSetPathCancellation( self ) :

Expand Down Expand Up @@ -351,19 +355,20 @@ def testSearchFilterRemovesEmptyParents( self ) :

def testEmptySetFilter( self ) :

plane = GafferScene.Plane()
plane["sets"].setValue( "A A:E B C D" )
script = Gaffer.ScriptNode()

emptySet = GafferScene.Set()
emptySet["name"].setValue( "EMPTY A:EMPTY" )
emptySet["in"].setInput( plane["out"] )
script["plane"] = GafferScene.Plane()
script["plane"]["sets"].setValue( "A A:E B C D" )

context = Gaffer.Context()
path = _GafferSceneUI._SetEditor.SetPath( emptySet["out"], context, "/" )
script["emptySet"] = GafferScene.Set()
script["emptySet"]["name"].setValue( "EMPTY A:EMPTY" )
script["emptySet"]["in"].setInput( script["plane"]["out"] )

path = _GafferSceneUI._SetEditor.SetPath( script["emptySet"]["out"], script.context(), "/" )

self.assertEqual( [ str( c ) for c in path.children() ], [ "/A", "/B", "/C", "/D", "/EMPTY" ] )

emptySetFilter = _GafferSceneUI._SetEditor.EmptySetFilter()
emptySetFilter = _GafferSceneUI._SetEditor.EmptySetFilter( script )
path.setFilter( emptySetFilter )

self.assertEqual( [ str( c ) for c in path.children() ], [ "/A", "/B", "/C", "/D" ] )
Expand All @@ -378,7 +383,7 @@ def testEmptySetFilter( self ) :
emptySetFilter.setEnabled( True )
self.assertEqual( [ str( c ) for c in path.children() ], [ "/A/A:E" ] )

def testEmptySetFilterWithSelectedMemberCount( self ) :
def testEmptySetFilterUsingSelection( self ) :

script = Gaffer.ScriptNode()

Expand All @@ -402,7 +407,7 @@ def testEmptySetFilterWithSelectedMemberCount( self ) :

self.assertEqual( [ str( c ) for c in path.children() ], [ "/A", "/B", "/C", "/D", "/EMPTY", "/F" ] )

emptySetFilter = _GafferSceneUI._SetEditor.EmptySetFilter( propertyName = "setPath:selectedMemberCount" )
emptySetFilter = _GafferSceneUI._SetEditor.EmptySetFilter( script, useSelection = True )
path.setFilter( emptySetFilter )

GafferSceneUI.ScriptNodeAlgo.setSelectedPaths( script, IECore.PathMatcher( [ "/plane" ] ) )
Expand Down
117 changes: 76 additions & 41 deletions src/GafferSceneUIModule/SetEditorBinding.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@

#include "SetEditorBinding.h"

#include "GafferSceneUI/ContextAlgo.h"
#include "GafferSceneUI/ScriptNodeAlgo.h"
#include "GafferSceneUI/TypeIds.h"

Expand All @@ -58,6 +57,7 @@

#include "IECorePython/RefCountedBinding.h"

#include "IECore/PathMatcherData.h"
#include "IECore/StringAlgo.h"

#include "boost/algorithm/string/predicate.hpp"
Expand Down Expand Up @@ -104,6 +104,19 @@ Path::Names parent( InternedString setName )
}
}

int numSelected( const PathMatcher &set, const PathMatcher &selectedPaths )
{
int result = 0;
// Consider inheritance in selected member count so descendants
// of set members are included in the count
for( PathMatcher::Iterator it = set.begin(), eIt = set.end(); it != eIt; ++it )
{
result += selectedPaths.subTree( *it ).size();
it.prune();
}
return result;
}

//////////////////////////////////////////////////////////////////////////
// LRU cache of PathMatchers built from set names
//////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -152,7 +165,7 @@ PathMatcherCache g_pathMatcherCache( pathMatcherCacheGetter, 25 );

const InternedString g_setNamePropertyName( "setPath:setName" );
const InternedString g_memberCountPropertyName( "setPath:memberCount" );
const InternedString g_selectedMemberCountPropertyName( "setPath:selectedMemberCount" );
const InternedString g_setPropertyName( "setPath:set" );

//////////////////////////////////////////////////////////////////////////
// SetPath
Expand Down Expand Up @@ -257,7 +270,7 @@ class SetPath : public Gaffer::Path
Path::propertyNames( names, canceller );
names.push_back( g_setNamePropertyName );
names.push_back( g_memberCountPropertyName );
names.push_back( g_selectedMemberCountPropertyName );
names.push_back( g_setPropertyName );
}

IECore::ConstRunTimeTypedPtr property( const IECore::InternedString &name, const IECore::Canceller *canceller = nullptr ) const override
Expand All @@ -270,7 +283,7 @@ class SetPath : public Gaffer::Path
return new StringData( names().back().string() );
}
}
else if( name == g_memberCountPropertyName )
else if( name == g_setPropertyName )
{
const PathMatcher p = pathMatcher( canceller );
if( p.match( names() ) & PathMatcher::ExactMatch )
Expand All @@ -280,33 +293,17 @@ class SetPath : public Gaffer::Path
{
scopedContext.setCanceller( canceller );
}
const auto setMembers = getScene()->set( names().back().string() );
return new IntData( setMembers->readable().size() );
return getScene()->set( names().back().string() );
}
}
else if( name == g_selectedMemberCountPropertyName )
else if( name == g_memberCountPropertyName )
{
const PathMatcher p = pathMatcher( canceller );
if( p.match( names() ) & PathMatcher::ExactMatch )
if( auto set = runTimeCast<const PathMatcherData>( property( g_setPropertyName, canceller ) ) )
{
Context::EditableScope scopedContext( getContext() );
if( canceller )
{
scopedContext.setCanceller( canceller );
}
const auto setMembers = getScene()->set( names().back().string() );
const auto selectedPaths = ContextAlgo::getSelectedPaths( Context::current() );
int memberCount = 0;
// Consider inheritance in selected member count so descendants
// of set members are included in the count
for( PathMatcher::Iterator it = setMembers->readable().begin(), eIt = setMembers->readable().end(); it != eIt; ++it )
{
memberCount += selectedPaths.subTree( *it ).size();
it.prune();
}
return new IntData( memberCount );
return new IntData( set->readable().size() );
}
}

return Path::property( name, canceller );
}

Expand Down Expand Up @@ -483,28 +480,31 @@ class SetMembersColumn : public StandardPathColumn
// SetSelectionColumn
//////////////////////////////////////////////////////////////////////////

class SetSelectionColumn : public StandardPathColumn
class SetSelectionColumn : public PathColumn
{

public :

IE_CORE_DECLAREMEMBERPTR( SetSelectionColumn )

SetSelectionColumn()
: StandardPathColumn( "Selection", g_selectedMemberCountPropertyName )
SetSelectionColumn( ScriptNodePtr script )
: PathColumn(), m_script( script ), m_selectedPaths( ScriptNodeAlgo::getSelectedPaths( script.get() ) )
{
ScriptNodeAlgo::selectedPathsChangedSignal( script.get() ).connect(
boost::bind( &SetSelectionColumn::selectedPathsChanged, this )
);
}

CellData cellData( const Gaffer::Path &path, const IECore::Canceller *canceller ) const override
{
CellData result = StandardPathColumn::cellData( path, canceller );
if( const auto setName = runTimeCast<const IECore::StringData>( path.property( g_setNamePropertyName, canceller ) ) )
CellData result;
if( const auto set = runTimeCast<const PathMatcherData>( path.property( g_setPropertyName, canceller ) ) )
{
const auto selectedMemberCount = runTimeCast<const IECore::IntData>( path.property( g_selectedMemberCountPropertyName, canceller ) );
const auto count = selectedMemberCount ? selectedMemberCount->readable() : 0;
int count = numSelected( set->readable(), m_selectedPaths );
result.value = new IntData( count );
result.toolTip = new IECore::StringData(
fmt::format( "{} selected scene location{} of {}",
count, count == 1 ? " is a member or descendant" : "s are members and/or descendants", setName->readable()
count, count == 1 ? " is a member or descendant" : "s are members and/or descendants", path.names().back().string()
)
);
}
Expand All @@ -514,17 +514,29 @@ class SetSelectionColumn : public StandardPathColumn

CellData headerData( const IECore::Canceller *canceller ) const override
{
CellData result = StandardPathColumn::headerData( canceller );
CellData result;
result.value = g_headerValue;
result.toolTip = g_headerToolTip;
return result;
}

private :

void selectedPathsChanged()
{
m_selectedPaths = ScriptNodeAlgo::getSelectedPaths( m_script.get() );
changedSignal()( this );
}

ScriptNodePtr m_script;
PathMatcher m_selectedPaths;

static const ConstStringDataPtr g_headerValue;
static const ConstStringDataPtr g_headerToolTip;

};

const ConstStringDataPtr SetSelectionColumn::g_headerValue = new StringData( "Selected" );
const ConstStringDataPtr SetSelectionColumn::g_headerToolTip = new StringData( "The number of selected scene locations that are set members, or their descendants" );

//////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -1047,9 +1059,16 @@ class SetEditorEmptySetFilter : public Gaffer::PathFilter

IE_CORE_DECLAREMEMBERPTR( SetEditorEmptySetFilter )

SetEditorEmptySetFilter( IECore::CompoundDataPtr userData = nullptr, const std::string &propertyName = g_memberCountPropertyName )
: PathFilter( userData ), m_propertyName( propertyName )
SetEditorEmptySetFilter( ScriptNodePtr scriptNode, bool useSelection = false, IECore::CompoundDataPtr userData = nullptr )
: PathFilter( userData ), m_scriptNode( scriptNode )
{
if( useSelection )
{
m_selectedPathsChangedConnection = ScriptNodeAlgo::selectedPathsChangedSignal( m_scriptNode.get() ).connect(
boost::bind( &SetEditorEmptySetFilter::selectedPathsChanged, this )
);
m_selectedPaths = ScriptNodeAlgo::getSelectedPaths( scriptNode.get() );
}
}

void doFilter( std::vector<PathPtr> &paths, const IECore::Canceller *canceller ) const override
Expand Down Expand Up @@ -1081,17 +1100,33 @@ class SetEditorEmptySetFilter : public Gaffer::PathFilter
}

bool members = false;
if( const auto memberCountData = IECore::runTimeCast<const IECore::IntData>( path->property( m_propertyName, canceller ) ) )
if( auto set = runTimeCast<const PathMatcherData>( path->property( g_setPropertyName ) ) )
{
members = memberCountData->readable() > 0;
if( m_selectedPathsChangedConnection.connected() )
{
members = numSelected( set->readable(), m_selectedPaths );
}
else
{
members = set->readable().size() > 0 ;
}
}

return leaf && !members;
}

private :

const InternedString m_propertyName;
void selectedPathsChanged()
{
m_selectedPaths = ScriptNodeAlgo::getSelectedPaths( m_scriptNode.get() );
changedSignal()( this );
}

ScriptNodePtr m_scriptNode;
// Only connected if using selection.
Gaffer::Signals::ScopedConnection m_selectedPathsChangedConnection;
PathMatcher m_selectedPaths;

};

Expand Down Expand Up @@ -1148,7 +1183,7 @@ void GafferSceneUIModule::bindSetEditor()
;

RefCountedClass<SetEditorEmptySetFilter, PathFilter>( "EmptySetFilter" )
.def( init<IECore::CompoundDataPtr, const std::string &>( ( boost::python::arg( "userData" ) = object(), boost::python::arg( "propertyName" ) = g_memberCountPropertyName ) ) )
.def( init<ScriptNodePtr, bool, CompoundDataPtr>( ( boost::python::arg( "scriptNode" ), boost::python::arg( "useSelection" ) = false, boost::python::arg( "userData" ) = object() ) ) )
;

RefCountedClass<SetNameColumn, GafferUI::PathColumn>( "SetNameColumn" )
Expand All @@ -1160,7 +1195,7 @@ void GafferSceneUIModule::bindSetEditor()
;

RefCountedClass<SetSelectionColumn, GafferUI::PathColumn>( "SetSelectionColumn" )
.def( init<>() )
.def( init<ScriptNodePtr>() )
;

RefCountedClass<VisibleSetInclusionsColumn, GafferUI::PathColumn>( "VisibleSetInclusionsColumn" )
Expand Down

0 comments on commit 1c0dfe1

Please sign in to comment.