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

MAYA-108563 - Support default material shading using new SDK APIs #1339

Merged

Conversation

JGamache-autodesk
Copy link
Collaborator

Draws correctly, but selection fails to update in default material repr.

Draws correctly, but selection fails to update in default material repr.
@JGamache-autodesk JGamache-autodesk added bug Something isn't working do-not-merge-yet Development is not finished, PR not ready for merge vp2renderdelegate Related to VP2RenderDelegate labels Apr 14, 2021
@JGamache-autodesk
Copy link
Collaborator Author

Incomplete. One item remaining: selection does not update when dealing with the default material repr.

@JGamache-autodesk JGamache-autodesk removed the do-not-merge-yet Development is not finished, PR not ready for merge label Apr 16, 2021
@JGamache-autodesk JGamache-autodesk changed the title MAYA-108563 - Initial implementation MAYA-108563 - Support default material shading using new SDK APIs Apr 16, 2021
@@ -1597,7 +1642,8 @@ void HdVP2Mesh::_UpdateDrawItem(
}
#endif

if (desc.geomStyle == HdMeshGeomStyleHull) {
if (desc.geomStyle == HdMeshGeomStyleHull
&& desc.shadingTerminal == HdMeshReprDescTokens->surfaceShader) {
Copy link
Collaborator Author

@JGamache-autodesk JGamache-autodesk Apr 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

defaultMaterial repr has a different shading terminal and will skip the shading update.
Regular smoothMesh has "surfaceShader" as terminal, so this addition does not require version check.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we modify the real material while in default material mode does it update correctly when we turn off "use default material"? IIRC somewhere we clear all the dirty flags, so we'd forget about the dirty material id and then not update.

@JGamache-autodesk JGamache-autodesk added do-not-merge-yet Development is not finished, PR not ready for merge and removed bug Something isn't working labels Apr 16, 2021
@JGamache-autodesk
Copy link
Collaborator Author

Done and working, but can not be run thru preflight until some Maya updates are integrated.

@JGamache-autodesk JGamache-autodesk removed the do-not-merge-yet Development is not finished, PR not ready for merge label Apr 21, 2021
Copy link
Contributor

@williamkrick williamkrick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I put request changes until we clarify the question about material updates while in "use default material" mode.

if (reprToken == HdReprTokens->smoothHull
|| reprToken == HdVP2ReprTokens->defaultMaterial) {
// Share selection highlight render item between smoothHull and defaultMaterial:
bool foundShared = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we end up using this pattern to share more render items then this seems like a good candidate to be refactored into a method.

@@ -1597,7 +1642,8 @@ void HdVP2Mesh::_UpdateDrawItem(
}
#endif

if (desc.geomStyle == HdMeshGeomStyleHull) {
if (desc.geomStyle == HdMeshGeomStyleHull
&& desc.shadingTerminal == HdMeshReprDescTokens->surfaceShader) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we modify the real material while in default material mode does it update correctly when we turn off "use default material"? IIRC somewhere we clear all the dirty flags, so we'd forget about the dirty material id and then not update.

@kxl-adsk kxl-adsk merged commit 51988ce into dev May 6, 2021
@kxl-adsk kxl-adsk deleted the t_gamaj/MAYA-108563/implement_kDefaultMaterial_shading_mode branch May 6, 2021 15:00
JGamache-autodesk added a commit that referenced this pull request Oct 18, 2022
In #1339 code was added to share the selection render item between
reprs. The code did not track usage, so as soon as one of the reprs is
removed, the render item was removed from the subscene as well, leaving
dangling pointers in the remaining reprs.

In #1597 code was added that seem to indicate there was suspicion about
render item issues, but sharing was not identified as the cause.

This PR adds use count tracking to the render item so they only get
removed from the subscene once all reprs have stopped using the shared
render item.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
vp2renderdelegate Related to VP2RenderDelegate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants