Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Destination Plug on All BranchCreators #6215

Open
danieldresser-ie opened this issue Jan 16, 2025 · 0 comments
Open

Support Destination Plug on All BranchCreators #6215

danieldresser-ie opened this issue Jan 16, 2025 · 0 comments

Comments

@danieldresser-ie
Copy link
Contributor

John suggested moving the remaining conversation from #6213 to an issue. As well as fixing the specific bugs with the Instancer, we should sort out the problems with processesRootObject that prevents some of the subclasses of BranchCreator from exposing destinationPlug.

When discussing it this morning, it seems like we pretty well agreed on a direction that makes sense:

  • processesRootObject is not necessary - we can just always call computeBranchObject with branchPath.size() == 0 for a destination location, and the subclass can decide if it wants to always just do a passthrough when branchPath.size() == 0
  • What would be useful is a deleteSourceObject to control automatic deletion of the source.

These two seem to cover all existing usages of processesRootObject - they are either about supplying an object as the "root branch" ( bit of an oxymoron, but it kind of makes sense ... if you Unencapsulate a simple Cube, the cube object is the branch, that ends up at the root of the destination. ), or they're about deleting the source ( wanting to delete source mesh when doing a MeshSplit shouldn't depend on whether the destination is changed ... though maybe it should depend on a plug for the user to decide ).

What we discussed this morning is that processing the root object should override deleting the source. So Unencapsulate can delete the source, but if the destination is set to ${scene:path}, and computeBranchObject returns an object when the branchPath.size() == 0, then that object is used ( Unencapsulate always removes the capsule, but can also overwrite it if the Capsule contains an object at the root ).

I do have a question about how we implement this - I had originally pictured that the equivalent to having processesRootObject set to false would be computeBranchObject returning inPlug()->objectPlug()->getValue(); when branchPath.size() == 0. But in order to implement "computeBranchObject setting the root object overrides deleteSourceObject", we need to make it obvious whether it's setting something or passing it through? So maybe computeBranchObject should return nullptr when not setting the root object, and BranchCreator::computeObject will notice that, and replace it with a passthrough ( or a NullObject if it's a source and deleteSourceObject() is true? ).

We also discussed whether it should be optional to delete the source object - I proposed that we could implement this in subclasses inside the overridden deleteSourceObject, if appropriate? It's probably not useful to have an Unencapsulate that doesn't remove the capsule, but maybe it could be useful if MeshSplit::deleteSourceObject did something like return deleteSourceMeshPlug()->getValue()? Though, actually, then we'd need to have an affectsDeleteSourceObject call as well, that's a bit ugly? Maybe it's fine to have deleteSourceObject() be hardcoded. If you really want to keep your source mesh, you can always do a MergeScenes.

I think that covers most of what we discussed. The remaining thing is just how we implement the test in BranchCreator::computeObject for detecting source locations. I had suggested that querying the filter might not be that expensive compared to the BranchesData::locationOrAncestor query that's already happening ( a map lookup per length of the current scene path ), but I'd forgotten that filter matches aren't cached, so evaluating the filter would also require a map lookup per scene path element. So maybe it would be worth storing the source locations in BranchesData - but I don't see any reason to put them in the tree of Locations - if we're doing it for performance reasons, then it could just be a unordered_set of values of context->variableHash( scenePathContextName )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Up Next
Development

No branches or pull requests

1 participant