Skip to content

VisibilityQuery : Add node for querying scene visibility - #7137

Open
johnhaddon wants to merge 3 commits into
GafferHQ:1.7_maintenancefrom
johnhaddon:visibilityQuery
Open

VisibilityQuery : Add node for querying scene visibility#7137
johnhaddon wants to merge 3 commits into
GafferHQ:1.7_maintenancefrom
johnhaddon:visibilityQuery

Conversation

@johnhaddon

Copy link
Copy Markdown
Member

This outputs a boolean to say if a location is visible or not, and also a list of all ancestors which are invisible.

This outputs a boolean to say if a location is visible or not, and also a list of all ancestors which are invisible.

@murraystevenson murraystevenson left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks John! This looks like a handy addition to the query nodes. I've noted one potential hashing issue inline, but otherwise this is looking good to me.

Comment thread src/GafferScene/VisibilityQuery.cpp Outdated
void VisibilityQuery::hashInvisibleAncestors( const Gaffer::Context *context, IECore::MurmurHash &h ) const
{
ComputeNode::hash( invisibleAncestorsPlug(), context, h );
hashAncestorVisibilityForLocation( context, h );

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we also need to hash locationPlug() here? If I have two siblings "/group/sphere" & "/group/sphere1", both with scene:visible = False, changing the location plug from "/group/sphere" to "/group/sphere1" incorrectly shows "/group/sphere" in invisibleAncestors.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point - fixed in 2601d63.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Pending Review

Development

Successfully merging this pull request may close these issues.

2 participants