Skip to content

Conversation

@adubois-spin
Copy link

@adubois-spin adubois-spin commented Apr 2, 2025

There are two requests in this pull request:

1. At our studio the layer editor sometimes displays the wrong path when putting the mouse on the layer. Adding the right resolver context around the call and having a switch that takes the real path instead of the identifier solve the problem.

2. Rebuilding the model all the time takes time and only doing on a subset of cases improve the user experience.

@seando-adsk seando-adsk added do-not-merge-yet Development is not finished, PR not ready for merge and removed do-not-merge-yet Development is not finished, PR not ready for merge labels Apr 4, 2025
@seando-adsk seando-adsk added the workflows Related to in-context workflows label Apr 4, 2025
// to be empty for a brief time.
if (rowCount() > 0)
removeRows(0, rowCount());
clear();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this change required? Like the comments said, clear() would cause multiple notifications of changes to be sent and cause flickers. Any reason why changing it to call clear?

// Here we choose the identifier only if the layer is anonymous
std::string layerPath{usdLayer->IsAnonymous() ? usdLayer->GetIdentifier() :
usdLayer->GetRealPath()};
cmd += quote(layerPath);
Copy link
Collaborator

Choose a reason for hiding this comment

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

For cases like this where the same pattern is ued multiple times, it is better to put it in a function to avoid the repetition. Maybe a function named getLayerPathForEditorCommand()

Copy link
Author

Choose a reason for hiding this comment

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

Yes I agree a function would be better.
We did not make one because we didn't know where in the package it would be best defined.

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

Labels

workflows Related to in-context workflows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants