Skip to content

Conversation

@yannci
Copy link
Contributor

@yannci yannci commented Jan 10, 2023

PR #1290 addressed various issues when drawing scene caches in maya's ViewPort2. This PR attempts to fix a regression introduced by this PR. The actual visibility determination is done correctly by LiveScene but then stomped over by any value present in the cache itself, disregarding overrides made in the scene. This change was introduces to fix an issue where lower level locations would be drawn even with an override higher up. But with the LiveScene implementation this seems not needed anymore.

We also address an issue for scene shapes that represent a root location, e.g. when we have set it to objectOnly where the various instances never checked visibility when set to "draw root bounds". That means we would always draw the bounds in the viewport even when overriding the visibility.

yannci added 2 commits January 9, 2023 16:40
In df78cb8 changes were made to the
`LiveScene` implementation so that we take into account visibility
overrides higher up the hierarchy. This makes the changes introduced
in 07b40ee obsolete. In fact a
`scene:visibility` value stored in the cache stomps over the
visibility determination done by `LiveScene`.
In case we have a root location i.e. we set a shape node to
`objectOnly` and we also set "draw root bounds", the visibility
of the current instance was never taken into accout and we draw the
bounds regardless. We address this and skip adding a render item for
every instance that is set to invisible.
@ivanimanishi
Copy link
Member

Shouldn't this be a PR to RB-10.3?

@yannci yannci changed the base branch from main to RB-10.3 January 11, 2023 17:51
@yannci
Copy link
Contributor Author

yannci commented Jan 11, 2023

Shouldn't this be a PR to RB-10.3?

Thanks for the catch! Changed the base branch to RB-10.3

@ivanimanishi ivanimanishi merged commit 2d8c179 into ImageEngine:RB-10.3 Jan 12, 2023
@ivanimanishi ivanimanishi deleted the ieVisibility branch January 12, 2023 16:47
@ivanimanishi ivanimanishi restored the ieVisibility branch January 12, 2023 16:47
@bradleyhenke
Copy link
Contributor

I poked my head in here out of curiosity after running into Yannic on the street a while back. I'm glad you got it all sorted now!

I will say that one benefit of having that check in the code was the ability to prune the traversal of any hidden child locations. Without it, you could potentially lose a little performance if you visit a large number of descendent locations unnecessarily. I don't have a build of cortex readily available, but I wonder if it would be enough to use the visibility info gathered from LiveScene (via m_instances) if you are dealing with a root location, and fall back to the visibility attribute in the cache for any unexpanded locations? The unexpanded locations shouldn't have any way of being overridden, so I think it would be safe to read that value from the cache.

if(isRoot)
{
	if( std::none_of(m_instances.cbegin(), m_instance.cend(), [](const Instance &instance){ instance.visible } )
	{
		return;
	}
}
else
{
	if( sceneInterface->hasAttribute( SceneInterface::visibilityName ) )
	{
		ConstBoolDataPtr vis = runTimeCast<const BoolData>( sceneInterface->readAttribute( SceneInterface::visibilityName, m_time ) );
		if( vis && !vis->readable() )
		{
			return;
		}
	}
}

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants