-
Notifications
You must be signed in to change notification settings - Fork 327
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
Render points with per-fragment depth. #6508
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
#import <../global_bindings.wgsl> | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is a pretty neat estimate and I appreciate how well you documented all of this, but ...
we're already doing literally that. It's a bit burried and we need to get the results of it out (or rather pass it in) but we do so as part of the coverage estimation |
||
// Routines for computing per-fragment depth of billboarded spheres or cylinders. | ||
// | ||
// Note: This technique is only an approximation. It ignores that depth is non-linear, so the | ||
// curve of the sphere's surface will be distorted. | ||
// | ||
// It also ignores the fact that spheres not in line with the camera's optical axis are | ||
// distorted by perspective projection in an asymmetric way — their outlines may be ellipses, | ||
// but the peak of the screen-space depth is *not* centered on the screen-space center of the | ||
// ellipse. In addition to that lack of distortion, the `projected_with_offset` point is the | ||
// nearest to the camera in world space, but is not the point with the nearest depth in screen | ||
// space, so the sphere's depth is lower than it should be. | ||
// | ||
// To get it right, we'd need to, essentially, compute the ray-tracing of a sphere. | ||
// But, overall, this produces spheres that have roughly consistent depth behavior independent of | ||
// view direction, which is good enough to, for example, make the look of intersections of | ||
// points and lines consistent as the camera orbits them. Just don't look too closely. | ||
|
||
|
||
// Compute the maximum `frag_depth` offset that a sphere, of radius `object_radius` | ||
// with center located at `object_position`, should have on the middle of its surface. | ||
// This may be used for cylinders too by giving the position of the nearest point on its | ||
// center line. | ||
fn sphere_radius_projected_depth(object_position: vec3f, object_radius: f32) -> f32 { | ||
// If the billboard were a round mesh, where would the point on its surface nearest the camera be? | ||
let front_point = object_position | ||
+ normalize(frame.camera_position - object_position) * object_radius; | ||
|
||
// Project that point and the object's center point. | ||
let projected_without_offset = frame.projection_from_world * vec4f(object_position, 1.0); | ||
let projected_with_offset = frame.projection_from_world * vec4f(front_point, 1.0); | ||
|
||
// Take the difference of the projected values. | ||
return projected_with_offset.z / projected_with_offset.w | ||
- projected_without_offset.z / projected_without_offset.w; | ||
} | ||
|
||
// Given the value computed by `sphere_radius_projected_depth()`, and the world-space position | ||
// of the current fragment, compute what should be added to its `frag_depth` to produce the | ||
// (approximate) depth value of the sphere. | ||
fn sphere_fragment_projected_depth( | ||
object_radius: f32, | ||
sphere_radius_projected_depth: f32, | ||
world_frag_offset_from_center: vec3f, | ||
) -> f32 { | ||
let offset_radius_squared = | ||
dot(world_frag_offset_from_center, world_frag_offset_from_center); | ||
let normalized_radius_squared = offset_radius_squared / (object_radius * object_radius); | ||
|
||
return sphere_radius_projected_depth * sqrt(clamp(1.0 - normalized_radius_squared, 0.0, 1.0)); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this only makes sense when we're actually rendering a sphere. Right now, we use this shader for both flat circles and spheres.
All of the sphere calculation & depth adjustment has to be conditional on
has_any_flag(batch.flags, FLAG_DRAW_AS_CIRCLES)
otherwise we might get weird artifacts.Since the threat of arbitrary depth changes will disable early z and hierarchical depth, I think we should ideally use different shaders for both. Implementing that is fairly cumbersome, so if we can prove it's not a big deal I'd rather not go there. However, users previously reported issues for high pointcloud overdraw (especially on zoomed-out point clouds) and this is likely to make it worse.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe that early Z is already disabled because the fragment shader uses
discard
. I'm not familiar with optimizations based on hierarchical depth so I can't comment on that.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm I think you're right. I'm a bit hazy on the details but since depth test+write is supposed to behave like an atomic operation, discard would break that already. Found this confirmed here https://stackoverflow.com/a/68602660
In a way this is great news: what we're doing here won't affect performance nearly as much as I feared (on paper) and right now we only care about (how much) we regress 🙂
Good you bring up hierarchical z: For hierarchical z to still work in some fashion, we should use conservative depth which is quite easy :) https://docs.rs/wgpu/latest/wgpu/core/naga/enum.ConservativeDepth.html
Unfortunately this hasn't made it into the wgsl spec yet, it's just a Naga extension. So unless this is just drop in and won't cause issues on web (don't know if that's the case!) we should just use it. Otherwise a todo note will suffice