Auto merge of #24386 - jdm:no-preserve-drawing-buffer, r=nox

webgl: Clear the drawing buffer when preserveDrawingBuffer is false.

This adds an explicit clear operation to a webgl canvas when the preserveDrawingBuffer attribute is false when creating the context. Because we're not using double buffering this may end up making some demos worse (ie. a clear operation at a random time while drawing a frame), but this problem is expected to go away when #24256 is fixed and we move to a multi-buffered frame setup. This change is important at this time because so many babylon.js demos rely on the engine clearing the frame, per the specification.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #21132
- [x] There are tests for these changes

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/24386)
<!-- Reviewable:end -->

8 files changed, 156 insertions, 104 deletions

diff --git a/components/canvas/gl_context.rs b/components/canvas/gl_context.rs
index 886e6dea39a..7e6c4c135a2 100644
--- a/components/canvas/gl_context.rs
+++ b/components/canvas/gl_context.rs
@@ -179,6 +179,17 @@ impl GLContextWrapper {
         }
     }
 
+    pub fn framebuffer(&self) -> gl::GLuint {
+        match *self {
+            GLContextWrapper::Native(ref ctx) => {
+                ctx.borrow_draw_buffer().unwrap().get_framebuffer()
+            },
+            GLContextWrapper::OSMesa(ref ctx) => {
+                ctx.borrow_draw_buffer().unwrap().get_framebuffer()
+            },
+        }
+    }
+
     pub fn get_info(&self) -> (Size2D<i32>, u32, GLLimits, GLFormats) {
         match *self {
             GLContextWrapper::Native(ref ctx) => {
diff --git a/components/canvas/webgl_thread.rs b/components/canvas/webgl_thread.rs
index f8e63cd1fa3..a2e95955753 100644
--- a/components/canvas/webgl_thread.rs
+++ b/components/canvas/webgl_thread.rs
@@ -239,6 +239,12 @@ impl WebGLThread {
                 let result = self.create_webgl_context(version, size, attributes);
                 result_sender
                     .send(result.map(|(id, limits, share_mode, framebuffer_format)| {
+                        let image_key = self
+                            .cached_context_info
+                            .get_mut(&id)
+                            .expect("Where's the cached context info?")
+                            .image_key;
+
                         let data = Self::make_current_if_needed(
                             id,
                             &self.contexts,
@@ -277,6 +283,7 @@ impl WebGLThread {
                             glsl_version,
                             api_type,
                             framebuffer_format,
+                            image_key,
                         }
                     }))
                     .unwrap();
@@ -299,12 +306,12 @@ impl WebGLThread {
             WebGLMsg::Unlock(ctx_id) => {
                 self.handle_unlock(ctx_id);
             },
-            WebGLMsg::UpdateWebRenderImage(ctx_id, sender) => {
-                self.handle_update_wr_image(ctx_id, sender);
-            },
             WebGLMsg::DOMToTextureCommand(command) => {
                 self.handle_dom_to_texture(command);
             },
+            WebGLMsg::SwapBuffers(context_ids, sender) => {
+                self.handle_swap_buffers(context_ids, sender);
+            },
             WebGLMsg::Exit => {
                 return true;
             },
@@ -326,6 +333,11 @@ impl WebGLThread {
             &mut self.bound_context_id,
         );
         if let Some(data) = data {
+            let info = self.cached_context_info.get_mut(&context_id).unwrap();
+            if info.clear_required {
+                info.clear_required = false;
+                Self::clear_drawing_buffer(data);
+            }
             data.ctx.apply_command(
                 command,
                 data.use_apple_vertex_arrays,
@@ -388,6 +400,7 @@ impl WebGLThread {
     pub(crate) fn handle_unlock(&mut self, context_id: WebGLContextId) {
         let info = self.cached_context_info.get_mut(&context_id).unwrap();
         info.render_state = ContextRenderState::Unlocked;
+
         if let Some(gl_sync) = info.gl_sync.take() {
             let data = Self::make_current_if_needed(
                 context_id,
@@ -401,6 +414,36 @@ impl WebGLThread {
         }
     }
 
+    #[allow(unsafe_code)]
+    fn clear_drawing_buffer(data: &mut GLContextData) {
+        trace!("clearing GL framebuffer");
+
+        // Ensure we're clearing the default framebuffer.
+        let mut fb = [0];
+        unsafe {
+            data.ctx
+                .gl()
+                .get_integer_v(gl::FRAMEBUFFER_BINDING, &mut fb);
+        }
+        data.ctx
+            .gl()
+            .bind_framebuffer(gl::FRAMEBUFFER, data.ctx.framebuffer());
+
+        data.ctx.gl().clear_color(0., 0., 0., 0.);
+        data.ctx.gl().clear_depth(1.0);
+        data.ctx.gl().clear_stencil(0);
+        data.ctx
+            .gl()
+            .clear(gl::COLOR_BUFFER_BIT | gl::DEPTH_BUFFER_BIT | gl::STENCIL_BUFFER_BIT);
+
+        let (r, g, b, a) = data.state.clear_color;
+        data.ctx.gl().clear_color(r, g, b, a);
+        data.ctx.gl().clear_depth(data.state.depth_clear_value);
+        data.ctx.gl().clear_stencil(data.state.stencil_clear_value);
+
+        data.ctx.gl().bind_framebuffer(gl::FRAMEBUFFER, fb[0] as _);
+    }
+
     /// Creates a new WebGLContext
     fn create_webgl_context(
         &mut self,
@@ -412,6 +455,8 @@ impl WebGLThread {
         // Clear it to ensure that  make_current() is called in subsequent commands.
         self.bound_context_id = None;
 
+        let preserve_drawing_buffer = attributes.preserve_drawing_buffer;
+
         // First try to create a shared context for the best performance.
         // Fallback to readback mode if the shared context creation fails.
         let (ctx, share_mode) = self
@@ -446,16 +491,33 @@ impl WebGLThread {
             },
         );
 
+        let image_key = match share_mode {
+            WebGLContextShareMode::Readback => Self::create_wr_readback_image(
+                &self.webrender_api,
+                size,
+                attributes.alpha,
+                vec![0; 4 * size.width as usize * size.height as usize],
+            ),
+            WebGLContextShareMode::SharedTexture => Self::create_wr_external_image(
+                &self.webrender_api,
+                size.to_i32(),
+                attributes.alpha,
+                id,
+            ),
+        };
+
         self.cached_context_info.insert(
             id,
             WebGLContextInfo {
                 texture_id,
                 size,
                 alpha: attributes.alpha,
-                image_key: None,
+                image_key: image_key,
                 share_mode,
                 gl_sync: None,
                 render_state: ContextRenderState::Unlocked,
+                preserve_drawing_buffer,
+                clear_required: false,
             },
         );
 
@@ -497,7 +559,7 @@ impl WebGLThread {
                 // Readback mode already updates the image every frame to send the raw pixels.
                 // See `handle_update_wr_image`.
                 match (info.image_key, info.share_mode) {
-                    (Some(image_key), WebGLContextShareMode::SharedTexture) => {
+                    (image_key, WebGLContextShareMode::SharedTexture) => {
                         Self::update_wr_external_image(
                             &self.webrender_api,
                             info.size,
@@ -523,9 +585,7 @@ impl WebGLThread {
         if let Some(info) = self.cached_context_info.remove(&context_id) {
             let mut txn = webrender_api::Transaction::new();
 
-            if let Some(image_key) = info.image_key {
-                txn.delete_image(image_key);
-            }
+            txn.delete_image(info.image_key);
 
             self.webrender_api.update_resources(txn.resource_updates)
         }
@@ -541,62 +601,33 @@ impl WebGLThread {
         self.bound_context_id = None;
     }
 
-    /// Handles the creation/update of webrender_api::ImageKeys for a specific WebGLContext.
-    /// This method is invoked from a UpdateWebRenderImage message sent by the layout thread.
-    /// If SharedTexture is used the UpdateWebRenderImage message is sent only after a WebGLContext creation.
-    /// If Readback is used UpdateWebRenderImage message is sent always on each layout iteration in order to
-    /// submit the updated raw pixels.
-    fn handle_update_wr_image(
+    fn handle_swap_buffers(
         &mut self,
-        context_id: WebGLContextId,
-        sender: WebGLSender<webrender_api::ImageKey>,
+        context_ids: Vec<WebGLContextId>,
+        completed_sender: WebGLSender<()>,
     ) {
-        let info = self.cached_context_info.get_mut(&context_id).unwrap();
-        let webrender_api = &self.webrender_api;
-
-        let image_key = match info.share_mode {
-            WebGLContextShareMode::SharedTexture => {
-                let size = info.size;
-                let alpha = info.alpha;
-                // Reuse existing ImageKey or generate a new one.
-                // When using a shared texture ImageKeys are only generated after a WebGLContext creation.
-                *info.image_key.get_or_insert_with(|| {
-                    Self::create_wr_external_image(webrender_api, size, alpha, context_id)
-                })
-            },
-            WebGLContextShareMode::Readback => {
+        for context_id in context_ids {
+            let info = self.cached_context_info.get_mut(&context_id).unwrap();
+            let webrender_api = &self.webrender_api;
+
+            if let WebGLContextShareMode::Readback = info.share_mode {
                 let pixels = Self::raw_pixels(&self.contexts[&context_id].ctx, info.size);
-                match info.image_key.clone() {
-                    Some(image_key) => {
-                        // ImageKey was already created, but WR Images must
-                        // be updated every frame in readback mode to send the new raw pixels.
-                        Self::update_wr_readback_image(
-                            webrender_api,
-                            info.size,
-                            info.alpha,
-                            image_key,
-                            pixels,
-                        );
+                // WR Images must be updated every frame in readback mode to send the new raw pixels.
+                Self::update_wr_readback_image(
+                    webrender_api,
+                    info.size,
+                    info.alpha,
+                    info.image_key,
+                    pixels,
+                );
+            }
 
-                        image_key
-                    },
-                    None => {
-                        // Generate a new ImageKey for Readback mode.
-                        let image_key = Self::create_wr_readback_image(
-                            webrender_api,
-                            info.size,
-                            info.alpha,
-                            pixels,
-                        );
-                        info.image_key = Some(image_key);
-                        image_key
-                    },
-                }
-            },
-        };
+            if !info.preserve_drawing_buffer {
+                info.clear_required = true;
+            }
+        }
 
-        // Send the ImageKey to the Layout thread.
-        sender.send(image_key).unwrap();
+        let _ = completed_sender.send(());
     }
 
     fn handle_dom_to_texture(&mut self, command: DOMToTextureCommand) {
@@ -875,13 +906,17 @@ struct WebGLContextInfo {
     /// True if the WebGLContext uses an alpha channel.
     alpha: bool,
     /// Currently used WebRender image key.
-    image_key: Option<webrender_api::ImageKey>,
+    image_key: webrender_api::ImageKey,
     /// The sharing mode used to send the image to WebRender.
     share_mode: WebGLContextShareMode,
     /// GLSync Object used for a correct synchronization with Webrender external image callbacks.
     gl_sync: Option<gl::GLsync>,
     /// The status of this context with respect to external consumers.
     render_state: ContextRenderState,
+    /// Should the drawing buffer be preserved between frames?
+    preserve_drawing_buffer: bool,
+    /// Does the canvas need to be cleared before executing further WebGL commands?
+    clear_required: bool,
 }
 
 /// Data about the linked DOM<->WebGLTexture elements.
diff --git a/components/canvas_traits/webgl.rs b/components/canvas_traits/webgl.rs
index 16174a15525..62a04cac7fc 100644
--- a/components/canvas_traits/webgl.rs
+++ b/components/canvas_traits/webgl.rs
@@ -64,10 +64,10 @@ pub enum WebGLMsg {
     /// The WR unlocks a context when it finished reading the shared texture contents.
     /// Unlock messages are always sent after a Lock message.
     Unlock(WebGLContextId),
-    /// Creates or updates the image keys required for WebRender.
-    UpdateWebRenderImage(WebGLContextId, WebGLSender<ImageKey>),
     /// Commands used for the DOMToTexture feature.
     DOMToTextureCommand(DOMToTextureCommand),
+    /// Performs a buffer swap.
+    SwapBuffers(Vec<WebGLContextId>, WebGLSender<()>),
     /// Frees all resources and closes the thread.
     Exit,
 }
@@ -93,6 +93,8 @@ pub struct WebGLCreateContextResult {
     pub api_type: GlType,
     /// The format for creating new offscreen framebuffers for this context.
     pub framebuffer_format: GLFormats,
+    /// The WebRender image key.
+    pub image_key: ImageKey,
 }
 
 #[derive(Clone, Copy, Debug, Deserialize, MallocSizeOf, Serialize)]
@@ -174,12 +176,6 @@ impl WebGLMsgSender {
         self.sender.send(WebGLMsg::RemoveContext(self.ctx_id))
     }
 
-    #[inline]
-    pub fn send_update_wr_image(&self, sender: WebGLSender<ImageKey>) -> WebGLSendResult {
-        self.sender
-            .send(WebGLMsg::UpdateWebRenderImage(self.ctx_id, sender))
-    }
-
     pub fn send_dom_to_texture(&self, command: DOMToTextureCommand) -> WebGLSendResult {
         self.sender.send(WebGLMsg::DOMToTextureCommand(command))
     }
diff --git a/components/script/dom/bindings/trace.rs b/components/script/dom/bindings/trace.rs
index 5fbc9629f68..e86c96e0a9e 100644
--- a/components/script/dom/bindings/trace.rs
+++ b/components/script/dom/bindings/trace.rs
@@ -48,7 +48,9 @@ use canvas_traits::canvas::{CompositionOrBlending, LineCapStyle, LineJoinStyle,
 use canvas_traits::webgl::WebGLVertexArrayId;
 use canvas_traits::webgl::{ActiveAttribInfo, ActiveUniformInfo, GlType, TexDataType, TexFormat};
 use canvas_traits::webgl::{GLFormats, GLLimits, WebGLQueryId, WebGLSamplerId};
-use canvas_traits::webgl::{WebGLBufferId, WebGLChan, WebGLContextShareMode, WebGLError};
+use canvas_traits::webgl::{
+    WebGLBufferId, WebGLChan, WebGLContextId, WebGLContextShareMode, WebGLError,
+};
 use canvas_traits::webgl::{WebGLFramebufferId, WebGLMsgSender, WebGLPipeline, WebGLProgramId};
 use canvas_traits::webgl::{WebGLReceiver, WebGLRenderbufferId, WebGLSLVersion, WebGLSender};
 use canvas_traits::webgl::{WebGLShaderId, WebGLSyncId, WebGLTextureId, WebGLVersion};
@@ -519,6 +521,7 @@ unsafe_no_jsmanaged_fields!(Rect<f32>);
 unsafe_no_jsmanaged_fields!(CascadeData);
 unsafe_no_jsmanaged_fields!(WindowGLContext);
 unsafe_no_jsmanaged_fields!(Frame);
+unsafe_no_jsmanaged_fields!(WebGLContextId);
 
 unsafe impl<'a> JSTraceable for &'a str {
     #[inline]
diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs
index da44169ce59..452212a2b3c 100644
--- a/components/script/dom/document.rs
+++ b/components/script/dom/document.rs
@@ -109,6 +109,7 @@ use crate::stylesheet_set::StylesheetSetRef;
 use crate::task::TaskBox;
 use crate::task_source::{TaskSource, TaskSourceName};
 use crate::timers::OneshotTimerCallback;
+use canvas_traits::webgl::{self, WebGLContextId, WebGLMsg};
 use cookie::Cookie;
 use devtools_traits::ScriptToDevtoolsControlMsg;
 use dom_struct::dom_struct;
@@ -395,6 +396,8 @@ pub struct Document {
     /// where `id` needs to match any of the registered ShadowRoots
     /// hosting the media controls UI.
     media_controls: DomRefCell<HashMap<String, Dom<ShadowRoot>>>,
+    /// List of all WebGL context IDs that need flushing.
+    dirty_webgl_contexts: DomRefCell<HashSet<WebGLContextId>>,
 }
 
 #[derive(JSTraceable, MallocSizeOf)]
@@ -2490,6 +2493,24 @@ impl Document {
             debug_assert!(false, "Trying to unregister unknown media controls");
         }
     }
+
+    pub fn add_dirty_canvas(&self, context_id: WebGLContextId) {
+        self.dirty_webgl_contexts.borrow_mut().insert(context_id);
+    }
+
+    pub fn flush_dirty_canvases(&self) {
+        let dirty_context_ids: Vec<_> = self.dirty_webgl_contexts.borrow_mut().drain().collect();
+        if dirty_context_ids.is_empty() {
+            return;
+        }
+        let (sender, receiver) = webgl::webgl_channel().unwrap();
+        self.window
+            .webgl_chan()
+            .expect("Where's the WebGL channel?")
+            .send(WebGLMsg::SwapBuffers(dirty_context_ids, sender))
+            .unwrap();
+        receiver.recv().unwrap();
+    }
 }
 
 #[derive(MallocSizeOf, PartialEq)]
@@ -2784,6 +2805,7 @@ impl Document {
             shadow_roots: DomRefCell::new(HashSet::new()),
             shadow_roots_styles_changed: Cell::new(false),
             media_controls: DomRefCell::new(HashMap::new()),
+            dirty_webgl_contexts: DomRefCell::new(HashSet::new()),
         }
     }
 
diff --git a/components/script/dom/webglrenderingcontext.rs b/components/script/dom/webglrenderingcontext.rs
index 06141d5b5e7..f19664ad0ad 100644
--- a/components/script/dom/webglrenderingcontext.rs
+++ b/components/script/dom/webglrenderingcontext.rs
@@ -81,7 +81,6 @@ use std::cell::Cell;
 use std::cmp;
 use std::ptr::{self, NonNull};
 use std::rc::Rc;
-use webrender_api::ImageKey;
 
 // From the GLES 2.0.25 spec, page 85:
 //
@@ -130,7 +129,7 @@ pub struct WebGLRenderingContext {
     #[ignore_malloc_size_of = "Channels are hard"]
     webgl_sender: WebGLMessageSender,
     #[ignore_malloc_size_of = "Defined in webrender"]
-    webrender_image: Cell<Option<webrender_api::ImageKey>>,
+    webrender_image: webrender_api::ImageKey,
     share_mode: WebGLContextShareMode,
     webgl_version: WebGLVersion,
     glsl_version: WebGLSLVersion,
@@ -195,7 +194,7 @@ impl WebGLRenderingContext {
                     ctx_data.sender,
                     window.get_event_loop_waker(),
                 ),
-                webrender_image: Cell::new(None),
+                webrender_image: ctx_data.image_key,
                 share_mode: ctx_data.share_mode,
                 webgl_version,
                 glsl_version: ctx_data.glsl_version,
@@ -453,13 +452,17 @@ impl WebGLRenderingContext {
     }
 
     fn mark_as_dirty(&self) {
-        // If we don't have a bound framebuffer, then don't mark the canvas
-        // as dirty.
-        if self.bound_framebuffer.get().is_none() {
-            self.canvas
-                .upcast::<Node>()
-                .dirty(NodeDamage::OtherNodeDamage);
+        // If we have a bound framebuffer, then don't mark the canvas as dirty.
+        if self.bound_framebuffer.get().is_some() {
+            return;
         }
+
+        self.canvas
+            .upcast::<Node>()
+            .dirty(NodeDamage::OtherNodeDamage);
+
+        let document = document_from_node(&*self.canvas);
+        document.add_dirty_canvas(self.context_id());
     }
 
     fn vertex_attrib(&self, indx: u32, x: f32, y: f32, z: f32, w: f32) {
@@ -808,26 +811,7 @@ impl WebGLRenderingContext {
     }
 
     pub fn layout_handle(&self) -> webrender_api::ImageKey {
-        match self.share_mode {
-            WebGLContextShareMode::SharedTexture => {
-                // WR using ExternalTexture requires a single update message.
-                self.webrender_image.get().unwrap_or_else(|| {
-                    let (sender, receiver) = webgl_channel().unwrap();
-                    self.webgl_sender.send_update_wr_image(sender).unwrap();
-                    let image_key = receiver.recv().unwrap();
-                    self.webrender_image.set(Some(image_key));
-
-                    image_key
-                })
-            },
-            WebGLContextShareMode::Readback => {
-                // WR using Readback requires to update WR image every frame
-                // in order to send the new raw pixels.
-                let (sender, receiver) = webgl_channel().unwrap();
-                self.webgl_sender.send_update_wr_image(sender).unwrap();
-                receiver.recv().unwrap()
-            },
-        }
+        self.webrender_image
     }
 
     // https://www.khronos.org/registry/webgl/extensions/ANGLE_instanced_arrays/
@@ -4438,10 +4422,6 @@ impl WebGLMessageSender {
         self.wake_after_send(|| self.sender.send_remove())
     }
 
-    pub fn send_update_wr_image(&self, sender: WebGLSender<ImageKey>) -> WebGLSendResult {
-        self.wake_after_send(|| self.sender.send_update_wr_image(sender))
-    }
-
     pub fn send_dom_to_texture(&self, command: DOMToTextureCommand) -> WebGLSendResult {
         self.wake_after_send(|| self.sender.send_dom_to_texture(command))
     }
diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs
index 8e139158427..565308ff36f 100644
--- a/components/script/dom/window.rs
+++ b/components/script/dom/window.rs
@@ -1520,6 +1520,13 @@ impl Window {
 
         let stylesheets_changed = document.flush_stylesheets_for_reflow();
 
+        // If this reflow is for display, ensure webgl canvases are composited with
+        // up-to-date contents.
+        match reflow_goal {
+            ReflowGoal::Full => document.flush_dirty_canvases(),
+            ReflowGoal::TickAnimations | ReflowGoal::LayoutQuery(..) => {},
+        }
+
         // Send new document and relevant styles to layout.
         let needs_display = reflow_goal.needs_display();
         let reflow = ScriptReflow {
diff --git a/tests/wpt/webgl/meta/conformance/context/context-creation.html.ini b/tests/wpt/webgl/meta/conformance/context/context-creation.html.ini
deleted file mode 100644
index d926d27cf37..00000000000
--- a/tests/wpt/webgl/meta/conformance/context/context-creation.html.ini
+++ /dev/null
@@ -1,2 +0,0 @@
-[context-creation.html]
-  expected: TIMEOUT