scale width/height to match SVG aspect ratio if undefined

- now able to access the viewbox and using it to set the height to 150 and width to the appropriate proportion to match the original aspect ratio (i.e., Chrome's behavior)

CHANGELOG.md

@@ -10,7 +10,7 @@
 
 #### Imagery
 - Added initial SVG rendering support. **Image**s can now load SVG files and can be drawn in a resolution-independent manner via [`drawImage()`][mdn_drawImage] (thanks to @mpaperno #180). Note that **Image**s loaded from SVG files that don't have a `width` and `height` set on their root `<svg>` element have some quirks as of this release:
-  - The **Image** object's `width` and `height` will both (misleadingly) report to be `150`.
+  - The **Image** object's `height` will report being `150` and the `width` will be set to accurately capture the image's aspect ratio
   - When passed to `drawImage()` without size arguments, the SVG will be scaled to a size that fits within the **Canvas**'s current bounds (using an approach akin to CSS's `object-fit: contain`).
   - When using the 9-argument version of `drawImage()`, the ‘crop’ arguments (`sx`, `sy`, `sWidth`, & `sHeight`) will correspond to this scaled-to-fit size, *not* the **Image**'s reported `width` & `height`.
 - WEBP support

docs/api/image.md

@@ -75,7 +75,7 @@ The images you load can be from a variety of formats:
 In the browser these are writable properties that can control the display size of the image within the HTML page. But the context's [`drawImage`][drawImage()] method ignores them in favor of the image's intrinsic size. As a result, Skia Canvas doesn't let you overwrite the `width` and `height` properties (since it would have no effect anyway) and provides them as read-only values derived from the image data.
 
 :::info[Note]
-When loading an image from an SVG file, the intrinsic size may not be defined since the root `<svg>` element is not required to have a defined `width` and `height`. At the moment, Skia Canvas simply returns 150×150 as the image size regardless of the image's aspect ratio. In a future release, these dimensions will be adjusted to reflect the aspect ratio of the SVG's `viewbox` property.
+When loading an image from an SVG file, the intrinsic size may not be defined since the root `<svg>` element is not required to have a defined `width` and `height`. In these cases, the Image will be set to a fixed height of `150` and its width will be scaled to a value that matches the aspect ratio of the SVG's `viewbox` size.
 :::
 
 
@@ -120,9 +120,6 @@ img.decode().then(({width, height}) =>
 )
 ```
 
-
-
-
 ### `on()` / `off()` / `once()`
 
 ```js returns="Image"

lib/classes/context.js

@@ -182,7 +182,7 @@ class CanvasRenderingContext2D extends RustClass{
       this.ƒ('drawImage', core(image.getContext('2d')), ...coords)
     }else if (image instanceof Image){
       if (image.complete) this.ƒ('drawImage', core(image), ...coords)
-      else throw new Error("Image has not completed loading: listen for `load` event or `await` decode() first")
+      else throw new Error("Image has not completed loading: listen for `load` event or await `decode()` first")
     }else if (image instanceof ImageData){
       this.ƒ('drawImage', image, ...coords)
     }else if (image instanceof Promise) {

src/image.rs

@@ -6,8 +6,9 @@ use std::cell::RefCell;
 use neon::{prelude::*, types::buffer::TypedArray};
 use skia_safe::{
   Image as SkImage, ImageInfo, ISize, ColorType, ColorSpace, AlphaType, Data, Size,
-  FontMgr, Picture, PictureRecorder, Rect, image::images, svg,
-  wrapper::PointerWrapper // for SVG Dom access, temporary until next skia-safe update
+  FontMgr, Picture, PictureRecorder, Rect, image::images,
+  svg::{self, Length, LengthUnit},
+  // wrapper::PointerWrapper // for SVG Dom access, temporary until next skia-safe update
 };
 use crate::utils::*;
 use crate::context::Context2D;
@@ -177,53 +178,43 @@ pub fn set_src(mut cx: FunctionContext) -> JsResult<JsUndefined> {
 pub fn set_data(mut cx: FunctionContext) -> JsResult<JsBoolean> {
   let this = cx.argument::<BoxedImage>(0)?;
   let mut this = this.borrow_mut();
-
   let buffer = cx.argument::<JsBuffer>(1)?;
   let data = Data::new_copy(buffer.as_slice(&cx));
 
-  // First try decoding the data as a bitmap
-  // If it's not recognized, try parsing as SVG and create a picture if it is valid
+  // First try decoding the data as a bitmap, if invalid try parsing as SVG
   if let Some(image) = images::deferred_from_encoded_data(&data, None){
     this.content = Content::Bitmap(image);
   }else if let Ok(mut dom) = svg::Dom::from_bytes(&data, FONT_LIBRARY.lock().unwrap().font_mgr()){
-    // Get the intrinsic size of the `svg` root element as specified in the width/height attributes, if any.
-    // So far skia-safe doesn't provide direct access to the needed methods, so we have to go direct to the source.
-    let i_size = unsafe { *dom.inner().containerSize() };  // skia_bindings::SkSize
-    // let i_size = dom.inner().fContainerSize;  // "safe" but this is using a private member of the C++ class (somehow... skia-"safe" :-P )
-    // TODO: Switch to these once available in skia-safe 0.79+
-    // let mut root = dom.root();
-    // let i_size = root.intrinsic_size();
-
-    // Set a flag to indicate that the image doesn't have its own intrinsic size.
-    // This may be used at drawing time if user doesn't specify a size in `drawImage()`,
-    // in which case the the canvas' size will be used as the image size.
-    // This is a "complication" to match Chrome's behavior... one could argue that it should
-    // just be drawn at the default size (set below). Which is what FF does (though that has its own anomalies).
-    let mut bounds = Rect::from_wh(i_size.fWidth, i_size.fHeight);
-    this.autosized = bounds.is_empty();
-
-    // Check if width/height are valid attribute values in the root `<svg>` element.
-    // If w/h aren't specified in an SVG (which is not uncommon), both Chrome and FF will:
-    //  - If only one dimension is missing then use the same size for both;
-    //  - If both are missing then assign a default of 150 (which seems arbitrary but I guess as good as any);
-    // `Dom::containerSize()` will return zero for both width and height if _either_ attribute is missing from `<svg>`.
-    // This seems a bit suspicious (as in may change in future?), so in the interest of paranoia let's check them individually.
-    // TODO: See if we can get actual width/height attribute values from DOM with skia-safe 0.79+
-    (bounds.right, bounds.bottom) = match (bounds.width(), bounds.height()){
-      (0.0, 0.0) => (150.0, 150.0),
-      (width, 0.0) => (width, width),
-      (0.0, height) => (height, height),
-      (width, height) => (width, height)
+    let mut root = dom.root();
+
+    let mut size = root.intrinsic_size();
+    if size.is_empty(){
+      // flag that image lacks an intrinsic size so it will be drawn to match the canvas size
+      // if dimensions aren't provided in the drawImage() call
+      this.autosized = true;
+
+      // If width or height attributes aren't defined on the root `<svg>` element, they will be reported as "100%".
+      // If only one is defined, use it for both dimensions, and if both are missing use the aspect ratio to scale the
+      // width vs a fixed height of 150 (i.e., Chrome's behavior)
+      let Length{ value:width, unit:w_unit } = root.width();
+      let Length{ value:height, unit:h_unit } = root.height();
+      size = match ((width, w_unit), (height, h_unit)){
+        // NB: only unitless numeric lengths are currently being handled; values in em, cm, in, etc. are ignored,
+        // but perhaps they should be converted to px?
+        ((100.0, LengthUnit::Percentage), (height, LengthUnit::Number)) => (*height, *height).into(),
+        ((width, LengthUnit::Number),     (100.0,  LengthUnit::Percentage)) => (*width, *width).into(),
+        _ => {
+          let aspect = root.view_box().map(|vb| vb.width()/vb.height()).unwrap_or(1.0);
+          (150.0 * aspect, 150.0).into()
+        }
+      };
     };
-    dom.set_container_size(bounds.size());
 
-    // Save the image as a Picture so it can be scaled properly later.
+    // Save the SVG contents as a Picture (to be drawn later)
+    let bounds = Rect::from_size(size);
     let mut compositor = PictureRecorder::new();
-    compositor.begin_recording(bounds, None);
-    if let Some(canvas) = compositor.recording_canvas() {
-      dom.render(canvas);
-    }
-
+    dom.set_container_size(bounds.size());
+    dom.render(compositor.begin_recording(bounds, None));
     if let Some(picture) = compositor.finish_recording_as_picture(Some(&bounds)){
       this.content = Content::Vector(picture);
     }