From a208990a8463694776f8edf8918faa3dd70c1e84 Mon Sep 17 00:00:00 2001 From: Jamie Nicol Date: Fri, 9 Apr 2021 13:58:33 +0000 Subject: [PATCH] Bug 1638593 - Use shader to clear alpha targets on Mali-Txxx devices. r=gw On several Mali-Txxx devices we have observed crashes during draw calls when rendering to an alpha target. The crashes were all occuring when rendering with either the cs_blur_ALPHA_TARGET shader or the cs_clip_box_shadow shader. What both these shaders have in common is that immediately prior to rendering with them we clear the render target using glClear. The crash appears to be caused by a timing related issue between clearing an alpha target with glClear and subsequently rendering to it with a draw call. Adding enough logging inbetween those two steps prevents the crash, as does adding a glFlush. This patch makes it so that we use the ps_clear shader to clear the regions of the alpha target on affected devices, rather than glClear. This avoids the crash altogether. Differential Revision: https://phabricator.services.mozilla.com/D111279 --- gfx/wr/webrender/src/device/gl.rs | 10 +++ gfx/wr/webrender/src/renderer/mod.rs | 91 ++++++++++++++++++++++------ 2 files changed, 81 insertions(+), 20 deletions(-) diff --git a/gfx/wr/webrender/src/device/gl.rs b/gfx/wr/webrender/src/device/gl.rs index 2bfe3873292e..26686ca59283 100644 --- a/gfx/wr/webrender/src/device/gl.rs +++ b/gfx/wr/webrender/src/device/gl.rs @@ -974,6 +974,9 @@ pub struct Capabilities { /// Whether to enforce that texture uploads be batched regardless of what /// the pref says. pub requires_batched_texture_uploads: Option, + /// Whether we are able to ue glClear to clear regions of an alpha render target. + /// If false, we must use a shader to clear instead. + pub supports_alpha_target_clears: bool, /// Whether the driver can reliably upload data to R8 format textures. pub supports_r8_texture_upload: bool, /// Whether clip-masking is supported natively by the GL implementation @@ -1696,6 +1699,12 @@ impl Device { requires_batched_texture_uploads = Some(true); } + // On Mali-Txxx devices we have observed crashes during draw calls when rendering + // to an alpha target immediately after using glClear to clear regions of it. + // Using a shader to clear the regions avoids the crash. See bug 1638593. + let is_mali_t = renderer_name.starts_with("Mali-T"); + let supports_alpha_target_clears = !is_mali_t; + // On Linux we we have seen uploads to R8 format textures result in // corruption on some AMD cards. // See https://bugzilla.mozilla.org/show_bug.cgi?id=1687554#c13 @@ -1733,6 +1742,7 @@ impl Device { supports_render_target_partial_update, supports_shader_storage_object, requires_batched_texture_uploads, + supports_alpha_target_clears, supports_r8_texture_upload, uses_native_clip_mask, uses_native_antialiasing, diff --git a/gfx/wr/webrender/src/renderer/mod.rs b/gfx/wr/webrender/src/renderer/mod.rs index 7e7afbe7cb55..3acd3f1432cd 100644 --- a/gfx/wr/webrender/src/renderer/mod.rs +++ b/gfx/wr/webrender/src/renderer/mod.rs @@ -760,6 +760,7 @@ pub struct Renderer { enable_clear_scissor: bool, enable_advanced_blend_barriers: bool, clear_caches_with_quads: bool, + clear_alpha_targets_with_quads: bool, debug: debug::LazyInitializedDebugRenderer, debug_flags: DebugFlags, @@ -1108,6 +1109,8 @@ impl Renderer { let backend_notifier = notifier.clone(); + let clear_alpha_targets_with_quads = !device.get_capabilities().supports_alpha_target_clears; + let prefer_subpixel_aa = options.force_subpixel_aa || (options.enable_subpixel_aa && use_dual_source_blending); let default_font_render_mode = match (options.enable_aa, prefer_subpixel_aa) { (true, true) => FontRenderMode::Subpixel, @@ -1325,6 +1328,7 @@ impl Renderer { enable_clear_scissor: options.enable_clear_scissor, enable_advanced_blend_barriers: !ext_blend_equation_advanced_coherent, clear_caches_with_quads: options.clear_caches_with_quads, + clear_alpha_targets_with_quads, last_time: 0, gpu_profiler, vaos, @@ -3834,30 +3838,77 @@ impl Renderer { self.device.disable_depth_write(); self.set_blend(false, FramebufferKind::Other); - // TODO(gw): Applying a scissor rect and minimal clear here - // is a very large performance win on the Intel and nVidia - // GPUs that I have tested with. It's possible it may be a - // performance penalty on other GPU types - we should test this - // and consider different code paths. - let zero_color = [0.0, 0.0, 0.0, 0.0]; - for &task_id in &target.zero_clears { - let rect = render_tasks[task_id].get_target_rect(); - self.device.clear_target( - Some(zero_color), - None, - Some(draw_target.to_framebuffer_rect(rect)), - ); - } - let one_color = [1.0, 1.0, 1.0, 1.0]; - for &task_id in &target.one_clears { - let rect = render_tasks[task_id].get_target_rect(); - self.device.clear_target( - Some(one_color), + + // On some Mali-T devices we have observed crashes in subsequent draw calls + // immediately after clearing the alpha render target regions with glClear(). + // Using the shader to clear the regions avoids the crash. See bug 1638593. + if self.clear_alpha_targets_with_quads + && !(target.zero_clears.is_empty() && target.one_clears.is_empty()) + { + let zeroes = target.zero_clears + .iter() + .map(|task_id| { + let rect = render_tasks[*task_id].get_target_rect().to_f32(); + ClearInstance { + rect: [ + rect.origin.x, rect.origin.y, + rect.size.width, rect.size.height, + ], + color: zero_color, + } + }); + + let ones = target.one_clears + .iter() + .map(|task_id| { + let rect = render_tasks[*task_id].get_target_rect().to_f32(); + ClearInstance { + rect: [ + rect.origin.x, rect.origin.y, + rect.size.width, rect.size.height, + ], + color: one_color, + } + }); + + let instances = zeroes.chain(ones).collect::>(); + self.shaders.borrow_mut().ps_clear.bind( + &mut self.device, + &projection, None, - Some(draw_target.to_framebuffer_rect(rect)), + &mut self.renderer_errors, ); + self.draw_instanced_batch( + &instances, + VertexArrayKind::Clear, + &BatchTextures::empty(), + stats, + ); + } else { + // TODO(gw): Applying a scissor rect and minimal clear here + // is a very large performance win on the Intel and nVidia + // GPUs that I have tested with. It's possible it may be a + // performance penalty on other GPU types - we should test this + // and consider different code paths. + for &task_id in &target.zero_clears { + let rect = render_tasks[task_id].get_target_rect(); + self.device.clear_target( + Some(zero_color), + None, + Some(draw_target.to_framebuffer_rect(rect)), + ); + } + + for &task_id in &target.one_clears { + let rect = render_tasks[task_id].get_target_rect(); + self.device.clear_target( + Some(one_color), + None, + Some(draw_target.to_framebuffer_rect(rect)), + ); + } } }