Reorder Helix elements for Isolate Geometry Mode to display isolated geometry #11163

aparajit-pratap · 2020-10-05T23:06:43Z

Purpose

https://jira.autodesk.com/browse/DYN-3130

This attempts to fix a regression with Isolate Selected Geometry feature where point-line geometry when selected does not appear when hidden by mesh geometry (solids and surfaces). The reason for this is that the geometry needs to be reordered so that selected geometries will have higher precedence and rendered closer to the camera compared to unselected geometries.

Caveat: This comes with some performance penalty when rendering selected geometry in isolated mode.

Declarations

Check these if you believe they are true

The codebase is in a better state after this PR
Is documented according to the standards
The level of testing this PR includes is appropriate
User facing strings, if any, are extracted into *.resx files
All tests pass using the self-service CI.
Snapshot of UI changes, if any.
Changes to the API follow Semantic Versioning and are documented in the API Changes document.
This PR modifies some build requirements and the readme is updated

FYIs

(FILL ME IN, Optional) Names of anyone else you wish to be notified of

aparajit-pratap · 2020-10-05T23:08:28Z

src/DynamoCoreWpf/ViewModels/Watch3D/HelixWatch3DViewModel.cs

+            if (Element3DDictionary == null)
+            {
+                sceneItems = new ObservableElement3DCollection();
+                return;
+            }
+
+            var values = Element3DDictionary.Values.ToList();
+            if (Camera != null)
+            {
+                values.Sort(new Element3DComparer(Camera.Position));
+            }
+
+            if (sceneItems == null)
+                sceneItems = new ObservableElement3DCollection();
+
+            sceneItems.Clear();
+            sceneItems.AddRange(values);


This logic has been lifted off of UpdateSceneItems, however, UpdateSceneItems seems to be doing some extra work like cycling through the list of elements and updating vertices, triangles, and colors, which I'm not sure if that is necessary in this case. Skipping that saves some time.

src/DynamoCoreWpf/ViewModels/Watch3D/HelixWatch3DViewModel.cs

mjkkirschner · 2020-10-06T00:34:45Z

src/DynamoCoreWpf/ViewModels/Watch3D/HelixWatch3DViewModel.cs

+            var values = Element3DDictionary.Values.ToList();
+            if (Camera != null)
+            {
+                values.Sort(new Element3DComparer(Camera.Position));


Why does this need to happen every time the selection changes? Couldn't it be done once when the isolation mode is enabled?

Tried that. Doesn't work for some reason.

mjkkirschner · 2020-10-06T13:39:09Z

nice! @aparajit-pratap

would you also mind adding an image comparison test for this so it does not regress again? I added a few for the mesh mesh case in the last pr.

89818c3

sort helix elements for every selection changed event

c7b108c

aparajit-pratap commented Oct 5, 2020

View reviewed changes

aparajit-pratap requested a review from mjkkirschner October 5, 2020 23:08

mjkkirschner reviewed Oct 6, 2020

View reviewed changes

src/DynamoCoreWpf/ViewModels/Watch3D/HelixWatch3DViewModel.cs Outdated Show resolved Hide resolved

mjkkirschner reviewed Oct 6, 2020

View reviewed changes

aparajit-pratap added the WIP label Oct 6, 2020

refactor

072f8b7

aparajit-pratap removed the WIP label Oct 6, 2020

aparajit-pratap requested a review from mjkkirschner October 6, 2020 12:47

mjkkirschner approved these changes Oct 6, 2020

View reviewed changes

image comparison test

082917d

aparajit-pratap changed the title ~~Sort helix elements for every node selection changed event~~ Reorder Helix elements for Isolate Geometry Mode to display isolated geometry Oct 6, 2020

aparajit-pratap merged commit d7777be into DynamoDS:master Oct 6, 2020

aparajit-pratap deleted the isolate branch October 6, 2020 20:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Reorder Helix elements for Isolate Geometry Mode to display isolated geometry #11163

Reorder Helix elements for Isolate Geometry Mode to display isolated geometry #11163

Uh oh!

aparajit-pratap commented Oct 5, 2020 •

edited

Loading

Uh oh!

aparajit-pratap Oct 5, 2020

Uh oh!

Uh oh!

mjkkirschner Oct 6, 2020

Uh oh!

aparajit-pratap Oct 6, 2020

Uh oh!

mjkkirschner commented Oct 6, 2020

Uh oh!

Uh oh!

Reorder Helix elements for Isolate Geometry Mode to display isolated geometry #11163

Reorder Helix elements for Isolate Geometry Mode to display isolated geometry #11163

Uh oh!

Conversation

aparajit-pratap commented Oct 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose

Declarations

FYIs

Uh oh!

aparajit-pratap Oct 5, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mjkkirschner Oct 6, 2020

Choose a reason for hiding this comment

Uh oh!

aparajit-pratap Oct 6, 2020

Choose a reason for hiding this comment

Uh oh!

mjkkirschner commented Oct 6, 2020

Uh oh!

Uh oh!

aparajit-pratap commented Oct 5, 2020 •

edited

Loading