-
Notifications
You must be signed in to change notification settings - Fork 202
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
Updates to support core USD imaging-related changes in latest dev branch #534
Conversation
…Hgi() for USD > 20.05 This change corresponds to core USD commit PixarAnimationStudios/OpenUSD@9d75ff8 (Internal change: 2069092)
This change corresponds to core USD commit PixarAnimationStudios/OpenUSD@8a8ab0b (Internal change: 2069810)
… 5 and up This change corresponds to core USD commit PixarAnimationStudios/OpenUSD@8a7ce92 (Internal change: 2069976)
This identifies core USD commit e7ff8e8 (post 20.05 release) as the most recent supported version of USD: PixarAnimationStudios/OpenUSD@e7ff8e8
We are still pretty much locked to 20.05 due to on-going work on Python3. @mattyjams can you confirm that this change should have no effect on us since all the changes should be guarded with USD version check? |
@kxl-adsk: That's right. These changes will continue to build with past USD releases (including 20.05, the current master branch), as well as the current dev branch, whose commit I identified in the build.md. |
@@ -1412,9 +1413,16 @@ HdDirtyBits HdVP2BasisCurves::_PropagateDirtyBits(HdDirtyBits bits) const | |||
// Propagate dirty bits to all draw items. | |||
for (const std::pair<TfToken, HdReprSharedPtr>& pair : _reprs) { | |||
const HdReprSharedPtr& repr = pair.second; | |||
#if HD_API_VERSION < 35 | |||
const HdRepr::DrawItems& items = repr->GetDrawItems(); |
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.
Would simple const auto& work in this case?
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.
Yeah, I think it probably would.
I'm not the original author of these changes so I'm not certain about their intent, but my guess is that they chose not to use auto
in order to make the change in API more obvious to the reader. I can pull the GetDrawItems()
call out of the conditional and use const auto&
instead if you prefer that though.
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.
Addressed by commit fd9aef0.
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 actually hoping it will make a bigger change...but didn't notice pointer vs reference difference. I will give up on it & thx for trying.
@@ -1472,9 +1480,16 @@ void HdVP2BasisCurves::_InitRepr(TfToken const &reprToken, HdDirtyBits *dirtyBit | |||
_reprs.begin(), _reprs.end(), _ReprComparator(reprToken)); | |||
if (it != _reprs.end()) { | |||
const HdReprSharedPtr& repr = it->second; | |||
#if HD_API_VERSION < 35 | |||
const HdRepr::DrawItems& items = repr->GetDrawItems(); |
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.
Same question about auto
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.
Addressed by commit fd9aef0.
@@ -573,9 +574,16 @@ HdDirtyBits HdVP2Mesh::_PropagateDirtyBits(HdDirtyBits bits) const { | |||
// Propagate dirty bits to all draw items. | |||
for (const std::pair<TfToken, HdReprSharedPtr>& pair : _reprs) { | |||
const HdReprSharedPtr& repr = pair.second; | |||
#if HD_API_VERSION < 35 | |||
const HdRepr::DrawItems& items = repr->GetDrawItems(); |
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.
We have some refactoring to do here to avoid duplication of code. Either way, the same comments from basisCurves apply in this file.
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.
Addressed by commit fd9aef0.
This PR includes and replaces PRs #530 and #531.