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

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

Merged
merged 3 commits into from
Oct 6, 2020

Conversation

aparajit-pratap
Copy link
Contributor

@aparajit-pratap aparajit-pratap commented Oct 5, 2020

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.
isolate_pointline_geom

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

Comment on lines 531 to 547
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);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

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

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tried that. Doesn't work for some reason.

@mjkkirschner
Copy link
Member

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

@aparajit-pratap 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 aparajit-pratap merged commit d7777be into DynamoDS:master Oct 6, 2020
@aparajit-pratap aparajit-pratap deleted the isolate branch October 6, 2020 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants