MAYA-106477 selection and point snapping support for mtoh #776

huidong-chen · 2020-09-16T16:47:41Z

The change requires new Maya APIs and needs to be merged after Maya PR119 is published.

kxl-adsk · 2020-09-18T19:59:50Z

@marsupial can you review this change since you reported the missing API? I can't set you as a reviewer.

marsupial

Overall, good to see some movement here.
I'm curious how the point-snapping and collection API is going to play with renderers other than Storm.

marsupial · 2020-09-19T00:27:06Z

lib/usd/hdMaya/delegates/proxyDelegate.cpp

+
+            SdfPath usdPath = objectId.ReplacePrefix(delegateId, SdfPath::AbsoluteRootPath());
+
+            printf("usdPath: %s\n", usdPath.GetText());


Should get commented out.

This should have been removed by commit 9afde83.

marsupial · 2020-09-19T00:29:45Z

lib/usd/hdMaya/delegates/proxyDelegate.cpp

+{
+    if (selectInfo.pointSnapping()) {
+        for (const HdxPickHit& hit : hits) {
+            std::lock_guard<std::mutex> lock(_allAdaptersMutex);


Can the lock be acquired outside the for loop, or a comment stating why not.

marsupial · 2020-09-19T00:43:14Z

lib/usd/hdMaya/delegates/proxyDelegate.cpp

+        const SdfPath& objectId = hit.objectId;
+        const int instanceIndex = hit.instanceIndex;
+
+        std::lock_guard<std::mutex> lock(_allAdaptersMutex);


Can the lock be moved out of the for-loop? There's very-likely little contention for it, but avoiding the lock/unlock cycle for every hit seems better for code triggered from UI.

Good catch! I think it is better to move it outside of the for loop.

@kxl-adsk Can you confirm?

Of course, this makes perfect sense.

huidong-chen · 2020-09-22T17:58:35Z

I'm curious how the point-snapping and collection API is going to play with renderers other than Storm.

Good question. I tested HdStorm and HdEmbree, which also helps me understand the responsibility of each component.

The maya-to-hydra layer (which is TMK the render override code) manages the collections based on maya's frame context and selection info, then it executes the Hydra engine for the picking task.
The picking task is done by different render delegates/backends, and it should provide the hit result. We can compare the hit result of each render delegate here and identify bugs of different render delegates.
The maya-to-hydra layer will need to interpret the hit result and generate selection list (either Maya selection list or UFE selection list).

marsupial · 2020-09-23T22:41:22Z

I'm not sure 2. is accurate (or maybe it's just a semantic point), the Hydra pick-task is different between Storm & EverythingElse, but delegates don't really implement it...they push buffers compatible with what the Hydra pick-task would like.

That said, if Embree is doing what you want already, it's a pretty good sign. Waiting to get 119 installed so can check this out further.

huidong-chen · 2020-09-24T13:15:50Z

I didn't dig into how the pick task is implemented inside Hydra and/or render delegates, so 2. is more of a high-level concept.

huidong-chen · 2020-09-24T19:03:20Z

@marsupial Do you plan to test before approving the PR?

marsupial · 2020-09-24T19:18:14Z

I don't think I am an approver anyway, but starting to build/test now.

huidong-chen · 2020-09-24T19:19:56Z

Thanks. I see you are in the reviewers list, which is confusing.

kxl-adsk · 2020-09-24T20:16:48Z

@marsupial see my earlier comment - #776 (comment)

marsupial · 2020-09-24T22:40:54Z

Overall working great. There is an issue with instances though, what I wound up doing was replacing these lines:

maya-usd/lib/usd/hdMaya/delegates/proxyDelegate.cpp

Lines 306 to 315 in e031614

    
           auto proxyAdapter = findResult->second; 
        
           selectedSdfPaths.push_back(proxyAdapter->ConvertCachePathToIndexPath( 
        
               SdfPath(usdPathSegment.string()))); 
        
           selection->AddRprim( 
        
               HdSelection::HighlightModeSelect, selectedSdfPaths.back()); 
        
           TF_DEBUG(HDMAYA_AL_SELECTION) 
        
               .Msg( 
        
                   "HdMayaProxyDelegate::PopulateSelectedPaths - selecting %s\n", 
        
                   selectedSdfPaths.back().GetText());

With the following (the HdMayaProxyAdapter::delegate() method would need to be added as well):

 if (auto delegate = findResult->second->delegate())
    delegate->PopulateSelection(HdSelection::HighlightModeSelect, SdfPath(usdPathSegment.string()), usdImagingDelegate::ALL_INSTANCES, selection);

@pmolodo might have more info, but the original code was probably an older version of USD where instance-selection was much less robust then it currently is.

We've been using the PopulateSelection() call in this place for months, and should work for > 20.x (hopefully 19.11 too).

marsupial · 2020-09-24T22:47:41Z

Two outstanding issues related to selection (which I don't think should hold this up):

Really want this tied into the USD assembly, component selection mode in Maya (selecting leaf items is rarely what we want)
Out implementation also hooked this up to proxySelection via Storm when mtoh was not involved (otherwise the Storm selection selects the full usdProxyShape).

huidong-chen · 2020-09-25T14:17:05Z

@marsupial Thanks for pointing out the selection highlight issue and providing the code. I just pushed a new commit that follows your suggestion.

For the two outstanding issues, our priority for this task is to fix the blocking part, which is the missing API in maya for selection. For legacy issues and feature improvement, we will most likely rely on partners to do it. Hope it makes sense.

huidong-chen · 2020-09-25T14:19:40Z

We've been using the PopulateSelection() call in this place for months, and should work for > 20.x (hopefully 19.11 too).

Yes, the function has also been used in VP2RenderDelegate since last August.

marsupial · 2020-09-25T18:36:25Z

As noted, not an approver, but LGTM!
Agreed a great first step on the new API...just wanted to point out the other 2 points to where we'll still need to go, to try and minimize duplicate efforts.

kxl-adsk · 2020-09-26T11:21:27Z

@marsupial Re minimalizing duplicated efforts. We have no plans in near future to work on select by kind support in mtoh.

If I understood your second point correctly, you are talking about draw override integration of Storm (not mtoh). This is a legacy drawing routine and we are not investing in it. We are focusing on VP2RenderDelegate which removes a number of limitations like shadows across two data models, screen-space effects, etc. In which scenarios you are using this legacy rendering mode?

marsupial · 2020-09-28T17:15:30Z

@kxl-adsk: The 'legacy' rendering mode is the fallback for us. The VP2RenderDelegate is still quite a ways off from being useable. From prior conversations and the file we sent you:

40 minutes to load a files vs 40 sec with other delegates
UI very unresponsive when load completes
Everything is blue anyway

kxl-adsk · 2020-09-28T18:02:11Z

@marsupial Many materials problem you reported is next in line to look at. Good to know that it's the only adoption blocker you have.

pmolodo

Looks good. I think we might make situationally better use of the resolveMode, but I suppose that falls more under the class of an optimization.

pmolodo · 2020-09-29T19:42:27Z

lib/mayaUsd/render/mayaToHydra/renderOverride.cpp

+    pickParams.resolution.Set(view_w, view_h);
+    pickParams.viewMatrix.Set(viewMatrix.matrix);
+    pickParams.projectionMatrix.Set(projMatrix.matrix);
+    pickParams.resolveMode = HdxPickTokens->resolveUnique;


I think we probably want to vary this depending on what selectInfo.singleSelection and selectInfo.selectClosest return. resolveUnique makes sense for a marquee-selection scenario (within the limits under discussion here), but probably doesn't for single select.

Maybe, but I haven't seen issue for single selection when testing the code. With the code merged, I am sure there are still some improvements in the plugin side as we iterate over time. So maybe something for future?

Well, like I said, this falls more under the class of an optimization. It's a pretty easy check to make, and you'd then be able to skip the ResolveUniqueHits_Workaround if you were doing resolveNearestToCenter or resolveNearestToCamera... but you're likely also right, in that it may not be noticeable for single select, since the pick region is generally going to be pretty small. So your call.

pmolodo · 2020-09-29T21:39:30Z

lib/mayaUsd/render/mayaToHydra/renderOverride.h

@@ -175,6 +184,11 @@ class MtohRenderOverride : public MHWRender::MRenderOverride {
    HdRprimCollection _selectionCollection{
        HdReprTokens->wire, HdReprSelector(HdReprTokens->wire)
    };
+    HdRprimCollection _pointSnappingCollection{


Might be worth also hiding this one behind a 20210000 version guard, since it's only used there.

Good point.

huidong-chen · 2020-09-29T22:06:54Z

@kxl-adsk Please merge. Thanks.

MAYA-106477 selection and point snapping support for mtoh

8e107d8

huidong-chen requested review from pmolodo and kxl-adsk September 16, 2020 16:47

huidong-chen assigned ecp-maya-devops-adsk Sep 16, 2020

huidong-chen added mtoh Related to legacy Maya to Hydra plugin. do-not-merge-yet Development is not finished, PR not ready for merge pending beta This issue or PR will be testable in next beta release labels Sep 16, 2020

MAYA-106477 remove a debugging printf

9afde83

huidong-chen assigned ecp-maya-devops-adsk and unassigned ecp-maya-devops-adsk Sep 16, 2020

fix build errors

43b6f8f

huidong-chen assigned ecp-maya-devops-adsk and unassigned ecp-maya-devops-adsk Sep 17, 2020

marsupial reviewed Sep 22, 2020

View reviewed changes

MAYA-106477 Reduce mutex locking frequency

e031614

huidong-chen assigned ecp-maya-devops-adsk and unassigned ecp-maya-devops-adsk Sep 22, 2020

huidong-chen added ready-for-merge Development process is finished, PR is ready for merge and removed do-not-merge-yet Development is not finished, PR not ready for merge pending beta This issue or PR will be testable in next beta release labels Sep 24, 2020

huidong-chen removed the ready-for-merge Development process is finished, PR is ready for merge label Sep 25, 2020

MAYA-106477 fix selection highlight in moth for instanced prims

12ee864

huidong-chen assigned ecp-maya-devops-adsk and unassigned ecp-maya-devops-adsk Sep 25, 2020

kxl-adsk approved these changes Sep 26, 2020

View reviewed changes

pmolodo approved these changes Sep 29, 2020

View reviewed changes

Conditional compilation on the point snapping collection

edff755

huidong-chen added the ready-for-merge Development process is finished, PR is ready for merge label Sep 29, 2020

huidong-chen assigned ecp-maya-devops-adsk and unassigned ecp-maya-devops-adsk Sep 29, 2020

kxl-adsk merged commit 2400a86 into dev Sep 30, 2020

kxl-adsk deleted the HdC-adsk/MAYA-106477/mtoh-selection branch September 30, 2020 14:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MAYA-106477 selection and point snapping support for mtoh #776

MAYA-106477 selection and point snapping support for mtoh #776

huidong-chen commented Sep 16, 2020

kxl-adsk commented Sep 18, 2020 •

edited

Loading

marsupial left a comment

marsupial Sep 19, 2020

huidong-chen Sep 22, 2020

marsupial Sep 19, 2020

marsupial Sep 19, 2020

huidong-chen Sep 22, 2020

kxl-adsk Sep 22, 2020

huidong-chen commented Sep 22, 2020

marsupial commented Sep 23, 2020

huidong-chen commented Sep 24, 2020

huidong-chen commented Sep 24, 2020

marsupial commented Sep 24, 2020

huidong-chen commented Sep 24, 2020

kxl-adsk commented Sep 24, 2020

marsupial commented Sep 24, 2020

marsupial commented Sep 24, 2020

huidong-chen commented Sep 25, 2020

huidong-chen commented Sep 25, 2020

marsupial commented Sep 25, 2020

kxl-adsk commented Sep 26, 2020

marsupial commented Sep 28, 2020

kxl-adsk commented Sep 28, 2020

pmolodo left a comment

pmolodo Sep 29, 2020

huidong-chen Sep 29, 2020

pmolodo Sep 29, 2020

pmolodo Sep 29, 2020

huidong-chen Sep 29, 2020

huidong-chen commented Sep 29, 2020


		SdfPath usdPath = objectId.ReplacePrefix(delegateId, SdfPath::AbsoluteRootPath());

		printf("usdPath: %s\n", usdPath.GetText());

MAYA-106477 selection and point snapping support for mtoh #776

MAYA-106477 selection and point snapping support for mtoh #776

Conversation

huidong-chen commented Sep 16, 2020

kxl-adsk commented Sep 18, 2020 • edited Loading

marsupial left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

huidong-chen commented Sep 22, 2020

marsupial commented Sep 23, 2020

huidong-chen commented Sep 24, 2020

huidong-chen commented Sep 24, 2020

marsupial commented Sep 24, 2020

huidong-chen commented Sep 24, 2020

kxl-adsk commented Sep 24, 2020

marsupial commented Sep 24, 2020

marsupial commented Sep 24, 2020

huidong-chen commented Sep 25, 2020

huidong-chen commented Sep 25, 2020

marsupial commented Sep 25, 2020

kxl-adsk commented Sep 26, 2020

marsupial commented Sep 28, 2020

kxl-adsk commented Sep 28, 2020

pmolodo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

huidong-chen commented Sep 29, 2020

kxl-adsk commented Sep 18, 2020 •

edited

Loading