Ogs/selection #1831

AdamFelt · 2022-04-14T23:04:59Z

This submission adds deep selection capabilities to HdStorm. This allows for selection of all objects within a pick-point or frustum to be selected even if the objects are obscured by other objects drawn on top.

It uses a GPU rendering technique similar to ID based picking except that it accummulates all pick results into a one dimensional "pickBuffer" instead of storing a single selection per pixel like the ID buffer approach does.

It can be enabled by setting a new resolve mode defined as HdxPickTokens->resolveDeep.

A new helper function named TestIntersections (plural) has been added to UsdImagingGlEngine to exercise the new functionality. A new Unit test is included to test the new resolve mode and UsdImagingGLEngine helper function.

[x ] I have submitted a signed Contributor License Agreement

…put there only unique entries

- Predefined needed tokens

# Conflicts: # pxr/imaging/hdx/pickTask.cpp # pxr/imaging/hdx/pickTask.h

…discard translucent pixels

…put there only unique entries

- Predefined needed tokens

…discard translucent pixels

Basic start to deep selection and rectangular selection in the Engine class.

Restore single pick functionality when picking, not rectangle selection

Fix merge conflicts Drive deep selection through new methods on UsdImagingGLEngine Add unit tests and baselines

…nto ogs/selection # Conflicts: # pxr/imaging/hdx/pickTask.cpp # pxr/imaging/hdx/pickTask.h

jilliene · 2022-04-21T22:15:35Z

Filed as internal issue #USD-7330

tcauchois · 2022-05-21T00:01:52Z

Hey all, we've finished looking at this internally and it looks good. I have some minor feedback on the code I'll post in a second, if you're up for addressing it.

We were wondering if, for the PR, you could squash the history a bit. Also, have you tried this on Metal at all?

tcauchois · 2022-05-21T00:02:46Z

pxr/imaging/hdx/pickTask.cpp

@@ -56,6 +57,17 @@ PXR_NAMESPACE_OPEN_SCOPE

 TF_DEFINE_PUBLIC_TOKENS(HdxPickTokens, HDX_PICK_TOKENS);

+TF_DEFINE_PRIVATE_TOKENS(
+    HdxPickPrivateTokens,


Stylistic thing: we use "_tokens" to indicate private tokens.

tcauchois · 2022-05-21T00:03:07Z

pxr/imaging/hdx/pickTask.cpp

+    (Picking)
+);
+
+static const int PICK_BUFFER_HEADER_SIZE = 8;


A note indicating these are in units of "int" would be great!

tcauchois · 2022-05-21T00:06:53Z

pxr/imaging/hdx/shaders/renderPass.glslfx

+
+    // try to initialize the values and check if succeeded
+
+    primIdValue = atomicCompSwap(PickBuffer[primIdOffset], -9, primId);


Here and elsewhere: this needs to be updated for metal support. See top-of-tree WriteOitLayersToBufferCommon, in this same file, for the layout section and the ATOMIC_EXCHANGE macro usage.

tcauchois · 2022-05-21T00:09:57Z

pxr/imaging/hdx/shaders/renderPass.glslfx

+    // add the item to the pick buffer, but only unique
+#if defined(HD_HAS_PickBuffer)
+
+    // if the pick buffer is not initialized, we are done 


Somewhere (here, pickTask.cpp, elsewhere?) it would be fantastic to have both a block comment about the buffer layout, and a block comment about the algorithm you're using to write the entry out, since they're both tricky to pick up from reading the code.

tcauchois · 2022-05-21T00:11:10Z

pxr/imaging/hdx/shaders/renderPass.glslfx

+                primId, instanceId, partId)) {
+            keepMoving = false;
+        }
+        else if(++entryNumber == subBufferCapacity) {


Not important, but if you move this into the while() condition, and make the "keepMoving" above a "break", it makes the loop code a bit smaller.

tcauchois · 2022-05-21T00:23:37Z

pxr/usdImaging/usdImagingGL/engine.h

+    /// instancer (if applicable) of each gprim. 
+    ///
+    USDIMAGINGGL_API
+    bool TestIntersections(


We've been talking about a new TestIntersection API, unrelated to this change. If you want to do us a favor, if you call this "TestIntersection" and take a "const TfToken& resolveMode", and swap out the "const UsdPrim& root" for a "const SdfPathVector&" (like RenderBatch), we can call that the new intersection API, and I can mark the existing one as deprecated.

Also: if you can bump the version in usdImagingGL/version.h, that would be great.

tcauchois · 2022-05-23T17:57:59Z

pxr/usdImaging/usdImagingGL/testenv/testUsdImagingGLPickAndHighlight.cpp

+    bool res = lhsVector.size() == rhsVector.size();
+    if (!res) return false;
+
+    for (int i = 0; i < lhsVector.size(); ++i)


int -> size_t

tcauchois · 2022-05-23T17:58:13Z

pxr/usdImaging/usdImagingGL/testenv/testUsdImagingGLPickAndHighlight.cpp

+        params,
+        outResults)) {
+
+        for (int i = 0; i < outResults.size(); ++i)


int -> size_t

tcauchois · 2022-05-26T19:20:48Z

pxr/imaging/hdx/shaders/renderPass.glslfx

+}
+
+bool 
+compareOrSet(


Missed it before, but compareOrSet needs to be behind ifdef HD_HAS_PickBuffer as well.

tcauchois · 2022-05-26T19:23:23Z

pxr/usdImaging/usdImagingGL/testenv/testUsdImagingGLPickAndHighlight.cpp

@@ -238,6 +298,17 @@ My_TestGLDrawing::DrawTest(bool offscreen)
    } else {
        Draw();
    }
+


Missed this before, but can you add an if (renderer == HdStormRendererPlugin) here? Or else add a command line flag to control whether we test deep selection.

tcauchois · 2022-08-04T23:56:08Z

Adam: let me know if you need anything on this PR.

erikaharrison-adsk · 2024-04-16T22:32:08Z

Feedback has been addressed in the new PR: #3044

vlasovi added 30 commits November 4, 2021 13:04

Deep selection first prototype

b332099

Add-on to the previous commit: the pick shader

3d5a56e

Implement a hash table in the shader to reduce the number of entries

be5315e

Introducing a new resolve mode in HdxPickTask: resolveDeep

169ce03

Minor refactoring

e07ce6c

Changing the way we collect data in pick buffer to make sure that we …

cb456b0

…put there only unique entries

refactoring

264d000

- Disable depth write when doing deep selection

bdcfdf6

- Predefined needed tokens

Adding support for instanceId, faceId, edgeId and pointId

959398a

Small refactoring

d38d227

Improving the way we clear pick buffer

10f4456

Adding comments

4d6207e

Merge remote-tracking branch 'public/dev' into ogs/selection

b28c0a0

# Conflicts: # pxr/imaging/hdx/pickTask.cpp # pxr/imaging/hdx/pickTask.h

Fixing a bug caused by the merge

a8c2477

Minor improvements after the code review

d3dc119

Introducing alphaThreshold in HdxPickTaskContextParams to be able to …

f72e385

…discard translucent pixels

Deep selection first prototype

8d37f3c

Add-on to the previous commit: the pick shader

5a8f2c8

Implement a hash table in the shader to reduce the number of entries

f63f881

Introducing a new resolve mode in HdxPickTask: resolveDeep

7a97067

minor refactoring

bf78e0d

Changing the way we collect data in pick buffer to make sure that we …

862ff44

…put there only unique entries

refactoring

16a8999

- Disable depth write when doing deep selection

677b807

- Predefined needed tokens

Adding support for instanceId, faceId, edgeId and pointId

db60b2a

Small refactoring

e4c02dc

Improving the way we clear pick buffer

b629b88

Adding comments

c506da6

Fixing a bug caused by the merge

b00e4ba

Minor improvements after the code review

ebb3bc2

vlasovi and others added 6 commits April 7, 2022 11:04

Introducing alphaThreshold in HdxPickTaskContextParams to be able to …

3d35d62

…discard translucent pixels

Add deep selection to UsdImagingGlEngine

4c03638

Basic start to deep selection and rectangular selection in the Engine class.

Deep selection related changes.

c8d4a48

Restore single pick functionality when picking, not rectangle selection

Deep selection improvements

a6ca92c

Fix merge conflicts Drive deep selection through new methods on UsdImagingGLEngine Add unit tests and baselines

Merge branch 'ogs/selection' of git.autodesk.com:autodesk-forks/USD i…

2a29f43

…nto ogs/selection # Conflicts: # pxr/imaging/hdx/pickTask.cpp # pxr/imaging/hdx/pickTask.h

Code cleanup based on code review.

f9340ed

tcauchois reviewed May 21, 2022

View reviewed changes

tcauchois reviewed May 23, 2022

View reviewed changes

tcauchois reviewed May 26, 2022

View reviewed changes

erikaharrison-adsk mentioned this pull request Apr 16, 2024

Autodesk: Deep Selection #3044

Open

2 tasks

AdamFelt closed this Apr 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ogs/selection #1831

Ogs/selection #1831

AdamFelt commented Apr 14, 2022

jilliene commented Apr 21, 2022

tcauchois commented May 21, 2022

tcauchois May 21, 2022

tcauchois May 21, 2022

tcauchois May 21, 2022

tcauchois May 21, 2022

tcauchois May 21, 2022

tcauchois May 21, 2022

tcauchois May 21, 2022

tcauchois May 23, 2022

tcauchois May 23, 2022

tcauchois May 26, 2022

tcauchois May 26, 2022

tcauchois commented Aug 4, 2022

erikaharrison-adsk commented Apr 16, 2024


		// try to initialize the values and check if succeeded

		primIdValue = atomicCompSwap(PickBuffer[primIdOffset], -9, primId);

Ogs/selection #1831

Ogs/selection #1831

Conversation

AdamFelt commented Apr 14, 2022

jilliene commented Apr 21, 2022

tcauchois commented May 21, 2022

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

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

tcauchois commented Aug 4, 2022

erikaharrison-adsk commented Apr 16, 2024