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

[glf] Support uint16 type for GL tetxures and OpenImageIO. #1212

Merged
merged 1 commit into from
Jun 30, 2020

Conversation

marsupial
Copy link
Contributor

Description of Change(s)

Allow usage of GL_UNSIGNED_SHORT for texture creation, and support proper type conversion for OIIO writing of a GlfImage.

Make sure to use sized OIIO constants, as OpenGL's constants are sized (https://www.khronos.org/opengl/wiki/OpenGL_Type)

Fixes Issue(s)

GLF/Hydra support 16-bit textures for half and unsigned int already, but uint16 textures will be downgraded to 8-bit.

@jtran56
Copy link

jtran56 commented May 22, 2020

Filed as internal issue #USD-6078.

@c64kernal
Copy link
Contributor

Thanks for the PR, @marsupial -- we accepted this PR despite the fact that we don't internally exercise these code paths. This means that we are likely to inadvertently break it again. Please consider submitting another PR with a test case that covers your use-case for this. Also we resolved the conflict, we tried to be careful, but again there's a chance that introduced some subtle issue. A test in a new PR would be really awesome. (Note that if the test requires imagediffs, we currently don't support that in the open source repo but we do support it internally, so even if it's a test that doesn't run meaningfully in open source, we can adapt it internally to verify correct results). Thanks again!

@pixar-oss pixar-oss merged commit 1405a8c into PixarAnimationStudios:dev Jun 30, 2020
@marsupial
Copy link
Contributor Author

Is there a template to follow for these imagediffs ?
(We're starting to getting more testing on our delegate but it would be nice to also test Storm elsewhere and don't want to be re-inventing the wheel).

@c64kernal
Copy link
Contributor

Here's an example of a test that internally we actually do imagediffs against that only run in open source to produce the images, but we don't actually diff against the baselines (internally we do though):

pxr/usdImaging/bin/testusdview/testenv/testUsdviewSkinning/

I'm not necessarily suggesting that you add a testusdview test though, you might be able to get away with something more direct inside of pxr/imaging/plugin/glfOiio/testenv (which would be the first one).

Come to think of it, we actually have a bunch of tests that aren't exposed to Open Source (yet) -- and in fact there are some tests that we should move to glfOiio as well.

So all of that is to say, maybe a testUsdview test is the easiest thing after all...

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

Successfully merging this pull request may close these issues.

5 participants