-
Notifications
You must be signed in to change notification settings - Fork 201
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
[al] added generate unique key method for translators (#1005) #248
[al] added generate unique key method for translators (#1005) #248
Conversation
…1005) * [PIPE-1121] Added generateUniqueKey method for translators * [PIPE-1121] Changed internal unique key type to std::size_t; minor code tweaked.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just added some very minor comments.
This looks good to me. I will be able to test building and running this code first thing on Monday. Sorry for the delay!
for(uint32_t j = 2; j < strings3.length(); ++j) | ||
{ | ||
if (strings3[j].substring(0, 10) == uniqueKeyPrefix) | ||
{ | ||
auto keyStr(strings3[j].substring(10, strings3[j].length())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it may be a good idea to have 10 be defined as a variable with a descriptive name (maybe uniqueKeyPrefixLength) instead of being hardcoded twice here
//---------------------------------------------------------------------------------------------------------------------- | ||
void TranslatorContext::updateUniqueKeys() | ||
{ | ||
auto stage = getUsdStage(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe check that stage != nullptr ?
@@ -64,25 +64,52 @@ PrimFilter::PrimFilter( | |||
// if the type remains the same, and the type supports update | |||
if(existingTranslatorId == newTranslatorId) | |||
{ | |||
if(supportsUpdate) | |||
// locate the path and delete from the removed set (we do not want to delete this prim! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
close parenthesis at the end of this comment
if (!res) { | ||
return 0; | ||
} | ||
TfPyLock pyLock; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I'm not very familiar with TfPyLock... are we using this lock here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was able to build and run this without any issue on our side.
Typically when using a custom translator, the following action occurs on a variant switch:
If a prim that had previously been translated into Maya, remains in the stage after a variant switch, and that prim supports update, then the translator's update method will be called. In some cases this update step may be entirely superfluous (because none of the data changed).
For some of the translators we use internally, there are cases where the cost of updating can be very large, so this PR adds a way to optionally generate a hash for a translated prim, which can be used again later on to determine whether update should be called or not.
Within a translator, you would override the generateUniqueKey method, to return a hashed value for a specific prim.