propertiesView: Use image dimensions from decoder

This also shows the units for SVG dimensions if not pixels.

Issue #43

Part-of: <https://gitlab.gnome.org/Incubator/loupe/-/merge_requests/136>

Cargo.lock

@@ -310,12 +310,6 @@ version = "3.12.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "0d261e256854913907f67ed06efbc3338dfe6179796deefc1ff763fc1aee5535"
 
-[[package]]
-name = "bytecount"
-version = "0.6.3"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "2c676a478f63e9fa2dd5368a42f28bba0d6c560b775f38583c8bbaa7fcd67c9c"
-
 [[package]]
 name = "bytemuck"
 version = "1.13.1"
@@ -353,37 +347,6 @@ dependencies = [
  "system-deps",
 ]
 
-[[package]]
-name = "camino"
-version = "1.1.3"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "6031a462f977dd38968b6f23378356512feeace69cef817e1a4475108093cec3"
-dependencies = [
- "serde",
-]
-
-[[package]]
-name = "cargo-platform"
-version = "0.1.2"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "cbdb825da8a5df079a43676dbe042702f1707b1109f713a01420fbb4cc71fa27"
-dependencies = [
- "serde",
-]
-
-[[package]]
-name = "cargo_metadata"
-version = "0.14.2"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "4acbb09d9ee8e23699b9634375c72795d095bf268439da88562cf9b501f181fa"
-dependencies = [
- "camino",
- "cargo-platform",
- "semver",
- "serde",
- "serde_json",
-]
-
 [[package]]
 name = "cast"
 version = "0.3.0"
@@ -726,12 +689,6 @@ dependencies = [
  "winapi",
 ]
 
-[[package]]
-name = "doc-comment"
-version = "0.3.3"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "fea41bba32d969b513997752735605054bc0dfa92b4c56bf1189f2e174be7a10"
-
 [[package]]
 name = "dtoa"
 version = "0.4.8"
@@ -883,15 +840,6 @@ dependencies = [
  "libc",
 ]
 
-[[package]]
-name = "error-chain"
-version = "0.12.4"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "2d2f06b9cac1506ece98fe3231e3cc9c4410ec3d5b1f24ae1c8946f0742cdefc"
-dependencies = [
- "version_check",
-]
-
 [[package]]
 name = "event-listener"
 version = "2.5.3"
@@ -1363,12 +1311,6 @@ dependencies = [
  "system-deps",
 ]
 
-[[package]]
-name = "glob"
-version = "0.3.1"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "d2fabcfbdc87f4758337ca535fb41a6d701b65693ce38287d856d1674551ec9b"
-
 [[package]]
 name = "gloo-timers"
 version = "0.2.6"
@@ -1936,7 +1878,6 @@ dependencies = [
  "once_cell",
  "pretty_env_logger",
  "safe-transmute",
- "svg_metadata",
  "url",
 ]
 
@@ -2515,17 +2456,6 @@ dependencies = [
  "unicode-ident",
 ]
 
-[[package]]
-name = "pulldown-cmark"
-version = "0.9.2"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "2d9cc634bc78768157b5cbfe988ffcd1dcba95cd2b2f03a88316c08c6d00ed63"
-dependencies = [
- "bitflags",
- "memchr",
- "unicase",
-]
-
 [[package]]
 name = "qoi"
 version = "0.4.1"
@@ -2720,15 +2650,6 @@ dependencies = [
  "bytemuck",
 ]
 
-[[package]]
-name = "roxmltree"
-version = "0.18.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "d8f595a457b6b8c6cda66a48503e92ee8d19342f905948f29c383200ec9eb1d8"
-dependencies = [
- "xmlparser",
-]
-
 [[package]]
 name = "rustc-demangle"
 version = "0.1.21"
@@ -2758,12 +2679,6 @@ dependencies = [
  "windows-sys 0.45.0",
 ]
 
-[[package]]
-name = "ryu"
-version = "1.0.13"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "f91339c0467de62360649f8d3e185ca8de4224ff281f66000de5eb2a77a79041"
-
 [[package]]
 name = "safe-transmute"
 version = "0.11.2"
@@ -2779,15 +2694,6 @@ dependencies = [
  "bytemuck",
 ]
 
-[[package]]
-name = "same-file"
-version = "1.0.6"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "93fc1dc3aaa9bfed95e02e6eadabb4baf7e3078b0bd1b4d7b6b0b68378900502"
-dependencies = [
- "winapi-util",
-]
-
 [[package]]
 name = "scopeguard"
 version = "1.1.0"
@@ -2823,9 +2729,6 @@ name = "semver"
 version = "1.0.16"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "58bc9567378fc7690d6b2addae4e60ac2eeea07becb2c64b9f218b53865cba2a"
-dependencies = [
- "serde",
-]
 
 [[package]]
 name = "serde"
@@ -2847,17 +2750,6 @@ dependencies = [
  "syn",
 ]
 
-[[package]]
-name = "serde_json"
-version = "1.0.94"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "1c533a59c9d8a93a09c6ab31f0fd5e5f4dd1b8fc9434804029839884765d04ea"
-dependencies = [
- "itoa",
- "ryu",
- "serde",
-]
-
 [[package]]
 name = "serde_repr"
 version = "0.1.11"
@@ -2915,21 +2807,6 @@ version = "0.3.10"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "7bd3e3206899af3f8b12af284fafc038cc1dc2b41d1b89dd17297221c5d225de"
 
-[[package]]
-name = "skeptic"
-version = "0.13.7"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "16d23b015676c90a0f01c197bfdc786c20342c73a0afdda9025adb0bc42940a8"
-dependencies = [
- "bytecount",
- "cargo_metadata",
- "error-chain",
- "glob",
- "pulldown-cmark",
- "tempfile",
- "walkdir",
-]
-
 [[package]]
 name = "slab"
 version = "0.4.8"
@@ -3008,19 +2885,6 @@ version = "0.10.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "73473c0e59e6d5812c5dfe2a064a6444949f089e20eec9a2e5506596494e4623"
 
-[[package]]
-name = "svg_metadata"
-version = "0.4.4"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "a42f3d2c6f543a8c57cdb434697334f33faf56f9acc141e2bbe4bb1994055200"
-dependencies = [
- "doc-comment",
- "lazy_static",
- "regex",
- "roxmltree",
- "skeptic",
-]
-
 [[package]]
 name = "syn"
 version = "1.0.109"
@@ -3213,15 +3077,6 @@ dependencies = [
  "winapi",
 ]
 
-[[package]]
-name = "unicase"
-version = "2.6.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "50f37be617794602aabbeee0be4f259dc1778fabe05e2d67ee8f79326d5cb4f6"
-dependencies = [
- "version_check",
-]
-
 [[package]]
 name = "unicode-bidi"
 version = "0.3.10"
@@ -3295,17 +3150,6 @@ version = "1.1.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "9d5b2c62b4012a3e1eca5a7e077d13b3bf498c4073e33ccd58626607748ceeca"
 
-[[package]]
-name = "walkdir"
-version = "2.3.2"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "808cf2735cd4b6866113f648b791c6adc5714537bc222d9347bb203386ffda56"
-dependencies = [
- "same-file",
- "winapi",
- "winapi-util",
-]
-
 [[package]]
 name = "wasi"
 version = "0.9.0+wasi-snapshot-preview1"
@@ -3551,12 +3395,6 @@ dependencies = [
  "markup5ever",
 ]
 
-[[package]]
-name = "xmlparser"
-version = "0.13.5"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "4d25c75bf9ea12c4040a97f829154768bbbce366287e2dc044af160cd79a13fd"
-
 [[package]]
 name = "zbus"
 version = "3.11.0"

Cargo.toml

@@ -24,7 +24,6 @@ log = "0.4.17"
 once_cell = "1.16.0"
 pretty_env_logger = "0.4.0"
 safe-transmute = "0.11.2"
-svg_metadata = "0.4.3"
 url = "2.3.1"
 
 # decoders

data/gtk/properties_view.ui

@@ -59,7 +59,11 @@
                     <property name="title" translatable="yes">Image Size</property>
                     <property name="subtitle-selectable">True</property>
                     <binding name="subtitle">
-                      <lookup name="dimensions">LpPropertiesView</lookup>
+                      <closure type="gchararray" function="image_size">
+                        <lookup type="LpImage" name="image-size">
+                          <lookup name="image">LpPropertiesView</lookup>
+                         </lookup>
+                      </closure>
                     </binding>
                     <style>
                       <class name="property"/>

src/decoder/formats/mod.rs

@@ -33,3 +33,10 @@ impl std::fmt::Display for ImageFormat {
         }
     }
 }
+
+#[derive(Clone, Debug, Default)]
+pub enum ImageDimensionDetails {
+    Svg((rsvg::Length, rsvg::Length)),
+    #[default]
+    None,
+}

src/decoder/formats/svg.rs

@@ -58,7 +58,11 @@ impl Svg {
 
             let (original_width, original_height) = svg_dimensions(&renderer);
 
-            tiles.set_original_dimensions((original_width, original_height));
+            let intrisic_dimensions = renderer.intrinsic_dimensions();
+            tiles.set_original_dimensions_full(
+                (original_width, original_height),
+                ImageDimensionDetails::Svg((intrisic_dimensions.width, intrisic_dimensions.height)),
+            );
 
             loop {
                 let tile_request = {

src/decoder/mod.rs

@@ -2,6 +2,8 @@
 mod formats;
 pub mod tiling;
 
+pub use formats::ImageDimensionDetails;
+
 use crate::image_metadata::ImageMetadata;
 use formats::*;
 use tiling::TilingStoreExt;
@@ -36,7 +38,7 @@ pub struct UpdateSender {
 /// Signals for renderer
 pub enum DecoderUpdate {
     /// Dimensions of image in `TilingSore` available/updated
-    Dimensions,
+    Dimensions(ImageDimensionDetails),
     /// Metadata available
     Metadata(ImageMetadata),
     /// Image format determined

src/decoder/tiling.rs

@@ -8,7 +8,7 @@
 ///! be re-genearted for each zoom level. It can also be used to allow
 ///! showing large JPEGs etc where the complete decoded image would not
 ///! even fit in the VRAM.
-use super::{DecoderUpdate, UpdateSender};
+use super::{DecoderUpdate, ImageDimensionDetails, UpdateSender};
 use crate::deps::*;
 
 use arc_swap::ArcSwap;
@@ -134,9 +134,17 @@ impl TilingStore {
     }
 
     pub fn set_original_dimensions(&mut self, dimensions: Coordinates) {
+        self.set_original_dimensions_full(dimensions, Default::default());
+    }
+
+    pub fn set_original_dimensions_full(
+        &mut self,
+        dimensions: Coordinates,
+        dimension_details: ImageDimensionDetails,
+    ) {
         self.original_dimensions = Some(dimensions);
         if let Some(updater) = &self.update_sender {
-            updater.send(DecoderUpdate::Dimensions);
+            updater.send(DecoderUpdate::Dimensions(dimension_details));
         }
     }
 
@@ -335,6 +343,11 @@ impl RectExt for graphene::Rect {
 pub trait TilingStoreExt {
     fn push(&self, tile: Tile);
     fn set_original_dimensions(&self, size: Coordinates);
+    fn set_original_dimensions_full(
+        &self,
+        size: Coordinates,
+        dimension_details: ImageDimensionDetails,
+    );
     fn set_update_sender(&self, sender: UpdateSender);
 }
 
@@ -355,6 +368,18 @@ impl TilingStoreExt for ArcSwap<TilingStore> {
         });
     }
 
+    fn set_original_dimensions_full(
+        &self,
+        size: Coordinates,
+        dimension_details: ImageDimensionDetails,
+    ) {
+        self.rcu(|tiling_store| {
+            let mut new_store = (**tiling_store).clone();
+            new_store.set_original_dimensions_full(size, dimension_details.clone());
+            Arc::new(new_store)
+        });
+    }
+
     fn set_update_sender(&self, sender: UpdateSender) {
         self.rcu(|tiling_store| {
             let mut new_store = (**tiling_store).clone();

src/widgets/image.rs

@@ -126,6 +126,8 @@ mod imp {
 
         /// Currently EXIF data
         pub image_metadata: RefCell<LpImageMetadata>,
+        /// Image dimension details for SVGs
+        pub dimension_details: RefCell<decoder::ImageDimensionDetails>,
 
         /// Current pointer position
         pub pointer_position: Cell<Option<(f64, f64)>>,
@@ -692,8 +694,9 @@ impl LpImage {
 
                 self.reset_rotation();
             }
-            DecoderUpdate::Dimensions => {
+            DecoderUpdate::Dimensions(dimension_details) => {
                 log::debug!("Received dimensions: {:?}", self.original_dimensions());
+                self.imp().dimension_details.replace(dimension_details);
                 self.notify("image-size");
                 self.configure_best_fit();
                 self.request_tiles();
@@ -1428,6 +1431,10 @@ impl LpImage {
         self.imp().image_metadata.borrow().clone()
     }
 
+    pub fn dimension_details(&self) -> decoder::ImageDimensionDetails {
+        self.imp().dimension_details.borrow().clone()
+    }
+
     /// Drag and drop content provider
     pub fn content_provider(&self) -> Option<gdk::ContentProvider> {
         let file = self.file()?;

src/widgets/properties_view.rs

@@ -17,6 +17,7 @@
 //
 // SPDX-License-Identifier: GPL-3.0-or-later
 
+use crate::decoder::ImageDimensionDetails;
 use crate::deps::*;
 use crate::util;
 
@@ -25,7 +26,6 @@ use adw::subclass::prelude::*;
 use glib::clone;
 use gtk::CompositeTemplate;
 
-use anyhow::Context;
 use futures::future::{AbortHandle, Abortable};
 use once_cell::sync::Lazy;
 use std::cell::RefCell;
@@ -49,10 +49,7 @@ mod imp {
         pub metadata: RefCell<LpImageMetadata>,
 
         pub file_info: RefCell<Option<gio::FileInfo>>,
-        pub dimensions: RefCell<Option<String>>,
-
         pub info_handle: RefCell<Option<AbortHandle>>,
-        pub dimensions_handle: RefCell<Option<AbortHandle>>,
     }
 
     #[glib::object_subclass]
@@ -94,11 +91,6 @@ mod imp {
                         .read_only()
                         .explicit_notify()
                         .build(),
-                    glib::ParamSpecString::builder("dimensions")
-                        .default_value(Some(FALLBACK))
-                        .read_only()
-                        .explicit_notify()
-                        .build(),
                     glib::ParamSpecObject::builder::<LpImage>("image")
                         .explicit_notify()
                         .build(),
@@ -120,7 +112,6 @@ mod imp {
                 "image" => obj.image().to_value(),
                 "metadata" => self.metadata.borrow().to_value(),
                 "file-info" => obj.file_info().to_value(),
-                "dimensions" => self.dimensions.borrow().to_value(),
                 _ => unimplemented!("Failed to get property \"{prop_name}\""),
             }
         }
@@ -174,10 +165,6 @@ impl LpPropertiesView {
                 handle.abort();
             }
 
-            if let Some(handle) = imp.dimensions_handle.take() {
-                handle.abort();
-            }
-
             self.build_file_info(file);
         }
 
@@ -228,7 +215,6 @@ impl LpPropertiesView {
                 let info_result = util::query_attributes_future(
                     &file,
                     vec![
-                        gio::FILE_ATTRIBUTE_STANDARD_CONTENT_TYPE,
                         gio::FILE_ATTRIBUTE_STANDARD_SIZE,
                         gio::FILE_ATTRIBUTE_TIME_CREATED,
                         gio::FILE_ATTRIBUTE_TIME_MODIFIED,
@@ -237,7 +223,7 @@ impl LpPropertiesView {
                 .await;
 
                 match info_result {
-                    Ok(info) => view.on_info_loaded(info, &file),
+                    Ok(info) => view.on_info_loaded(info),
                     Err(err) => log::warn!("Failed to load file info: {err}"),
                 }
             }),
@@ -262,78 +248,12 @@ impl LpPropertiesView {
     // This is where we handle the results of the future.
     // This could technically be part of the future, but we keep
     // them separate for a bit of cleanliness.
-    fn on_info_loaded(&self, info: gio::FileInfo, file: &gio::File) {
+    fn on_info_loaded(&self, info: gio::FileInfo) {
         let imp = self.imp();
-        self.build_dimensions(&info, file);
         imp.file_info.replace(Some(info));
         self.notify("file-info");
     }
 
-    // Determines the dimensions of the file asynchronously. We do this
-    // asynchronously to avoid blocking the UI as we parse the file info.
-    // In addition, we're loading the image metadata separately from the file
-    // here so that the properties sidebar's information can load separately
-    // from the current file.
-    fn build_dimensions(&self, info: &gio::FileInfo, file: &gio::File) {
-        let imp = self.imp();
-
-        // Here we do the same thing we do for the rest of the file info.
-        let (handle, reg) = AbortHandle::new_pair();
-        let fut = Abortable::new(
-            clone!(@weak self as view, @weak file, @weak info => async move {
-                let is_svg = info.content_type()
-                    .map(|t| t.to_string())
-                    .map(|t| t == "image/svg+xml")
-                    .unwrap_or_default();
-
-                let path = file.peek_path();
-                if path.is_none() {
-                    return;
-                }
-
-                let dimensions = if is_svg {
-                    parse_svg_file_future(path.unwrap())
-                        .await
-                        .ok()
-                        .and_then(|m| {
-                            match (m.width(), m.height()) {
-                                (Some(w), Some(h)) => Some((w, h)),
-                                _ => None::<(f64, f64)>,
-                            }
-                        })
-                        .map(|(w, h)| {
-                            if w.fract() != 0.0 || h.fract() != 0.0 {
-                                format!("{w:.1} \u{D7} {h:.1}")
-                            } else {
-                                format!("{w} \u{D7} {h}")
-                            }
-                        })
-                } else {
-                    gdk_pixbuf::Pixbuf::file_info_future(path.unwrap())
-                        .await
-                        .ok()
-                        .flatten()
-                        .map(|(_, w, h)| format!("{w} \u{D7} {h}"))
-                };
-
-                view.on_dimensions_loaded(dimensions.unwrap_or_else(|| FALLBACK.to_string()));
-            }),
-            reg,
-        );
-
-        imp.dimensions_handle.replace(Some(handle));
-
-        spawn!(async {
-            let _ = fut.await;
-        });
-    }
-
-    fn on_dimensions_loaded(&self, dimensions: String) {
-        let imp = self.imp();
-        imp.dimensions.replace(Some(dimensions));
-        self.notify("dimensions");
-    }
-
     async fn open_directory(&self) -> anyhow::Result<()> {
         let launcher = gtk::FileLauncher::new(self.file().as_ref());
         let win = self.native().and_downcast::<gtk::Window>();
@@ -380,6 +300,49 @@ impl LpPropertiesView {
             .unwrap_or_else(|| FALLBACK.to_owned()) // and if we get nothing, use `FALLBACK`
     }
 
+    #[template_callback]
+    fn image_size(&self, _image_size: Option<glib::Variant>) -> String {
+        if let Some(image) = self.image() {
+            match image.dimension_details() {
+                ImageDimensionDetails::Svg((width, height))
+                    if width.unit != rsvg::LengthUnit::Px
+                        && height.unit != rsvg::LengthUnit::Px =>
+                {
+                    // Percent is not stored as percentile
+                    let width_factor = if width.unit == rsvg::LengthUnit::Percent {
+                        100.
+                    } else {
+                        1.
+                    };
+                    let height_factor = if height.unit == rsvg::LengthUnit::Percent {
+                        100.
+                    } else {
+                        1.
+                    };
+
+                    // Only show two digits
+                    let width_n = (width.length * width_factor * 100.).round() / 100.;
+                    let height_n = (height.length * height_factor * 100.).round() / 100.;
+
+                    let width_unit = width.unit;
+                    let height_unit = height.unit;
+
+                    format!("{width_n}\u{202F}{width_unit} \u{D7} {height_n}\u{202F}{height_unit}")
+                }
+                _ => {
+                    let (width, height) = image.image_size();
+                    if width > 0 && height > 0 {
+                        format!("{width}\u{202F}\u{D7}\u{202F}{height}")
+                    } else {
+                        FALLBACK.to_owned()
+                    }
+                }
+            }
+        } else {
+            FALLBACK.to_owned()
+        }
+    }
+
     #[template_callback]
     fn file_size(&self, info: Option<gio::FileInfo>) -> String {
         info.map(|info| {
@@ -410,16 +373,3 @@ impl LpPropertiesView {
         content.map_or(false, |x| !x.is_empty())
     }
 }
-
-async fn parse_svg_file_future(path: std::path::PathBuf) -> anyhow::Result<svg_metadata::Metadata> {
-    let (sender, receiver) = futures_channel::oneshot::channel();
-
-    let _ = std::thread::Builder::new()
-        .name("Parse SVG".to_string())
-        .spawn(move || {
-            let result = svg_metadata::Metadata::parse_file(path);
-            let _ = sender.send(result);
-        });
-
-    receiver.await.unwrap().context("Could not load metadata")
-}