-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Autodesk: Skip use of geometry shader for patch tessellation when possible #3422
base: dev
Are you sure you want to change the base?
Autodesk: Skip use of geometry shader for patch tessellation when possible #3422
Conversation
Filed as internal issue #USD-10438 |
/AzurePipelines run |
Azure Pipelines successfully started running 1 pipeline(s). |
Exciting! |
I don't think the TfDebug code introduced here is needed. It matches existing functionality provided by TF_DEBUG=HDST_DUMP_GLSLFX_CONFIG |
@davidgyu PR has been updated based on this feedback. |
/AzurePipelines run |
Azure Pipelines successfully started running 1 pipeline(s). |
uint8_t tesIndex = 0; | ||
TES[tesIndex++] = _tokens->instancing; | ||
|
||
if (geomStyle == HdMeshGeomStyleEdgeOnSurf) { |
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 seems not quite right. Minimally, this should also test hasBuiltinBarycentrics
, which is the only thing that edges need from GS.
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.
Ah, right, when the fragment shader has barycentrics, it can render edges.
vec2 t1 = HdGet_st(i1); | ||
vec2 t2 = HdGet_st(i2); | ||
vec2 t3 = HdGet_st(i3); | ||
vec2 t = InterpolatePrimvar(t0, t1, t2, t3, basis, uv); |
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.
The signal driving displacement might not be named st
, e.g. an asset like usdImagingGL/testenv/testUsdImagingGLDisplacement/testUsdImagingGLDisplacement.usda
will fail during shader compilation since the displacement input coord is named st_faceVarying
.
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 see. I'm not quite sure what a comprehensive solution would look like. Will have to think about that.
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, that part of the codeGen texture accessors is delicate. I've been looking into refactoring the displacement terminal a little differently than you have here and I'll tty to share that back with you (possibly next week).
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.
That will help, thanks. I appreciate that.
Description of Change(s)
This updates the shader configuration for surface patches, for better performance. The geometry shader is in most cases omitted except when the display style is HdMeshGeomStyleEdgeOnSurf, in which case a geometry shader is required to render edges of output triangles.
The changes include new tessellation evaluation shaders, and an alternative DisplacementTerminal protocol in order to evaluate displacements correctly in a tessellation shader. Some changes also in codegen to include required supporting code such as interstage elements. Also included here is a TfDebug variable for listing shader keys.
Performance tests indicate approximately 40% speedup on average when rendering this type of geometry on a high-end laptop with an NVidia RTX A3000 GPU.
Link to proposal (if applicable)
Fixes Issue(s)
Checklist
I have created this PR based on the dev branch
I have followed the coding conventions
I have added unit tests that exercise this functionality (Reference:
testing guidelines)
I have verified that all unit tests pass with the proposed changes
I have submitted a signed Contributor License Agreement (Reference:
Contributor License Agreement instructions)