[librsvg/librsvg-2.48] Reimplement ExclusiveImageSurface::from_pixbuf() in terms of iterators

[Federico Mena Quintero <federico src gnome org>]

commit 86fe2c045ddbc19f0269042596085a25a8670b2d
Author: Federico Mena Quintero <federico gnome org>
Date:   Fri May 29 13:38:38 2020 -0500

    Reimplement ExclusiveImageSurface::from_pixbuf() in terms of iterators
    
    Also, fix pixbuf_from_surface() from a case I found in the reverse
    conversion:  when using a pixbuf's data as a slice, one can't use
    ChunksExact; it must be just Chunks.  This is because gdk-pixbuf tends
    to avoid allocating the rowstride padding at the end of the last row
    in an image, i.e. the last row is only as wide as its pixels, instead
    of having the full rowstride.

 librsvg/pixbuf_utils.rs                            |   5 +-
 rsvg_internals/src/surface_utils/shared_surface.rs | 104 +++++++++++++++------
 2 files changed, 79 insertions(+), 30 deletions(-)
---
diff --git a/librsvg/pixbuf_utils.rs b/librsvg/pixbuf_utils.rs
index 6d5df5a7..139328d9 100644
--- a/librsvg/pixbuf_utils.rs
+++ b/librsvg/pixbuf_utils.rs
@@ -48,7 +48,10 @@ pub fn pixbuf_from_surface(surface: &SharedImageSurface) -> Result<Pixbuf, Rende
     let width_in_bytes = width * 4;
     assert!(width_in_bytes <= stride);
 
-    let pixbuf_rows = pixels.chunks_exact_mut(stride).take(height);
+    // We use chunks_mut(), not chunks_exact_mut(), because gdk-pixbuf tends
+    // to make the last row *not* have the full stride (i.e. it is
+    // only as wide as the pixels in that row).
+    let pixbuf_rows = pixels.chunks_mut(stride).take(height);
 
     for (src_row, dest_row) in surface.rows().zip(pixbuf_rows) {
         let row: &mut [RGBA8] = dest_row[..width_in_bytes].as_pixels_mut();
diff --git a/rsvg_internals/src/surface_utils/shared_surface.rs 
b/rsvg_internals/src/surface_utils/shared_surface.rs
index 7811a27c..8602975a 100644
--- a/rsvg_internals/src/surface_utils/shared_surface.rs
+++ b/rsvg_internals/src/surface_utils/shared_surface.rs
@@ -6,7 +6,7 @@ use std::slice;
 
 use gdk_pixbuf::{Colorspace, Pixbuf};
 use nalgebra::{storage::Storage, Dim, Matrix};
-use rgb::AsPixels;
+use rgb::{AsPixels, RGB8, RGBA8};
 
 use crate::rect::{IRect, Rect};
 use crate::surface_utils::srgb;
@@ -125,6 +125,13 @@ pub struct Rows<'a> {
     next_row: i32,
 }
 
+/// Iterator over the mutable rows of an `ExclusiveImageSurface`.
+pub struct RowsMut<'a> {
+    surface: &'a mut ExclusiveImageSurface,
+    data_ptr: *mut u8,
+    next_row: i32,
+}
+
 impl IsAlphaOnly for AlphaOnly {
     const IS_ALPHA_ONLY: bool = true;
 }
@@ -251,59 +258,66 @@ impl ImageSurface<Shared> {
         content_type: Option<&str>,
     ) -> Result<SharedImageSurface, cairo::Status> {
         assert!(pixbuf.get_colorspace() == Colorspace::Rgb);
+        assert!(pixbuf.get_bits_per_sample() == 8);
 
         let n_channels = pixbuf.get_n_channels();
         assert!(n_channels == 3 || n_channels == 4);
         let has_alpha = n_channels == 4;
 
-        let (width, height) = (pixbuf.get_width(), pixbuf.get_height());
+        let width = pixbuf.get_width();
+        let height = pixbuf.get_height();
         assert!(width > 0 && height > 0);
 
-        let pixbuf_stride = pixbuf.get_rowstride();
+        let pixbuf_stride = pixbuf.get_rowstride() as usize;
         assert!(pixbuf_stride > 0);
 
         let pixbuf_data = unsafe { pixbuf.get_pixels() };
 
-        let mut surf = cairo::ImageSurface::create(cairo::Format::ARgb32, width, height)?;
+        let mut surf = ExclusiveImageSurface::new(width, height, SurfaceType::SRgb)?;
 
-        let (width, height) = (width as usize, height as usize);
-        let pixbuf_stride = pixbuf_stride as usize;
+        let width = width as usize;
+        let height = height as usize;
 
         {
-            let surf_stride = surf.get_stride() as usize;
+            // We use chunks(), not chunks_exact(), because gdk-pixbuf tends
+            // to make the last row *not* have the full stride (i.e. it is
+            // only as wide as the pixels in that row).
+            let pixbuf_rows = pixbuf_data.chunks(pixbuf_stride).take(height);
 
-            let mut surf_data = surf.get_data().unwrap();
+            let surf_rows = surf.rows_mut();
 
             if has_alpha {
-                for y in 0..height {
-                    for x in 0..width {
-                        let ofs = pixbuf_stride * y + 4 * x;
+                let width_in_bytes = width * 4;
+
+                for (pixbuf_row, surf_row) in pixbuf_rows.zip(surf_rows) {
+                    let pixbuf_row: &[RGBA8] = pixbuf_row[..width_in_bytes].as_pixels();
 
+                    for (src, dest) in pixbuf_row.iter().zip(surf_row.iter_mut()) {
                         let pixel = Pixel {
-                            r: pixbuf_data[ofs],
-                            g: pixbuf_data[ofs + 1],
-                            b: pixbuf_data[ofs + 2],
-                            a: pixbuf_data[ofs + 3],
+                            r: src.r,
+                            g: src.g,
+                            b: src.b,
+                            a: src.a,
                         };
 
                         let pixel = pixel.premultiply();
-
-                        surf_data.set_pixel(surf_stride, pixel, x as u32, y as u32);
+                        dest.r = pixel.r;
+                        dest.g = pixel.g;
+                        dest.b = pixel.b;
+                        dest.a = pixel.a;
                     }
                 }
             } else {
-                for y in 0..height {
-                    for x in 0..width {
-                        let ofs = pixbuf_stride * y + 3 * x;
+                let width_in_bytes = width * 3;
 
-                        let pixel = Pixel {
-                            r: pixbuf_data[ofs],
-                            g: pixbuf_data[ofs + 1],
-                            b: pixbuf_data[ofs + 2],
-                            a: 0xff,
-                        };
+                for (pixbuf_row, surf_row) in pixbuf_rows.zip(surf_rows) {
+                    let pixbuf_row: &[RGB8] = pixbuf_row[..width_in_bytes].as_pixels();
 
-                        surf_data.set_pixel(surf_stride, pixel, x as u32, y as u32);
+                    for (src, dest) in pixbuf_row.iter().zip(surf_row.iter_mut()) {
+                        dest.r = src.r;
+                        dest.g = src.g;
+                        dest.b = src.b;
+                        dest.a = 0xff;
                     }
                 }
             }
@@ -311,13 +325,13 @@ impl ImageSurface<Shared> {
 
         match (data, content_type) {
             (Some(bytes), Some(content_type)) => {
-                surf.set_mime_data(content_type, bytes)?;
+                surf.surface.set_mime_data(content_type, bytes)?;
             }
 
             (_, _) => (),
         }
 
-        Self::wrap(surf, SurfaceType::SRgb)
+        surf.share()
     }
 
     /// Returns `true` if the surface contains meaningful data only in the alpha channel.
@@ -1184,6 +1198,28 @@ impl<'a> Iterator for Rows<'a> {
     }
 }
 
+impl<'a> Iterator for RowsMut<'a> {
+    type Item = &'a mut [CairoARGB];
+
+    fn next(&mut self) -> Option<Self::Item> {
+        if self.next_row == self.surface.height {
+            return None;
+        }
+
+        let row = self.next_row as usize;
+
+        self.next_row += 1;
+
+        unsafe {
+            let row_ptr = self.data_ptr.offset(row as isize * self.surface.stride);
+            let row_of_bytes = slice::from_raw_parts_mut(row_ptr, self.surface.width as usize * 4);
+            let pixels = row_of_bytes.as_pixels_mut();
+            assert!(pixels.len() == self.surface.width as usize);
+            Some(pixels)
+        }
+    }
+}
+
 /// Performs the arithmetic composite operation. Public for benchmarking.
 #[inline]
 pub fn composite_arithmetic(
@@ -1293,6 +1329,16 @@ impl ImageSurface<Exclusive> {
         let cr = cairo::Context::new(&self.surface);
         draw_fn(&cr)
     }
+
+    pub fn rows_mut(&mut self) -> RowsMut {
+        let data_ptr = self.surface.get_data().unwrap().as_mut_ptr();
+
+        RowsMut {
+            surface: self,
+            data_ptr,
+            next_row: 0,
+        }
+    }
 }
 
 #[cfg(test)]