Repository: zng-ui/zng (language: rust)

Issue:
Frame image capture fails in extend_view example
Following these steps:

1 - Run `extend_view`.
2 - Use the Inspector to save a screenshot.

I saw this happen:

Image mostly black, just one tile visible.

I expected this to happen:

A normal screenshot.


Discussion / hints:

Fix patch:
diff --git a/README.md b/README.md
index cf28ed931..b9ab58a6a 100644
--- a/README.md
+++ b/README.md
@@ -220,7 +220,7 @@ The task runner can be used to run the examples in [`./examples`] folder, for ex
 See [`./examples/README.md`] for a list of examples with description and screenshots.
 
 [`./examples`]: https://github.com/zng-ui/zng/tree/master/examples
-[`./examples/README.md`]: https://github.com/zng-ui/zng/tree/master/examples
+[`./examples/README.md`]: https://github.com/zng-ui/zng/tree/master/examples#readme
 
 ### `cargo do install`
 
diff --git a/zng-app/src/view_process.rs b/zng-app/src/view_process.rs
index 50c2dc531..2bfec75b6 100644
--- a/zng-app/src/view_process.rs
+++ b/zng-app/src/view_process.rs
@@ -742,7 +742,9 @@ impl ViewWindow {
     }
 
     /// Sets if the headed window is in *capture-mode*. If `true` the resources used to capture
-    /// a screenshot are kept in memory to be reused in the next screenshot capture.
+    /// a screenshot may be kept in memory to be reused in the next screenshot capture.
+    ///
+    /// Note that capture must still be requested in each frame request.
     pub fn set_capture_mode(&self, enabled: bool) -> Result<()> {
         self.0.call(|id, p| p.set_capture_mode(id, enabled))
     }
diff --git a/zng-view-api/src/lib.rs b/zng-view-api/src/lib.rs
index 488ff7f63..289fcec4d 100644
--- a/zng-view-api/src/lib.rs
+++ b/zng-view-api/src/lib.rs
@@ -436,9 +436,9 @@ declare_api! {
     pub fn delete_font(&mut self, id: WindowId, font_id: FontId);
 
     /// Sets if the headed window is in *capture-mode*. If `true` the resources used to capture
-    /// a screenshot are kept in memory to be reused in the next screenshot capture.
+    /// a screenshot may be kept in memory to be reused in the next screenshot capture.
     ///
-    /// Headless surfaces are always capture-mode enabled.
+    /// Note that capture must still be requested in each frame request.
     pub fn set_capture_mode(&mut self, id: WindowId, enable: bool);
 
     /// Create a new image resource from the current rendered frame.
diff --git a/zng-view/src/image_cache.rs b/zng-view/src/image_cache.rs
index 1aab473df..804a86de5 100644
--- a/zng-view/src/image_cache.rs
+++ b/zng-view/src/image_cache.rs
@@ -1200,10 +1200,7 @@ pub(crate) use external::{ImageUseMap, WrImageCache};
 mod capture {
     use std::sync::Arc;
 
-    use webrender::{
-        api::{ImageDescriptor, ImageDescriptorFlags, ImageFormat},
-        Renderer,
-    };
+    use webrender::api::{ImageDescriptor, ImageDescriptorFlags, ImageFormat};
     use zng_txt::formatx;
     use zng_unit::{Factor, PxRect};
     use zng_view_api::{
@@ -1215,7 +1212,6 @@ mod capture {
 
     use crate::{
         image_cache::{Image, ImageData},
-        px_wr::{PxToWr as _, WrToPx as _},
         AppEvent,
     };
 
@@ -1223,14 +1219,11 @@ mod capture {
 
     impl ImageCache {
         /// Create frame_image for an `Api::frame_image` request.
-        ///
-        /// Gl context must be current.
         #[allow(clippy::too_many_arguments)]
         pub fn frame_image(
             &mut self,
-            renderer: &mut Renderer,
+            gl: &dyn gleam::gl::Gl,
             rect: PxRect,
-            capture_mode: bool,
             window_id: WindowId,
             frame_id: FrameId,
             scale_factor: Factor,
@@ -1251,7 +1244,7 @@ mod capture {
                 return id;
             }
 
-            let data = self.frame_image_data(renderer, rect, capture_mode, scale_factor, mask);
+            let data = self.frame_image_data(gl, rect, scale_factor, mask);
 
             let id = data.id;
 
@@ -1267,17 +1260,14 @@ mod capture {
         }
 
         /// Create frame_image for a capture request in the FrameRequest.
-        ///
-        /// Gl context must be current.
         pub fn frame_image_data(
             &mut self,
-            renderer: &mut Renderer,
+            gl: &dyn gleam::gl::Gl,
             rect: PxRect,
-            capture_mode: bool,
             scale_factor: Factor,
             mask: Option<ImageMaskMode>,
         ) -> ImageLoadedData {
-            let data = self.frame_image_data_impl(renderer, rect, capture_mode, scale_factor, mask);
+            let data = self.frame_image_data_impl(gl, rect, scale_factor, mask);
 
             let flags = if data.is_opaque {
                 ImageDescriptorFlags::IS_OPAQUE
@@ -1305,70 +1295,77 @@ mod capture {
 
         fn frame_image_data_impl(
             &mut self,
-            renderer: &mut Renderer,
+            gl: &dyn gleam::gl::Gl,
             rect: PxRect,
-            capture_mode: bool,
             scale_factor: Factor,
             mask: Option<ImageMaskMode>,
         ) -> ImageLoadedData {
-            // Firefox uses this API here:
-            // https://searchfox.org/mozilla-central/source/gfx/webrender_bindings/RendererScreenshotGrabber.cpp#87
-            let (handle, s) = renderer.get_screenshot_async(rect.to_wr_device(), rect.size.to_wr_device(), ImageFormat::BGRA8);
-            let mut buf = vec![0; s.width as usize * s.height as usize * 4];
-            if renderer.map_and_recycle_screenshot(handle, &mut buf, s.width as usize * 4) {
-                if !capture_mode {
-                    renderer.release_profiler_structures();
-                }
-
-                if let Some(mask) = mask {
-                    let (pixels, size, ppi, is_opaque, is_mask) = Self::convert_decoded(
-                        image::DynamicImage::ImageRgba8(image::ImageBuffer::from_raw(s.width as u32, s.height as u32, buf).unwrap()),
-                        Some(mask),
-                    );
-
-                    let id = self.add(ImageRequest {
-                        format: ImageDataFormat::A8 { size },
-                        data: pixels.clone(),
-                        max_decoded_len: u64::MAX,
-                        downscale: None,
-                        mask: Some(mask),
-                    });
-
-                    ImageLoadedData {
-                        id,
-                        size,
-                        ppi,
-                        is_opaque,
-                        is_mask,
-                        pixels,
-                    }
-                } else {
-                    let is_opaque = buf.chunks_exact(4).all(|bgra| bgra[3] == 255);
-
-                    let data = IpcBytes::from_vec(buf);
-                    let ppi = 96.0 * scale_factor.0;
-                    let ppi = Some(ImagePpi::splat(ppi));
-                    let size = s.to_px();
-
-                    let id = self.add(ImageRequest {
-                        format: ImageDataFormat::Bgra8 { size, ppi },
-                        data: data.clone(),
-                        max_decoded_len: u64::MAX,
-                        downscale: None,
-                        mask,
-                    });
+            let pixels_flipped = gl.read_pixels(
+                rect.origin.x.0,
+                rect.origin.y.0,
+                rect.size.width.0,
+                rect.size.height.0,
+                gleam::gl::BGRA,
+                gleam::gl::UNSIGNED_BYTE,
+            );
+            let mut buf = vec![0u8; pixels_flipped.len()];
+            assert_eq!(rect.size.width.0 as usize * rect.size.height.0 as usize * 4, buf.len());
+            let stride = 4 * rect.size.width.0 as usize;
+            for (px, buf) in pixels_flipped.chunks_exact(stride).rev().zip(buf.chunks_exact_mut(stride)) {
+                buf.copy_from_slice(px);
+            }
 
-                    ImageLoadedData {
-                        id,
-                        size,
-                        ppi,
-                        is_opaque,
-                        pixels: data,
-                        is_mask: false,
-                    }
+            if let Some(mask) = mask {
+                for bgra in buf.chunks_exact_mut(4) {
+                    bgra.swap(0, 3);
+                }
+                let (pixels, size, ppi, is_opaque, is_mask) = Self::convert_decoded(
+                    image::DynamicImage::ImageRgba8(
+                        image::ImageBuffer::from_raw(rect.size.width.0 as u32, rect.size.height.0 as u32, buf).unwrap(),
+                    ),
+                    Some(mask),
+                );
+
+                let id = self.add(ImageRequest {
+                    format: ImageDataFormat::A8 { size },
+                    data: pixels.clone(),
+                    max_decoded_len: u64::MAX,
+                    downscale: None,
+                    mask: Some(mask),
+                });
+
+                ImageLoadedData {
+                    id,
+                    size,
+                    ppi,
+                    is_opaque,
+                    is_mask,
+                    pixels,
                 }
             } else {
-                panic!("map_and_recycle_screenshot failed");
+                let is_opaque = buf.chunks_exact(4).all(|bgra| bgra[3] == 255);
+
+                let data = IpcBytes::from_vec(buf);
+                let ppi = 96.0 * scale_factor.0;
+                let ppi = Some(ImagePpi::splat(ppi));
+                let size = rect.size;
+
+                let id = self.add(ImageRequest {
+                    format: ImageDataFormat::Bgra8 { size, ppi },
+                    data: data.clone(),
+                    max_decoded_len: u64::MAX,
+                    downscale: None,
+                    mask,
+                });
+
+                ImageLoadedData {
+                    id,
+                    size,
+                    ppi,
+                    is_opaque,
+                    pixels: data,
+                    is_mask: false,
+                }
             }
         }
     }
diff --git a/zng-view/src/surface.rs b/zng-view/src/surface.rs
index 6946c2450..6c399fccd 100644
--- a/zng-view/src/surface.rs
+++ b/zng-view/src/surface.rs
@@ -404,11 +404,9 @@ impl Surface {
                 FrameCapture::Mask(m) => Some(Some(m)),
             };
             if let Some(mask) = capture {
-                self.context.make_current();
                 captured_data = Some(images.frame_image_data(
-                    renderer,
+                    &**self.context.gl(),
                     PxRect::from_size(self.size.to_px(self.scale_factor)),
-                    true,
                     self.scale_factor,
                     mask,
                 ));
@@ -418,11 +416,9 @@ impl Surface {
     }
 
     pub fn frame_image(&mut self, images: &mut ImageCache, mask: Option<ImageMaskMode>) -> ImageId {
-        self.context.make_current();
         images.frame_image(
-            self.renderer.as_mut().unwrap(),
+            &**self.context.gl(),
             PxRect::from_size(self.size.to_px(self.scale_factor)),
-            true,
             self.id,
             self.rendered_frame_id,
             self.scale_factor,
@@ -432,16 +428,7 @@ impl Surface {
 
     pub fn frame_image_rect(&mut self, images: &mut ImageCache, rect: PxRect, mask: Option<ImageMaskMode>) -> ImageId {
         let rect = PxRect::from_size(self.size.to_px(self.scale_factor)).intersection(&rect).unwrap();
-        self.context.make_current();
-        images.frame_image(
-            self.renderer.as_mut().unwrap(),
-            rect,
-            true,
-            self.id,
-            self.rendered_frame_id,
-            self.scale_factor,
-            mask,
-        )
+        images.frame_image(&**self.context.gl(), rect, self.id, self.rendered_frame_id, self.scale_factor, mask)
     }
 
     /// Calls the render extension command.
diff --git a/zng-view/src/window.rs b/zng-view/src/window.rs
index 8b793a326..9ec0a3360 100644
--- a/zng-view/src/window.rs
+++ b/zng-view/src/window.rs
@@ -1395,12 +1395,9 @@ impl Window {
             if ext_args.redraw || msg.composite_needed {
                 self.redraw();
             }
-            let renderer = self.renderer.as_mut().unwrap();
-            self.context.make_current();
             Some(images.frame_image_data(
-                renderer,
+                &**self.context.gl(),
                 PxRect::from_size(self.window.inner_size().to_px()),
-                true,
                 scale_factor,
                 mask,
             ))
@@ -1460,11 +1457,12 @@ impl Window {
 
     pub fn frame_image(&mut self, images: &mut ImageCache, mask: Option<ImageMaskMode>) -> ImageId {
         let scale_factor = self.scale_factor();
-        self.context.make_current();
+        if !self.context.is_software() {
+            self.redraw(); // refresh back buffer
+        }
         images.frame_image(
-            self.renderer.as_mut().unwrap(),
+            &**self.context.gl(),
             PxRect::from_size(self.window.inner_size().to_px()),
-            self.capture_mode,
             self.id,
             self.rendered_frame_id,
             scale_factor,
@@ -1477,16 +1475,10 @@ impl Window {
         let rect = PxRect::from_size(self.window.inner_size().to_px())
             .intersection(&rect)
             .unwrap_or_default();
-        self.context.make_current();
-        images.frame_image(
-            self.renderer.as_mut().unwrap(),
-            rect,
-            self.capture_mode,
-            self.id,
-            self.rendered_frame_id,
-            scale_factor,
-            mask,
-        )
+        if !self.context.is_software() {
+            self.redraw(); // refresh back buffer
+        }
+        images.frame_image(&**self.context.gl(), rect, self.id, self.rendered_frame_id, scale_factor, mask)
     }
 
     /// (global_position, monitor_position)


Test patch: