Skip to content

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

Merged
merged 5 commits into from
Dec 23, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion replay/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,6 @@ fn main() {
debug: false,
enable_subpixel_aa: false,
clear_framebuffer: true,
clear_empty_tiles: false,
clear_color: ColorF::new(1.0, 1.0, 1.0, 1.0),
Copy link
Contributor

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?

Copy link
Member

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.

Copy link
Member Author

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.

};

Expand Down
1 change: 0 additions & 1 deletion sample/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@ fn main() {
renderer_kind: RendererKind::Native,
enable_subpixel_aa: false,
clear_framebuffer: true,
clear_empty_tiles: false,
clear_color: ColorF::new(1.0, 1.0, 1.0, 1.0),
};

Expand Down
11 changes: 9 additions & 2 deletions webrender/res/prim_shared.glsl
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -286,6 +287,7 @@ struct PrimitiveInstance {
int clip_task_index;
int layer_index;
int sub_index;
int z;
Copy link
Member

Choose a reason for hiding this comment

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

we could probably reduce the user_data to a single int in order to keep the 32 byte size

Copy link
Member Author

Choose a reason for hiding this comment

The 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;
};

Expand All @@ -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;
}
Expand Down Expand Up @@ -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) {
Expand All @@ -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;
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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);
}
Expand Down
1 change: 1 addition & 0 deletions webrender/res/ps_angle_gradient.vs.glsl
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ void main(void) {

VertexInfo vi = write_vertex(prim.local_rect,
prim.local_clip_rect,
prim.z,
prim.layer,
prim.tile);

Expand Down
4 changes: 3 additions & 1 deletion webrender/res/ps_blend.vs.glsl
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

struct Blend {
ivec4 src_id_target_id_op_amount;
int z;
};

Blend fetch_blend() {
Expand All @@ -15,6 +16,7 @@ Blend fetch_blend() {
pi.render_task_index,
pi.sub_index,
pi.user_data.y);
blend.z = pi.z;

return blend;
}
Expand All @@ -40,5 +42,5 @@ void main(void) {
vOp = blend.src_id_target_id_op_amount.z;
vAmount = blend.src_id_target_id_op_amount.w / 65535.0;

gl_Position = uTransform * vec4(local_pos, 0, 1);
gl_Position = uTransform * vec4(local_pos, blend.z, 1.0);
}
2 changes: 2 additions & 0 deletions webrender/res/ps_border.vs.glsl
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ void main(void) {
#ifdef WR_FEATURE_TRANSFORM
TransformVertexInfo vi = write_transform_vertex(segment_rect,
prim.local_clip_rect,
prim.z,
prim.layer,
prim.tile);
vLocalPos = vi.local_pos;
Expand All @@ -85,6 +86,7 @@ void main(void) {
#else
VertexInfo vi = write_vertex(segment_rect,
prim.local_clip_rect,
prim.z,
prim.layer,
prim.tile);
vLocalPos = vi.local_clamped_pos.xy;
Expand Down
1 change: 1 addition & 0 deletions webrender/res/ps_box_shadow.vs.glsl
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ void main(void) {

VertexInfo vi = write_vertex(segment_rect,
prim.local_clip_rect,
prim.z,
prim.layer,
prim.tile);

Expand Down
1 change: 1 addition & 0 deletions webrender/res/ps_cache_image.vs.glsl
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ void main(void) {

VertexInfo vi = write_vertex(prim.local_rect,
prim.local_clip_rect,
prim.z,
prim.layer,
prim.tile);

Expand Down
4 changes: 3 additions & 1 deletion webrender/res/ps_composite.vs.glsl
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

struct Composite {
ivec4 src0_src1_target_id_op;
int z;
};

Composite fetch_composite() {
Expand All @@ -14,6 +15,7 @@ Composite fetch_composite() {
composite.src0_src1_target_id_op = ivec4(pi.user_data.xy,
pi.render_task_index,
pi.sub_index);
composite.z = pi.z;

return composite;
}
Expand Down Expand Up @@ -44,5 +46,5 @@ void main(void) {

vOp = composite.src0_src1_target_id_op.w;

gl_Position = uTransform * vec4(local_pos, 0, 1);
gl_Position = uTransform * vec4(local_pos, composite.z, 1.0);
}
2 changes: 2 additions & 0 deletions webrender/res/ps_gradient.vs.glsl
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ void main(void) {
#ifdef WR_FEATURE_TRANSFORM
TransformVertexInfo vi = write_transform_vertex(segment_rect,
prim.local_clip_rect,
prim.z,
Copy link
Member

Choose a reason for hiding this comment

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

at this point, we might as well want to just pass prim there directly

Copy link
Member Author

Choose a reason for hiding this comment

The 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;
Expand All @@ -47,6 +48,7 @@ void main(void) {
#else
VertexInfo vi = write_vertex(segment_rect,
prim.local_clip_rect,
prim.z,
prim.layer,
prim.tile);

Expand Down
2 changes: 2 additions & 0 deletions webrender/res/ps_image.vs.glsl
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,15 @@ void main(void) {
#ifdef WR_FEATURE_TRANSFORM
TransformVertexInfo vi = write_transform_vertex(prim.local_rect,
prim.local_clip_rect,
prim.z,
prim.layer,
prim.tile);
vLocalRect = vi.clipped_local_rect;
vLocalPos = vi.local_pos;
#else
VertexInfo vi = write_vertex(prim.local_rect,
prim.local_clip_rect,
prim.z,
prim.layer,
prim.tile);
vLocalPos = vi.local_clamped_pos - vi.local_rect.p0;
Expand Down
2 changes: 2 additions & 0 deletions webrender/res/ps_rectangle.vs.glsl
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,15 @@ void main(void) {
#ifdef WR_FEATURE_TRANSFORM
TransformVertexInfo vi = write_transform_vertex(prim.local_rect,
prim.local_clip_rect,
prim.z,
prim.layer,
prim.tile);
vLocalRect = vi.clipped_local_rect;
vLocalPos = vi.local_pos;
#else
VertexInfo vi = write_vertex(prim.local_rect,
prim.local_clip_rect,
prim.z,
prim.layer,
prim.tile);
#endif
Expand Down
2 changes: 2 additions & 0 deletions webrender/res/ps_text_run.vs.glsl
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ void main(void) {
#ifdef WR_FEATURE_TRANSFORM
TransformVertexInfo vi = write_transform_vertex(local_rect,
prim.local_clip_rect,
prim.z,
prim.layer,
prim.tile);
vLocalRect = vi.clipped_local_rect;
Expand All @@ -22,6 +23,7 @@ void main(void) {
#else
VertexInfo vi = write_vertex(local_rect,
prim.local_clip_rect,
prim.z,
prim.layer,
prim.tile);
vec2 f = (vi.local_clamped_pos - vi.local_rect.p0) / (vi.local_rect.p1 - vi.local_rect.p0);
Expand Down
2 changes: 2 additions & 0 deletions webrender/res/ps_yuv_image.vs.glsl
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,15 @@ void main(void) {
#ifdef WR_FEATURE_TRANSFORM
TransformVertexInfo vi = write_transform_vertex(prim.local_rect,
prim.local_clip_rect,
prim.z,
prim.layer,
prim.tile);
vLocalRect = vi.clipped_local_rect;
vLocalPos = vi.local_pos;
#else
VertexInfo vi = write_vertex(prim.local_rect,
prim.local_clip_rect,
prim.z,
prim.layer,
prim.tile);
vLocalPos = vi.local_clamped_pos - vi.local_rect.p0;
Expand Down
55 changes: 52 additions & 3 deletions webrender/src/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,11 @@ lazy_static! {
};
}

#[repr(u32)]
pub enum DepthFunction {
Less = gl::LESS,
}

#[derive(Copy, Clone, Debug, PartialEq)]
pub enum TextureTarget {
Default,
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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");

Expand Down Expand Up @@ -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 => {
Expand Down Expand Up @@ -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) {
Copy link
Member

Choose a reason for hiding this comment

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

we could merge those enable_*/disable_* for tinier interface with the device

Copy link
Member Author

Choose a reason for hiding this comment

The 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);
}
Expand Down
9 changes: 9 additions & 0 deletions webrender/src/frame.rs
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,16 @@ impl Frame {
self.layers.insert(root_fixed_layer_id, layer.clone());
self.layers.insert(root_scroll_layer_id, layer);

let background_color = root_pipeline.background_color.and_then(|color| {
if color.a > 0.0 {
Some(color)
} else {
None
}
});

let mut frame_builder = FrameBuilder::new(root_pipeline.viewport_size,
background_color,
self.debug,
self.frame_builder_config);

Expand Down
1 change: 1 addition & 0 deletions webrender/src/internal_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ pub enum VertexAttribute {
LayerIndex,
ElementIndex,
UserData,
ZIndex,
}

#[derive(Clone, Copy, Debug)]
Expand Down
Loading