-
Notifications
You must be signed in to change notification settings - Fork 293
Use z-buffer for rejecting opaque fragments. #648
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
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 |
---|---|---|
|
@@ -73,6 +73,7 @@ in int aClipTaskIndex; | |
in int aLayerIndex; | ||
in int aElementIndex; | ||
in ivec2 aUserData; | ||
in int aZIndex; | ||
|
||
// get_fetch_uv is a macro to work around a macOS Intel driver parsing bug. | ||
// TODO: convert back to a function once the driver issues are resolved, if ever. | ||
|
@@ -286,6 +287,7 @@ struct PrimitiveInstance { | |
int clip_task_index; | ||
int layer_index; | ||
int sub_index; | ||
int z; | ||
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. we could probably reduce the 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. Yep, I think it's fine to do this as a follow up though. |
||
ivec2 user_data; | ||
}; | ||
|
||
|
@@ -299,6 +301,7 @@ PrimitiveInstance fetch_prim_instance() { | |
pi.layer_index = aLayerIndex; | ||
pi.sub_index = aElementIndex; | ||
pi.user_data = aUserData; | ||
pi.z = aZIndex; | ||
|
||
return pi; | ||
} | ||
|
@@ -336,6 +339,7 @@ struct Primitive { | |
// this index allows the vertex shader to recognize the difference | ||
int sub_index; | ||
ivec2 user_data; | ||
float z; | ||
}; | ||
|
||
Primitive load_primitive_custom(PrimitiveInstance pi) { | ||
|
@@ -352,6 +356,7 @@ Primitive load_primitive_custom(PrimitiveInstance pi) { | |
prim.prim_index = pi.specific_prim_index; | ||
prim.sub_index = pi.sub_index; | ||
prim.user_data = pi.user_data; | ||
prim.z = float(pi.z); | ||
|
||
return prim; | ||
} | ||
|
@@ -424,6 +429,7 @@ struct VertexInfo { | |
|
||
VertexInfo write_vertex(vec4 instance_rect, | ||
vec4 local_clip_rect, | ||
float z, | ||
Layer layer, | ||
Tile tile) { | ||
vec2 p0 = floor(0.5 + instance_rect.xy * uDevicePixelRatio) / uDevicePixelRatio; | ||
|
@@ -451,7 +457,7 @@ VertexInfo write_vertex(vec4 instance_rect, | |
|
||
vec2 final_pos = clamped_pos + tile.screen_origin_task_origin.zw - tile.screen_origin_task_origin.xy; | ||
|
||
gl_Position = uTransform * vec4(final_pos, 0, 1); | ||
gl_Position = uTransform * vec4(final_pos, z, 1.0); | ||
|
||
VertexInfo vi = VertexInfo(Rect(p0, p1), local_clamped_pos.xy, clamped_pos.xy); | ||
return vi; | ||
|
@@ -467,6 +473,7 @@ struct TransformVertexInfo { | |
|
||
TransformVertexInfo write_transform_vertex(vec4 instance_rect, | ||
vec4 local_clip_rect, | ||
float z, | ||
Layer layer, | ||
Tile tile) { | ||
vec2 lp0_base = instance_rect.xy; | ||
|
@@ -518,7 +525,7 @@ TransformVertexInfo write_transform_vertex(vec4 instance_rect, | |
// apply the task offset | ||
vec2 final_pos = clamped_pos + tile.screen_origin_task_origin.zw - tile.screen_origin_task_origin.xy; | ||
|
||
gl_Position = uTransform * vec4(final_pos, 0.0, 1.0); | ||
gl_Position = uTransform * vec4(final_pos, z, 1.0); | ||
|
||
return TransformVertexInfo(layer_pos.xyw, clamped_pos, clipped_local_rect); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,6 +39,7 @@ void main(void) { | |
#ifdef WR_FEATURE_TRANSFORM | ||
TransformVertexInfo vi = write_transform_vertex(segment_rect, | ||
prim.local_clip_rect, | ||
prim.z, | ||
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. at this point, we might as well want to just pass 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. Some of the shaders generate the local rect separately (as in the case above), which is why I wasn't passing in the prim directly - to avoid bugs where that code accesses the prim.local_rect accidentally. But we could probably tidy up those structure definitions in the shader code. |
||
prim.layer, | ||
prim.tile); | ||
vLocalRect = vi.clipped_local_rect; | ||
|
@@ -47,6 +48,7 @@ void main(void) { | |
#else | ||
VertexInfo vi = write_vertex(segment_rect, | ||
prim.local_clip_rect, | ||
prim.z, | ||
prim.layer, | ||
prim.tile); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -48,6 +48,11 @@ lazy_static! { | |
}; | ||
} | ||
|
||
#[repr(u32)] | ||
pub enum DepthFunction { | ||
Less = gl::LESS, | ||
} | ||
|
||
#[derive(Copy, Clone, Debug, PartialEq)] | ||
pub enum TextureTarget { | ||
Default, | ||
|
@@ -172,6 +177,7 @@ impl VertexFormat { | |
VertexAttribute::ClipTaskIndex, | ||
VertexAttribute::LayerIndex, | ||
VertexAttribute::ElementIndex, | ||
VertexAttribute::ZIndex, | ||
].into_iter() { | ||
gl::enable_vertex_attrib_array(attrib as gl::GLuint); | ||
gl::vertex_attrib_divisor(attrib as gl::GLuint, 1); | ||
|
@@ -375,6 +381,7 @@ impl Program { | |
gl::bind_attrib_location(self.id, VertexAttribute::LayerIndex as gl::GLuint, "aLayerIndex"); | ||
gl::bind_attrib_location(self.id, VertexAttribute::ElementIndex as gl::GLuint, "aElementIndex"); | ||
gl::bind_attrib_location(self.id, VertexAttribute::UserData as gl::GLuint, "aUserData"); | ||
gl::bind_attrib_location(self.id, VertexAttribute::ZIndex as gl::GLuint, "aZIndex"); | ||
|
||
gl::bind_attrib_location(self.id, ClearAttribute::Rectangle as gl::GLuint, "aClearRectangle"); | ||
|
||
|
@@ -1127,6 +1134,21 @@ impl Device { | |
texture_id.name, | ||
0, | ||
fbo_index as gl::GLint); | ||
|
||
// TODO(gw): Share depth render buffer between FBOs to | ||
// save memory! | ||
// TODO(gw): Free these renderbuffers on exit! | ||
let renderbuffer_ids = gl::gen_renderbuffers(1); | ||
let depth_rb = renderbuffer_ids[0]; | ||
gl::bind_renderbuffer(gl::RENDERBUFFER, depth_rb); | ||
gl::renderbuffer_storage(gl::RENDERBUFFER, | ||
gl::DEPTH_COMPONENT24, | ||
texture.width as gl::GLsizei, | ||
texture.height as gl::GLsizei); | ||
gl::framebuffer_renderbuffer(gl::FRAMEBUFFER, | ||
gl::DEPTH_ATTACHMENT, | ||
gl::RENDERBUFFER, | ||
depth_rb); | ||
} | ||
} | ||
None => { | ||
|
@@ -1762,15 +1784,42 @@ impl Device { | |
} | ||
} | ||
|
||
pub fn clear_color(&self, c: [f32; 4]) { | ||
gl::clear_color(c[0], c[1], c[2], c[3]); | ||
gl::clear(gl::COLOR_BUFFER_BIT); | ||
pub fn clear_target(&self, | ||
color: Option<[f32; 4]>, | ||
depth: Option<f32>) { | ||
let mut clear_bits = 0; | ||
|
||
if let Some(color) = color { | ||
gl::clear_color(color[0], color[1], color[2], color[3]); | ||
clear_bits |= gl::COLOR_BUFFER_BIT; | ||
} | ||
|
||
if let Some(depth) = depth { | ||
gl::clear_depth(depth as f64); | ||
clear_bits |= gl::DEPTH_BUFFER_BIT; | ||
} | ||
|
||
if clear_bits != 0 { | ||
gl::clear(clear_bits); | ||
} | ||
} | ||
|
||
pub fn enable_depth(&self) { | ||
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. we could merge those 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. Yep, perhaps as a follow up we should tidy up the whole device interface? |
||
gl::enable(gl::DEPTH_TEST); | ||
} | ||
|
||
pub fn disable_depth(&self) { | ||
gl::disable(gl::DEPTH_TEST); | ||
} | ||
|
||
pub fn set_depth_func(&self, depth_func: DepthFunction) { | ||
gl::depth_func(depth_func as gl::GLuint); | ||
} | ||
|
||
pub fn enable_depth_write(&self) { | ||
gl::depth_mask(true); | ||
} | ||
|
||
pub fn disable_depth_write(&self) { | ||
gl::depth_mask(false); | ||
} | ||
|
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.
Why do we not clear empty tiles anymore?
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 think it's already guaranteed to be cleared in
draw_target
. Not sure though, at what point it became redundant/useless.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.
Since we have to clear the z-buffer now, I figure we might as well just do a clear of the color buffer as well. This also makes it perform much better on mobile / tiled GPUs which rely on a clear due to the way tiling works. We could definitely re-instate this later as an optimization if it makes sense to.