[librsvg: 3/4] surface_utils: move to_pixbuf method to shared surface

[Federico Mena Quintero <federico src gnome org>]

commit 82f77581a6f0b3cef21c0eeb6e274a105218e47e
Author: Paolo Borelli <pborelli gnome org>
Date:   Sun Jan 3 16:28:30 2021 +0100

    surface_utils: move to_pixbuf method to shared surface
    
    This keeps related code grouped together. Also use iterators
    instead of nested loops.

 benches/pixbuf_from_surface.rs      |  3 +--
 src/c_api/pixbuf_utils.rs           | 46 ++++++-------------------------------
 src/surface_utils/shared_surface.rs | 27 ++++++++++++++++++++++
 3 files changed, 35 insertions(+), 41 deletions(-)
---
diff --git a/benches/pixbuf_from_surface.rs b/benches/pixbuf_from_surface.rs
index 8d3e22f0..b88ac612 100644
--- a/benches/pixbuf_from_surface.rs
+++ b/benches/pixbuf_from_surface.rs
@@ -1,7 +1,6 @@
 use criterion::{criterion_group, criterion_main, Criterion};
 
 use librsvg::{
-    c_api::pixbuf_utils::pixbuf_from_surface,
     surface_utils::{
         shared_surface::{ExclusiveImageSurface, SurfaceType},
         ImageSurfaceDataExt, Pixel,
@@ -38,7 +37,7 @@ fn bench_pixbuf_from_surface(c: &mut Criterion) {
 
         let surface = surface.share().unwrap();
 
-        b.iter(|| pixbuf_from_surface(&surface).unwrap())
+        b.iter(|| surface.to_pixbuf().unwrap())
     });
 }
 
diff --git a/src/c_api/pixbuf_utils.rs b/src/c_api/pixbuf_utils.rs
index 73433a74..a132a593 100644
--- a/src/c_api/pixbuf_utils.rs
+++ b/src/c_api/pixbuf_utils.rs
@@ -7,62 +7,30 @@ use std::ptr;
 
 use gdk_pixbuf::{Colorspace, Pixbuf};
 use glib::translate::*;
-use rgb::FromSlice;
 
 use super::dpi::Dpi;
 use super::handle::{checked_i32, set_gerror};
 use super::sizing::LegacySize;
 use crate::api::{CairoRenderer, Loader};
 
-use crate::{
-    error::RenderingError,
-    surface_utils::shared_surface::{SharedImageSurface, SurfaceType},
-    surface_utils::{Pixel, PixelOps},
-};
-
-fn pixbuf_new(width: i32, height: i32) -> Result<Pixbuf, RenderingError> {
-    assert!(width > 0 && height > 0);
-
-    Pixbuf::new(Colorspace::Rgb, true, 8, width, height)
-        .ok_or_else(|| RenderingError::OutOfMemory(String::from("creating a Pixbuf")))
-}
+use crate::error::RenderingError;
+use crate::surface_utils::shared_surface::{SharedImageSurface, SurfaceType};
 
 pub fn empty_pixbuf() -> Result<Pixbuf, RenderingError> {
     // GdkPixbuf does not allow zero-sized pixbufs, but Cairo allows zero-sized
     // surfaces.  In this case, return a 1-pixel transparent pixbuf.
 
-    let pixbuf = pixbuf_new(1, 1)?;
+    let pixbuf = Pixbuf::new(Colorspace::Rgb, true, 8, 1, 1)
+        .ok_or_else(|| RenderingError::OutOfMemory(String::from("creating a Pixbuf")))?;
     pixbuf.put_pixel(0, 0, 0, 0, 0, 0);
 
     Ok(pixbuf)
 }
 
 pub fn pixbuf_from_surface(surface: &SharedImageSurface) -> Result<Pixbuf, RenderingError> {
-    let width = surface.width();
-    let height = surface.height();
-
-    let pixbuf = pixbuf_new(width, height)?;
-    assert!(pixbuf.get_colorspace() == Colorspace::Rgb);
-    assert!(pixbuf.get_bits_per_sample() == 8);
-    assert!(pixbuf.get_n_channels() == 4);
-
-    let pixels = unsafe { pixbuf.get_pixels() };
-    let stride = pixbuf.get_rowstride() as usize;
-
-    // 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 as usize);
-
-    for (src_row, dest_row) in surface.rows().zip(pixbuf_rows) {
-        let row: &mut [Pixel] = dest_row.as_rgba_mut();
-
-        for (src, dest) in src_row.iter().zip(row.iter_mut()) {
-            *dest = Pixel::from(*src).unpremultiply();
-        }
-    }
-
-    Ok(pixbuf)
+    surface
+        .to_pixbuf()
+        .ok_or_else(|| RenderingError::OutOfMemory(String::from("creating a Pixbuf")))
 }
 
 enum SizeKind {
diff --git a/src/surface_utils/shared_surface.rs b/src/surface_utils/shared_surface.rs
index 899e8e58..86123512 100644
--- a/src/surface_utils/shared_surface.rs
+++ b/src/surface_utils/shared_surface.rs
@@ -305,6 +305,33 @@ impl ImageSurface<Shared> {
         surf.share()
     }
 
+    pub fn to_pixbuf(&self) -> Option<Pixbuf> {
+        let width = self.width();
+        let height = self.height();
+
+        let pixbuf = Pixbuf::new(Colorspace::Rgb, true, 8, width, height)?;
+
+        assert!(pixbuf.get_colorspace() == Colorspace::Rgb);
+        assert!(pixbuf.get_bits_per_sample() == 8);
+        assert!(pixbuf.get_n_channels() == 4);
+
+        let pixbuf_data = unsafe { pixbuf.get_pixels() };
+        let stride = pixbuf.get_rowstride() as usize;
+
+        // 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).
+        pixbuf_data
+            .chunks_mut(stride)
+            .take(height as usize)
+            .map(|row| row.as_rgba_mut())
+            .zip(self.rows())
+            .flat_map(|(dest_row, src_row)| src_row.iter().zip(dest_row.iter_mut()))
+            .for_each(|(src, dest)| *dest = Pixel::from(*src).unpremultiply());
+
+        Some(pixbuf)
+    }
+
     /// Returns `true` if the surface contains meaningful data only in the alpha channel.
     #[inline]
     pub fn is_alpha_only(&self) -> bool {