[librsvg] svg: store image resources in the svg struct



commit 6b2cdff4ce400ed6b85daff6dc42bfde0f68a4e3
Author: Paolo Borelli <pborelli gnome org>
Date:   Tue Jan 15 12:37:13 2019 +0100

    svg: store image resources in the svg struct
    
    This makes it consistent with xml resources. It also allows images
    to be cached in case they are rendered more than once.
    The images are loaded lazily during rendering which goes in the
    opposite direction with the goal of having an immutable Svg
    object, but at least this is the same of other resources. Beside
    images are just used in two places, and filters/images was
    already doing a hack to load the image at rendering time.

 rsvg_internals/src/drawing_ctx.rs   |   4 ++
 rsvg_internals/src/filters/image.rs |  22 +++-----
 rsvg_internals/src/handle.rs        |  64 +---------------------
 rsvg_internals/src/image.rs         |  32 +++--------
 rsvg_internals/src/node.rs          |  16 ------
 rsvg_internals/src/svg.rs           | 102 ++++++++++++++++++++++++++++++++++--
 rsvg_internals/src/xml.rs           |   4 --
 7 files changed, 118 insertions(+), 126 deletions(-)
---
diff --git a/rsvg_internals/src/drawing_ctx.rs b/rsvg_internals/src/drawing_ctx.rs
index a5116d39..b961e5e6 100644
--- a/rsvg_internals/src/drawing_ctx.rs
+++ b/rsvg_internals/src/drawing_ctx.rs
@@ -713,6 +713,10 @@ impl DrawingCtx {
         }
     }
 
+    pub fn lookup_image(&self, href: &str) -> Option<cairo::ImageSurface> {
+        self.svg.lookup_image(href)
+    }
+
     pub fn draw_node_on_surface(
         &mut self,
         node: &RsvgNode,
diff --git a/rsvg_internals/src/filters/image.rs b/rsvg_internals/src/filters/image.rs
index bd8fd21b..262cb866 100644
--- a/rsvg_internals/src/filters/image.rs
+++ b/rsvg_internals/src/filters/image.rs
@@ -7,7 +7,6 @@ use aspect_ratio::AspectRatio;
 use attributes::Attribute;
 use drawing_ctx::DrawingCtx;
 use error::{NodeError, RenderingError};
-use handle::{self, LoadOptions};
 use node::{CascadedValues, NodeResult, NodeTrait, RsvgNode};
 use parsers::{ParseError, ParseValue};
 use property_bag::PropertyBag;
@@ -23,7 +22,6 @@ pub struct Image {
     base: Primitive,
     aspect: Cell<AspectRatio>,
     href: RefCell<Option<Href>>,
-    load_options: RefCell<Option<LoadOptions>>,
 }
 
 impl Image {
@@ -34,7 +32,6 @@ impl Image {
             base: Primitive::new::<Self>(),
             aspect: Cell::new(AspectRatio::default()),
             href: RefCell::new(None),
-            load_options: RefCell::new(None),
         }
     }
 
@@ -119,17 +116,18 @@ impl Image {
         bounds_builder: BoundsBuilder<'_>,
         href: &Href,
     ) -> Result<ImageSurface, FilterError> {
-        let url = if let Href::PlainUrl(ref url) = *href {
-            url
+        let surface = if let Href::PlainUrl(ref url) = *href {
+            draw_ctx.lookup_image(&url)
         } else {
             unreachable!();
         };
 
-        let load_options_ref = self.load_options.borrow();
-
         // FIXME: translate the error better here
-        let surface = handle::load_image_to_surface(load_options_ref.as_ref().unwrap(), &url)
-            .map_err(|_| FilterError::InvalidInput)?;
+        if surface.is_none() {
+            return Err(FilterError::InvalidInput);
+        }
+
+        let surface = surface.unwrap();
 
         let output_surface = ImageSurface::create(
             cairo::Format::ARgb32,
@@ -196,12 +194,6 @@ impl NodeTrait for Image {
 
         Ok(())
     }
-
-    fn resolve_resources(&self, load_options: &LoadOptions) -> NodeResult {
-        *self.load_options.borrow_mut() = Some(load_options.clone());
-
-        Ok(())
-    }
 }
 
 impl Filter for Image {
diff --git a/rsvg_internals/src/handle.rs b/rsvg_internals/src/handle.rs
index 47735d07..9561469c 100644
--- a/rsvg_internals/src/handle.rs
+++ b/rsvg_internals/src/handle.rs
@@ -8,7 +8,7 @@ use std::slice;
 
 use cairo::{self, ImageSurface, Status};
 use cairo_sys;
-use gdk_pixbuf::{Pixbuf, PixbufLoader, PixbufLoaderExt};
+use gdk_pixbuf::Pixbuf;
 use gdk_pixbuf_sys;
 use gio::{self, FileExt};
 use gio_sys;
@@ -23,7 +23,6 @@ use allowed_url::{AllowedUrl, Href};
 use dpi::Dpi;
 use drawing_ctx::{DrawingCtx, RsvgRectangle};
 use error::{set_gerror, DefsLookupErrorKind, LoadingError, RenderingError};
-use io;
 use load::LoadContext;
 use node::RsvgNode;
 use pixbuf_utils::pixbuf_from_surface;
@@ -577,67 +576,6 @@ extern "C" {
     fn rsvg_handle_get_rust(handle: *const RsvgHandle) -> *mut Handle;
 }
 
-pub fn load_image_to_surface(
-    load_options: &LoadOptions,
-    url: &str,
-) -> Result<ImageSurface, LoadingError> {
-    let aurl = AllowedUrl::from_href(url, load_options.base_url.as_ref())
-        .map_err(|_| LoadingError::BadUrl)?;
-
-    let data = io::acquire_data(&aurl, None)?;
-
-    if data.data.len() == 0 {
-        return Err(LoadingError::EmptyData);
-    }
-
-    let loader = if let Some(ref content_type) = data.content_type {
-        PixbufLoader::new_with_mime_type(content_type)?
-    } else {
-        PixbufLoader::new()
-    };
-
-    loader.write(&data.data)?;
-    loader.close()?;
-
-    let pixbuf = loader.get_pixbuf().ok_or(LoadingError::Unknown)?;
-
-    let surface = SharedImageSurface::from_pixbuf(&pixbuf)?.into_image_surface()?;
-
-    if load_options.flags.keep_image_data {
-        if let Some(mime_type) = data.content_type {
-            extern "C" {
-                fn cairo_surface_set_mime_data(
-                    surface: *mut cairo_sys::cairo_surface_t,
-                    mime_type: *const libc::c_char,
-                    data: *mut libc::c_char,
-                    length: libc::c_ulong,
-                    destroy: cairo_sys::cairo_destroy_func_t,
-                    closure: *mut libc::c_void,
-                ) -> Status;
-            }
-
-            let data_ptr = ToGlibContainerFromSlice::to_glib_full_from_slice(&data.data);
-
-            unsafe {
-                let status = cairo_surface_set_mime_data(
-                    surface.to_glib_none().0,
-                    mime_type.to_glib_none().0,
-                    data_ptr as *mut _,
-                    data.data.len() as libc::c_ulong,
-                    Some(glib_sys::g_free),
-                    data_ptr as *mut _,
-                );
-
-                if status != Status::Success {
-                    return Err(LoadingError::Cairo(status));
-                }
-            }
-        }
-    }
-
-    Ok(surface)
-}
-
 #[no_mangle]
 pub unsafe extern "C" fn rsvg_handle_rust_new() -> *mut Handle {
     Box::into_raw(Box::new(Handle::new()))
diff --git a/rsvg_internals/src/image.rs b/rsvg_internals/src/image.rs
index 11bfb32c..74041e07 100644
--- a/rsvg_internals/src/image.rs
+++ b/rsvg_internals/src/image.rs
@@ -9,7 +9,6 @@ use bbox::BoundingBox;
 use drawing_ctx::DrawingCtx;
 use error::{NodeError, RenderingError};
 use float_eq_cairo::ApproxEqCairo;
-use handle::{self, LoadOptions};
 use length::*;
 use node::*;
 use parsers::ParseValue;
@@ -22,7 +21,6 @@ pub struct NodeImage {
     w: Cell<Length>,
     h: Cell<Length>,
     href: RefCell<Option<Href>>,
-    surface: RefCell<Option<cairo::ImageSurface>>,
 }
 
 impl NodeImage {
@@ -34,7 +32,6 @@ impl NodeImage {
             w: Cell::new(Length::default()),
             h: Cell::new(Length::default()),
             href: RefCell::new(None),
-            surface: RefCell::new(None),
         }
     }
 }
@@ -78,26 +75,6 @@ impl NodeTrait for NodeImage {
         Ok(())
     }
 
-    fn resolve_resources(&self, load_options: &LoadOptions) -> NodeResult {
-        match *self.href.borrow() {
-            None => (),
-            Some(Href::PlainUrl(ref url)) => {
-                *self.surface.borrow_mut() = Some(
-                    // FIXME: translate the error better here
-                    handle::load_image_to_surface(load_options, &url).map_err(|e| {
-                        NodeError::value_error(
-                            Attribute::XlinkHref,
-                            &format!("could not load image: {}", e),
-                        )
-                    })?,
-                );
-            }
-            _ => unreachable!(),
-        };
-
-        Ok(())
-    }
-
     fn draw(
         &self,
         node: &RsvgNode,
@@ -105,9 +82,14 @@ impl NodeTrait for NodeImage {
         draw_ctx: &mut DrawingCtx,
         clipping: bool,
     ) -> Result<(), RenderingError> {
-        let values = cascaded.get();
+        let surface = if let Some(Href::PlainUrl(ref url)) = *self.href.borrow() {
+            draw_ctx.lookup_image(&url)
+        } else {
+            None
+        };
 
-        if let Some(ref surface) = *self.surface.borrow() {
+        if let Some(ref surface) = surface {
+            let values = cascaded.get();
             let params = draw_ctx.get_view_params();
 
             let x = self.x.get().normalize(values, &params);
diff --git a/rsvg_internals/src/node.rs b/rsvg_internals/src/node.rs
index b7866594..6c0092b6 100644
--- a/rsvg_internals/src/node.rs
+++ b/rsvg_internals/src/node.rs
@@ -8,7 +8,6 @@ use cond::{locale_from_environment, RequiredExtensions, RequiredFeatures, System
 use css::CssStyles;
 use drawing_ctx::DrawingCtx;
 use error::*;
-use handle::LoadOptions;
 use parsers::Parse;
 use property_bag::PropertyBag;
 use state::{ComputedValues, Overflow, SpecifiedValue, State};
@@ -103,12 +102,6 @@ pub trait NodeTrait: Downcast {
     /// from defaults in the node's `State`.
     fn set_overridden_properties(&self, _state: &mut State) {}
 
-    /// Load resources specified in the attributes.
-    fn resolve_resources(&self, _load_options: &LoadOptions) -> NodeResult {
-        // Most of the nodes do not need to load resources
-        Ok(())
-    }
-
     fn draw(
         &self,
         _node: &RsvgNode,
@@ -445,15 +438,6 @@ impl Node {
         }
     }
 
-    pub fn resolve_resources(&self, load_options: &LoadOptions) {
-        match self.data.node_impl.resolve_resources(load_options) {
-            Ok(_) => (),
-            Err(e) => {
-                self.set_error(e);
-            }
-        }
-    }
-
     /// Implements a very limited CSS selection engine
     fn set_css_styles(&self, css_styles: &CssStyles) {
         // Try to properly support all of the following, including inheritance:
diff --git a/rsvg_internals/src/svg.rs b/rsvg_internals/src/svg.rs
index 2cc4c6d4..9d7abc11 100644
--- a/rsvg_internals/src/svg.rs
+++ b/rsvg_internals/src/svg.rs
@@ -1,4 +1,7 @@
+use cairo::{ImageSurface, Status};
+use gdk_pixbuf::{PixbufLoader, PixbufLoaderExt};
 use gio;
+use glib::translate::*;
 use std::cell::RefCell;
 use std::collections::hash_map::Entry;
 use std::collections::HashMap;
@@ -10,6 +13,7 @@ use handle::LoadOptions;
 use io;
 use node::RsvgNode;
 use state::ComputedValues;
+use surface_utils::shared_surface::SharedImageSurface;
 use xml::XmlState;
 use xml2_load::xml_state_load_from_possibly_compressed_stream;
 
@@ -22,10 +26,11 @@ pub struct Svg {
 
     ids: HashMap<String, RsvgNode>,
 
-    // This requires interior mutability because we load the extern
+    // These require interior mutability because we load the extern
     // resources all over the place.  Eventually we'll be able to do this
     // once, at loading time, and keep this immutable.
     externs: RefCell<Resources>,
+    images: RefCell<Images>,
 
     // Once we do not need to load externs, we can drop this as well
     load_options: LoadOptions,
@@ -40,6 +45,7 @@ impl Svg {
             tree,
             ids,
             externs: RefCell::new(Resources::new()),
+            images: RefCell::new(Images::new()),
             load_options,
         }
     }
@@ -74,6 +80,13 @@ impl Svg {
     pub fn lookup_node_by_id(&self, id: &str) -> Option<RsvgNode> {
         self.ids.get(id).map(|n| (*n).clone())
     }
+
+    pub fn lookup_image(&self, href: &str) -> Option<ImageSurface> {
+        self.images
+            .borrow_mut()
+            .lookup(&self.load_options, href)
+            .and_then(|s| s.into_image_surface().ok())
+    }
 }
 
 struct Resources {
@@ -87,8 +100,6 @@ impl Resources {
         }
     }
 
-    /// Returns a node referenced by a fragment ID, from an
-    /// externally-loaded SVG file.
     pub fn lookup(&mut self, load_options: &LoadOptions, fragment: &Fragment) -> Option<RsvgNode> {
         if let Some(ref href) = fragment.uri() {
             // FIXME: propagate errors from the loader
@@ -117,9 +128,94 @@ impl Resources {
     }
 }
 
+struct Images {
+    images: HashMap<AllowedUrl, SharedImageSurface>,
+}
+
+impl Images {
+    pub fn new() -> Images {
+        Images {
+            images: Default::default(),
+        }
+    }
+
+    pub fn lookup(&mut self, load_options: &LoadOptions, href: &str) -> Option<SharedImageSurface> {
+        // FIXME: propagate errors
+        let aurl = AllowedUrl::from_href(href, load_options.base_url.as_ref()).ok()?;
+
+        match self.images.entry(aurl) {
+            Entry::Occupied(e) => Some(e.get().clone()),
+            Entry::Vacant(e) => {
+                // FIXME: propagate errors
+                let surface = load_image(load_options, e.key()).ok()?;
+                let res = e.insert(surface);
+                Some(res.clone())
+            }
+        }
+    }
+}
+
 fn load_svg(load_options: &LoadOptions, aurl: &AllowedUrl) -> Result<Svg, LoadingError> {
     // FIXME: pass a cancellable to these
     io::acquire_stream(aurl, None).and_then(|stream| {
         Svg::load_from_stream(&load_options.copy_with_base_url(aurl), &stream, None)
     })
 }
+
+fn load_image(
+    load_options: &LoadOptions,
+    aurl: &AllowedUrl,
+) -> Result<SharedImageSurface, LoadingError> {
+    let data = io::acquire_data(&aurl, None)?;
+
+    if data.data.len() == 0 {
+        return Err(LoadingError::EmptyData);
+    }
+
+    let loader = if let Some(ref content_type) = data.content_type {
+        PixbufLoader::new_with_mime_type(content_type)?
+    } else {
+        PixbufLoader::new()
+    };
+
+    loader.write(&data.data)?;
+    loader.close()?;
+
+    let pixbuf = loader.get_pixbuf().ok_or(LoadingError::Unknown)?;
+
+    let surface = SharedImageSurface::from_pixbuf(&pixbuf)?;
+
+    if load_options.flags.keep_image_data {
+        if let Some(mime_type) = data.content_type {
+            extern "C" {
+                fn cairo_surface_set_mime_data(
+                    surface: *mut cairo_sys::cairo_surface_t,
+                    mime_type: *const libc::c_char,
+                    data: *mut libc::c_char,
+                    length: libc::c_ulong,
+                    destroy: cairo_sys::cairo_destroy_func_t,
+                    closure: *mut libc::c_void,
+                ) -> Status;
+            }
+
+            let data_ptr = ToGlibContainerFromSlice::to_glib_full_from_slice(&data.data);
+
+            unsafe {
+                let status = cairo_surface_set_mime_data(
+                    surface.to_glib_none().0,
+                    mime_type.to_glib_none().0,
+                    data_ptr as *mut _,
+                    data.data.len() as libc::c_ulong,
+                    Some(glib_sys::g_free),
+                    data_ptr as *mut _,
+                );
+
+                if status != Status::Success {
+                    return Err(LoadingError::Cairo(status));
+                }
+            }
+        }
+    }
+
+    Ok(surface)
+}
diff --git a/rsvg_internals/src/xml.rs b/rsvg_internals/src/xml.rs
index 45f02f8d..cefd2ec8 100644
--- a/rsvg_internals/src/xml.rs
+++ b/rsvg_internals/src/xml.rs
@@ -331,10 +331,6 @@ impl XmlState {
 
         new_node.set_overridden_properties();
 
-        // For now we load resources directly when parsing, but probably we should
-        // move this to a trasversal of the tree once we finished parsing
-        new_node.resolve_resources(&self.load_options);
-
         new_node
     }
 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]