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

fix isolate selected regression, and longstanding issue with selection highlight. #11037

Merged
merged 4 commits into from
Aug 24, 2020

Conversation

mjkkirschner
Copy link
Member

@mjkkirschner mjkkirschner commented Aug 21, 2020

Purpose

In 2.7 there was a regression with isolateSelection, but I also found there was a long standing issue with overlapping geometry in some cases not highlighting correctly even during normal selection.

this pr does the following:

  • inside the dynamo mesh vertex shader - selected geometry is moved along the normal of each vertex by .01 - this is large enough to prevent z-fighting even when zoomed outed until the geometry is no longer visible.

    • geometry which requires vertex colors maintains its original movement of .0001
    • if a vertex is both vertex colored and selected it moves by .0101
  • visualization tests are added for two spheres in exactly the same location in isolation mode and regular selection mode.

Screen Shot 2020-08-21 at 4 22 49 PM

The reason that we need an additional increment for selected geometry is because I found most mesh geometry we generate has white vertex colors so all verts are moved slightly... essentially nothing special was being done for selected geometry. These colors were applied in LibG many years ago, and I don't think it is worth the risk to modify that at this point.

For those who are curious I found that the reason this worked in 2.6 for isolated geometry, which was that after every interaction we would resort all geometry in the scene so that the selected geometry was rendered last... it's great to be able to skip this given that this sorting could be slow if there was a lot of geometry.

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

@mjkkirschner mjkkirschner added this to the 2.8 milestone Aug 21, 2020
float3 nudge = normalize(input.n) * 0.0001;
inputp = float4(input.p.x + nudge.x, input.p.y + nudge.y, input.p.z + nudge.z, input.p.w);
// Nudge the vertex out slightly along its normal a tiny bit.
inputp = float4(inputp.x + nudge.x, inputp.y + nudge.y, inputp.z + nudge.z, inputp.w);
Copy link
Contributor

Choose a reason for hiding this comment

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

In this case if there are say 2 overlapping geometries, won't they both be nudged outwards? If so, what will this achieve?

Copy link
Contributor

@aparajit-pratap aparajit-pratap left a comment

Choose a reason for hiding this comment

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

Just one comment/question.

@QilongTang
Copy link
Contributor

@DynamoDS/dynamo Team decided to merge for Dynamo 2.8 and cherry-pick for now.

@QilongTang QilongTang merged commit 89818c3 into DynamoDS:master Aug 24, 2020
QilongTang pushed a commit that referenced this pull request Aug 24, 2020
…n highlight. (#11037)

* this works, but ideally it would be done in the shader to avoid this sorting

* update shader to make selected geometry more prominent than vertColored geo
new tests

* move camera closer
regen images
disable update image define
remove unneccesary sorting

* fix copy paste error
@QilongTang QilongTang mentioned this pull request Aug 24, 2020
8 tasks
QilongTang added a commit that referenced this pull request Aug 24, 2020
…n highlight. (#11037) (#11041)

* this works, but ideally it would be done in the shader to avoid this sorting

* update shader to make selected geometry more prominent than vertColored geo
new tests

* move camera closer
regen images
disable update image define
remove unneccesary sorting

* fix copy paste error

Co-authored-by: Michael Kirschner <[email protected]>
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.

3 participants