Skip to content

Conversation

@mjkkirschner
Copy link
Member

@mjkkirschner mjkkirschner commented Feb 4, 2022

Purpose

A continuation of #11400 - now we also add depth bias to Dynamo point and line geometry.

This PR also removes the code related to vertex colors as it's even harder to figure out if points and lines need custom coloring or not - I've just removed it as it did not work anyway and would have been confusing to anyone who ventured here.

This PR assigns a default depth bias based on geometry type, and offsets this during selection, so that the depth bias ranges do not overlap.

This improves z fighting in most cases, though there are still some situations it does occur. Especially with curves - I've found that in some cases the issue is worse than z fighting - the lines are actually inside the surfaces because the faceting engine produces fewer straight line segments than the tessellated mesh surfaces, I think to fix this more robustly we would need to improve the tessellation algorithms in LibG for curves, or use higher precision for curves than for meshes, both potentially hurting performance.

IMO this PR is a pretty good improvement for a small change. Open to other suggestions though for sure.
the following images show this PR on the left and 2.13.1 on the right.
⚠️ note that for a pathologically bad case there are two copies of the tsplinesurface overlapping in the images.

Screen Shot 2022-02-03 at 5 17 26 PM
Screen Shot 2022-02-03 at 5 18 27 PM
Screen Shot 2022-02-03 at 5 18 06 PM
Screen Shot 2022-02-03 at 5 17 46 PM

Adds a few image comparison tests, I created them on a citrix machine so we'll need to see how/if they pass on the test machines.

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

Release Notes

improve rendering of coincident geometry

return;
}
//selected bias
newDepth = stdbias - DepthBiasSelectedOffset;
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this mean that for points, it will be negative? Is that a valid depth bias?

Copy link
Member Author

@mjkkirschner mjkkirschner Feb 4, 2022

Choose a reason for hiding this comment

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

yes - this is a good question - in addition to the DepthBias there is also a DepthBiasClamp -
https://docs.microsoft.com/en-us/windows/win32/direct3d11/d3d10-graphics-programming-guide-output-merger-stage-depth-bias

the docs seem to indicate that if the clamp is set to 0 - which ours should be unless helix does some other default, that it's not used.

then the final value is calculated like:

if ( (DepthBias != 0) || (SlopeScaledDepthBias != 0) )
    z = z + Bias

so this seems to be valid.

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.

Awesome stuff!

@mjkkirschner
Copy link
Member Author

@sm6srw I'm going to merge this, please feel free to send comments later, can address in another PR.

@mjkkirschner mjkkirschner merged commit f089517 into DynamoDS:master Feb 4, 2022
@mjkkirschner mjkkirschner deleted the linezfight branch February 4, 2022 19:09
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