Skip to content

Commit db9a055

Browse files
gw3583gw3583
committed
Fix for race condition when caching render tasks.
When the renderer receives multiple Frames from the backend in a single render loop, it drops the old frames and just draws the most recent frame. If that old frame contained a render task to write to the texture cache, then the texture cache will never be updated. This patches tracks whether a frame has any texture cache render tasks, and whether it has been drawn before. If a frame which meets those conditions is being replaced, then do a render of that frame first, skipping the main framebuffer pass. This is not ideal since we may end up drawing offscreen render tasks that were batched but which aren't strictly necessary for the texture cache render target. However, it is very rare that the render backend is (a) producing frames faster than they are being consumed by the renderer and (b) also contain texture cache tasks.
1 parent 39abbda commit db9a055

4 files changed

Lines changed: 167 additions & 110 deletions

File tree

‎webrender/src/debug_render.rs‎

Lines changed: 57 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -261,60 +261,66 @@ impl DebugRenderer {
261261
self.add_line(p0.x, p1.y, color, p0.x, p0.y, color);
262262
}
263263

264-
pub fn render(&mut self, device: &mut Device, viewport_size: &DeviceUintSize) {
265-
device.disable_depth();
266-
device.set_blend(true);
267-
device.set_blend_mode_premultiplied_alpha();
268-
269-
let projection = Transform3D::ortho(
270-
0.0,
271-
viewport_size.width as f32,
272-
viewport_size.height as f32,
273-
0.0,
274-
ORTHO_NEAR_PLANE,
275-
ORTHO_FAR_PLANE,
276-
);
277-
278-
// Triangles
279-
if !self.tri_vertices.is_empty() {
280-
device.bind_program(&self.color_program);
281-
device.set_uniforms(&self.color_program, &projection, 0);
282-
device.bind_vao(&self.tri_vao);
283-
device.update_vao_indices(&self.tri_vao, &self.tri_indices, VertexUsageHint::Dynamic);
284-
device.update_vao_main_vertices(
285-
&self.tri_vao,
286-
&self.tri_vertices,
287-
VertexUsageHint::Dynamic,
264+
pub fn render(
265+
&mut self,
266+
device: &mut Device,
267+
viewport_size: Option<DeviceUintSize>,
268+
) {
269+
if let Some(viewport_size) = viewport_size {
270+
device.disable_depth();
271+
device.set_blend(true);
272+
device.set_blend_mode_premultiplied_alpha();
273+
274+
let projection = Transform3D::ortho(
275+
0.0,
276+
viewport_size.width as f32,
277+
viewport_size.height as f32,
278+
0.0,
279+
ORTHO_NEAR_PLANE,
280+
ORTHO_FAR_PLANE,
288281
);
289-
device.draw_triangles_u32(0, self.tri_indices.len() as i32);
290-
}
291282

292-
// Lines
293-
if !self.line_vertices.is_empty() {
294-
device.bind_program(&self.color_program);
295-
device.set_uniforms(&self.color_program, &projection, 0);
296-
device.bind_vao(&self.line_vao);
297-
device.update_vao_main_vertices(
298-
&self.line_vao,
299-
&self.line_vertices,
300-
VertexUsageHint::Dynamic,
301-
);
302-
device.draw_nonindexed_lines(0, self.line_vertices.len() as i32);
303-
}
283+
// Triangles
284+
if !self.tri_vertices.is_empty() {
285+
device.bind_program(&self.color_program);
286+
device.set_uniforms(&self.color_program, &projection, 0);
287+
device.bind_vao(&self.tri_vao);
288+
device.update_vao_indices(&self.tri_vao, &self.tri_indices, VertexUsageHint::Dynamic);
289+
device.update_vao_main_vertices(
290+
&self.tri_vao,
291+
&self.tri_vertices,
292+
VertexUsageHint::Dynamic,
293+
);
294+
device.draw_triangles_u32(0, self.tri_indices.len() as i32);
295+
}
304296

305-
// Glyph
306-
if !self.font_indices.is_empty() {
307-
device.bind_program(&self.font_program);
308-
device.set_uniforms(&self.font_program, &projection, 0);
309-
device.bind_texture(DebugSampler::Font, &self.font_texture);
310-
device.bind_vao(&self.font_vao);
311-
device.update_vao_indices(&self.font_vao, &self.font_indices, VertexUsageHint::Dynamic);
312-
device.update_vao_main_vertices(
313-
&self.font_vao,
314-
&self.font_vertices,
315-
VertexUsageHint::Dynamic,
316-
);
317-
device.draw_triangles_u32(0, self.font_indices.len() as i32);
297+
// Lines
298+
if !self.line_vertices.is_empty() {
299+
device.bind_program(&self.color_program);
300+
device.set_uniforms(&self.color_program, &projection, 0);
301+
device.bind_vao(&self.line_vao);
302+
device.update_vao_main_vertices(
303+
&self.line_vao,
304+
&self.line_vertices,
305+
VertexUsageHint::Dynamic,
306+
);
307+
device.draw_nonindexed_lines(0, self.line_vertices.len() as i32);
308+
}
309+
310+
// Glyph
311+
if !self.font_indices.is_empty() {
312+
device.bind_program(&self.font_program);
313+
device.set_uniforms(&self.font_program, &projection, 0);
314+
device.bind_texture(DebugSampler::Font, &self.font_texture);
315+
device.bind_vao(&self.font_vao);
316+
device.update_vao_indices(&self.font_vao, &self.font_indices, VertexUsageHint::Dynamic);
317+
device.update_vao_main_vertices(
318+
&self.font_vao,
319+
&self.font_vertices,
320+
VertexUsageHint::Dynamic,
321+
);
322+
device.draw_triangles_u32(0, self.font_indices.len() as i32);
323+
}
318324
}
319325

320326
self.font_indices.clear();

‎webrender/src/frame_builder.rs‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ use resource_cache::ResourceCache;
3434
use scene::{ScenePipeline, SceneProperties};
3535
use std::{mem, usize, f32};
3636
use tiling::{CompositeOps, Frame, RenderPass, RenderTargetKind};
37-
use tiling::{RenderTargetContext, ScrollbarPrimitive};
37+
use tiling::{RenderPassKind, RenderTargetContext, ScrollbarPrimitive};
3838
use util::{self, MaxRect, pack_as_float, RectHelpers, recycle_vec};
3939

4040
#[derive(Debug)]
@@ -1757,6 +1757,7 @@ impl FrameBuilder {
17571757
}
17581758

17591759
let mut deferred_resolves = vec![];
1760+
let mut has_texture_cache_tasks = false;
17601761
let use_dual_source_blending = self.config.dual_source_blending_is_enabled &&
17611762
self.config.dual_source_blending_is_supported;
17621763

@@ -1778,6 +1779,10 @@ impl FrameBuilder {
17781779
&self.clip_store,
17791780
RenderPassIndex(pass_index),
17801781
);
1782+
1783+
if let RenderPassKind::OffScreen { ref texture_cache, .. } = pass.kind {
1784+
has_texture_cache_tasks |= !texture_cache.is_empty();
1785+
}
17811786
}
17821787

17831788
let gpu_cache_updates = gpu_cache.end_frame(gpu_cache_profile);
@@ -1799,6 +1804,8 @@ impl FrameBuilder {
17991804
render_tasks,
18001805
deferred_resolves,
18011806
gpu_cache_updates: Some(gpu_cache_updates),
1807+
has_been_rendered: false,
1808+
has_texture_cache_tasks,
18021809
}
18031810
}
18041811
}

‎webrender/src/renderer.rs‎

Lines changed: 86 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -2377,7 +2377,15 @@ impl Renderer {
23772377
// Add a new document to the active set, expressed as a `Vec` in order
23782378
// to re-order based on `DocumentLayer` during rendering.
23792379
match self.active_documents.iter().position(|&(id, _)| id == document_id) {
2380-
Some(pos) => self.active_documents[pos].1 = doc,
2380+
Some(pos) => {
2381+
// If the document we are replacing must be drawn
2382+
// (in order to update the texture cache), issue
2383+
// a render just to off-screen targets.
2384+
if self.active_documents[pos].1.frame.must_be_drawn() {
2385+
self.render_impl(None).ok();
2386+
}
2387+
self.active_documents[pos].1 = doc;
2388+
}
23812389
None => self.active_documents.push((document_id, doc)),
23822390
}
23832391
}
@@ -2716,7 +2724,18 @@ impl Renderer {
27162724
/// [genframe]: ../../webrender_api/struct.DocumentApi.html#method.generate_frame
27172725
pub fn render(
27182726
&mut self,
2719-
framebuffer_size: DeviceUintSize
2727+
framebuffer_size: DeviceUintSize,
2728+
) -> Result<RendererStats, Vec<RendererError>> {
2729+
self.render_impl(Some(framebuffer_size))
2730+
}
2731+
2732+
// If framebuffer_size is None, don't render
2733+
// to the main frame buffer. This is useful
2734+
// to update texture cache render tasks but
2735+
// avoid doing a full frame render.
2736+
fn render_impl(
2737+
&mut self,
2738+
framebuffer_size: Option<DeviceUintSize>
27202739
) -> Result<RendererStats, Vec<RendererError>> {
27212740
profile_scope!("render");
27222741

@@ -2775,24 +2794,26 @@ impl Renderer {
27752794
active_documents.sort_by_key(|&(_, ref render_doc)| render_doc.frame.layer);
27762795

27772796
// don't clear the framebuffer if one of the rendered documents will overwrite it
2778-
let needs_color_clear = !active_documents
2779-
.iter()
2780-
.any(|&(_, RenderedDocument { ref frame, .. })| {
2781-
frame.background_color.is_some() &&
2782-
frame.inner_rect.origin == DeviceUintPoint::zero() &&
2783-
frame.inner_rect.size == framebuffer_size
2784-
});
2797+
if let Some(framebuffer_size) = framebuffer_size {
2798+
let needs_color_clear = !active_documents
2799+
.iter()
2800+
.any(|&(_, RenderedDocument { ref frame, .. })| {
2801+
frame.background_color.is_some() &&
2802+
frame.inner_rect.origin == DeviceUintPoint::zero() &&
2803+
frame.inner_rect.size == framebuffer_size
2804+
});
27852805

2786-
if needs_color_clear || clear_depth_value.is_some() {
2787-
let clear_color = if needs_color_clear {
2788-
self.clear_color.map(|color| color.to_array())
2789-
} else {
2790-
None
2791-
};
2792-
self.device.bind_draw_target(None, None);
2793-
self.device.enable_depth_write();
2794-
self.device.clear_target(clear_color, clear_depth_value, None);
2795-
self.device.disable_depth_write();
2806+
if needs_color_clear || clear_depth_value.is_some() {
2807+
let clear_color = if needs_color_clear {
2808+
self.clear_color.map(|color| color.to_array())
2809+
} else {
2810+
None
2811+
};
2812+
self.device.bind_draw_target(None, None);
2813+
self.device.enable_depth_write();
2814+
self.device.clear_target(clear_color, clear_depth_value, None);
2815+
self.device.disable_depth_write();
2816+
}
27962817
}
27972818

27982819
// Re-use whatever targets possible from the pool, before
@@ -2844,24 +2865,26 @@ impl Renderer {
28442865
}
28452866

28462867
if self.debug_flags.contains(DebugFlags::PROFILER_DBG) {
2847-
//TODO: take device/pixel ratio into equation?
2848-
let screen_fraction = 1.0 / framebuffer_size.to_f32().area();
2849-
self.profiler.draw_profile(
2850-
&frame_profiles,
2851-
&self.backend_profile_counters,
2852-
&self.profile_counters,
2853-
&mut profile_timers,
2854-
&profile_samplers,
2855-
screen_fraction,
2856-
&mut self.debug,
2857-
self.debug_flags.contains(DebugFlags::COMPACT_PROFILER),
2858-
);
2868+
if let Some(framebuffer_size) = framebuffer_size {
2869+
//TODO: take device/pixel ratio into equation?
2870+
let screen_fraction = 1.0 / framebuffer_size.to_f32().area();
2871+
self.profiler.draw_profile(
2872+
&frame_profiles,
2873+
&self.backend_profile_counters,
2874+
&self.profile_counters,
2875+
&mut profile_timers,
2876+
&profile_samplers,
2877+
screen_fraction,
2878+
&mut self.debug,
2879+
self.debug_flags.contains(DebugFlags::COMPACT_PROFILER),
2880+
);
2881+
}
28592882
}
28602883

28612884
self.profile_counters.reset();
28622885
self.profile_counters.frame_counter.inc();
28632886

2864-
self.debug.render(&mut self.device, &framebuffer_size);
2887+
self.debug.render(&mut self.device, framebuffer_size);
28652888
profile_timers.cpu_time.profile(|| {
28662889
let _gm = self.gpu_profile.start_marker("end frame");
28672890
self.gpu_profile.end_frame();
@@ -4170,12 +4193,13 @@ impl Renderer {
41704193
fn draw_tile_frame(
41714194
&mut self,
41724195
frame: &mut Frame,
4173-
framebuffer_size: DeviceUintSize,
4196+
framebuffer_size: Option<DeviceUintSize>,
41744197
framebuffer_depth_is_ready: bool,
41754198
frame_id: FrameId,
41764199
stats: &mut RendererStats,
41774200
) {
41784201
let _gm = self.gpu_profile.start_marker("tile frame draw");
4202+
frame.has_been_rendered = true;
41794203

41804204
if frame.passes.is_empty() {
41814205
return;
@@ -4204,30 +4228,32 @@ impl Renderer {
42044228

42054229
let (cur_alpha, cur_color) = match pass.kind {
42064230
RenderPassKind::MainFramebuffer(ref target) => {
4207-
stats.color_target_count += 1;
4208-
4209-
let clear_color = frame.background_color.map(|color| color.to_array());
4210-
let projection = Transform3D::ortho(
4211-
0.0,
4212-
framebuffer_size.width as f32,
4213-
framebuffer_size.height as f32,
4214-
0.0,
4215-
ORTHO_NEAR_PLANE,
4216-
ORTHO_FAR_PLANE,
4217-
);
4231+
if let Some(framebuffer_size) = framebuffer_size {
4232+
stats.color_target_count += 1;
42184233

4219-
self.draw_color_target(
4220-
None,
4221-
target,
4222-
frame.inner_rect,
4223-
framebuffer_size,
4224-
framebuffer_depth_is_ready,
4225-
clear_color,
4226-
&frame.render_tasks,
4227-
&projection,
4228-
frame_id,
4229-
stats,
4230-
);
4234+
let clear_color = frame.background_color.map(|color| color.to_array());
4235+
let projection = Transform3D::ortho(
4236+
0.0,
4237+
framebuffer_size.width as f32,
4238+
framebuffer_size.height as f32,
4239+
0.0,
4240+
ORTHO_NEAR_PLANE,
4241+
ORTHO_FAR_PLANE,
4242+
);
4243+
4244+
self.draw_color_target(
4245+
None,
4246+
target,
4247+
frame.inner_rect,
4248+
framebuffer_size,
4249+
framebuffer_depth_is_ready,
4250+
clear_color,
4251+
&frame.render_tasks,
4252+
&projection,
4253+
frame_id,
4254+
stats,
4255+
);
4256+
}
42314257

42324258
(None, None)
42334259
}
@@ -4314,8 +4340,10 @@ impl Renderer {
43144340
}
43154341

43164342
self.texture_resolver.end_frame(RenderPassIndex(frame.passes.len()));
4317-
self.draw_render_target_debug(framebuffer_size);
4318-
self.draw_texture_cache_debug(framebuffer_size);
4343+
if let Some(framebuffer_size) = framebuffer_size {
4344+
self.draw_render_target_debug(framebuffer_size);
4345+
self.draw_texture_cache_debug(framebuffer_size);
4346+
}
43194347
self.draw_epoch_debug();
43204348

43214349
// Garbage collect any frame outputs that weren't used this frame.

‎webrender/src/tiling.rs‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -769,6 +769,22 @@ pub struct Frame {
769769
// patch the data structures.
770770
#[cfg_attr(feature = "capture", serde(skip))]
771771
pub deferred_resolves: Vec<DeferredResolve>,
772+
773+
// True if this frame contains any render tasks
774+
// that write to the texture cache.
775+
pub has_texture_cache_tasks: bool,
776+
777+
// True if this frame has been drawn by the
778+
// renderer.
779+
pub has_been_rendered: bool,
780+
}
781+
782+
impl Frame {
783+
// This frame must be flushed if it writes to the
784+
// texture cache, and hasn't been drawn yet.
785+
pub fn must_be_drawn(&self) -> bool {
786+
self.has_texture_cache_tasks && !self.has_been_rendered
787+
}
772788
}
773789

774790
impl BlurTask {

0 commit comments

Comments
 (0)