-
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 for building with latest post-20.11 dev branch of core USD #1025
updates for building with latest post-20.11 dev branch of core USD #1025
Conversation
UDIM texture loading for viewport renders moved from Glf to HdSt for core USD 21.02, and pxr/imaging/glf/udimTexture.{h,cpp} were removed. This change corresponds to core USD commit PixarAnimationStudios/OpenUSD@b386653 (Internal change: 2129931)
…should be included first Core USD 21.02 dropped the dependency on GLEW in favor of its own GL loader in garch.
…gate in mtoh For core USD 21.02, GlfContextCaps::InitInstance() was updated to call GarchGLApiLoad() itself in the following core USD commit: PixarAnimationStudios/OpenUSD@4034139 As a result, the call to GarchGLApiLoad() that was added in Autodesk#1007, and the accompanying include of glApi.h, were made redundant and are no longer necessary. The call to GlfGlewInit() remains for earlier versions of core USD.
For core USD 21.02 and beyond, the pxr imaging code no longer uses GLEW. These changes remove the GLEW dependency for 21.02+ and replace it with the Garch GL loader. (Internal change: 2130565)
This identifies core USD commit 5904885 (post 20.11 release) as the most recent supported version of USD: PixarAnimationStudios/OpenUSD@5904885
// We also disable clang-format for this block, since otherwise v10.0.0 fails | ||
// to recognize that "ProxyDrawOverride.h" is the related header. | ||
// clang-format off | ||
#include <pxr/pxr.h> |
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.
Is this include order intentional?
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 is, yeah. Internally, we define USD_VERSION_NUM=PXR_VERSION
, so we need to include pxr.h
first so that USD_VERSION_NUM
resolves to a valid value. I had left a comment to that effect in an earlier PR, but didn't apply the same comment here:
// GLEW must be included early (for core USD < 21.02), but we need pxr.h first |
The eventual goal would be that we could drop the maya-usd specific USD_VERSION_NUM
and just use PXR_VERSION
everywhere, since that's provided by USD. Now that I'm looking at it again, I think we'd actually be ready to do that now. @pmolodo fixed PXR_VERSION
to make it a useable value for version checks in PixarAnimationStudios/OpenUSD#886, which made it into core USD 19.11, so with that as the current minimum core USD version, it should be safe to switch over.
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.
This reminds me #645
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.
This reminds me #645
Ah, yup, well referenced. :)
A few small changes here adapting to recent updates in
garch
,glf
, andhdSt
in core USD.With core USD 21.02, the pxr imaging code no longer has a dependency on GLEW and instead uses its own GL loader in the
garch
library. Where code for USD prior to 21.02 would have calledGlfGlewInit()
, for 21.02 and later it should now callGarchGLApiLoad()
instead.GlfContextCaps::InitInstance()
callsGarchGLApiLoad()
itself, so any calls ofGlfContextCaps::InitInstance()
do not need an additional load call.Though it's not a strict requirement for
garch/glApi.h
to be included before any other GL header like it was forglew.h
, our Hydra team tells me it's probably still good practice, so it was added to the highest priority group in theclang-format
config.Some UDIM texture utilities also moved from
glf
tohdSt
, so the usage of that utility invp2RenderDelegate/material.cpp
was updated accordingly.