slight improvement to rendering of coincident line/point/mesh geometry using depth bias#12607
Merged
mjkkirschner merged 3 commits intoDynamoDS:masterfrom Feb 4, 2022
Merged
Conversation
decrease depth for selected geom, then reset. forget about vertex colors.
| return; | ||
| } | ||
| //selected bias | ||
| newDepth = stdbias - DepthBiasSelectedOffset; |
Contributor
There was a problem hiding this comment.
Does this mean that for points, it will be negative? Is that a valid depth bias?
Member
Author
There was a problem hiding this comment.
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.
Member
Author
|
@sm6srw I'm going to merge this, please feel free to send comments later, can address in another PR. |
8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.
⚠️ note that for a pathologically bad case there are two copies of the tsplinesurface overlapping in the images.
the following images show this PR on the left and 2.13.1 on the right.
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
*.resxfilesRelease Notes
improve rendering of coincident geometry