Improve sixtyfps::Image::size() API for consistency

Make size() return an unsigned integer size type instead of floats.

cc #431

CHANGELOG.md

@@ -16,6 +16,7 @@ This version changes some APIs in incompatible ways. For details how to migrate
  - The interpreter does not differentiate anymore between `Value::Array` and `Value::Model`
    everything is a `Value::Model`, which now contains a `ModelHandle`
  - In Rust, `sixtyfps::SharedPixelBuffer` and `sixtyfps::SharedImageBuffer` now use a `u32` instead of `usize` for `width`, `height` and `stride`.
+ - In Rust and C++, `sixtyfps::Image::size()` now returns an integer size type.
 
 ### Added
 

api/sixtyfps-cpp/include/sixtyfps_image.h

@@ -10,16 +10,16 @@
 namespace sixtyfps {
 
 #if !defined(DOXYGEN)
-using cbindgen_private::types::Size;
+using cbindgen_private::types::IntSize;
 #else
 /// The Size structure is used to represent a two-dimensional size
 /// with width and height.
-struct Size
+struct IntSize
 {
     /// The width of the size
-    float width;
+    uint32_t width;
     /// The height of the size
-    float height;
+    uint32_t height;
 };
 #endif
 
@@ -46,7 +46,7 @@ public:
     */
 
     /// Returns the size of the Image in pixels.
-    Size size() const { return cbindgen_private::types::sixtyfps_image_size(&data); }
+    IntSize size() const { return cbindgen_private::types::sixtyfps_image_size(&data); }
 
     /// Returns the path of the image on disk, if it was constructed via Image::load_from_path().
     std::optional<sixtyfps::SharedString> path() const

sixtyfps_runtime/corelib/backend.rs

@@ -9,7 +9,7 @@ use alloc::boxed::Box;
 use alloc::rc::Rc;
 use alloc::string::String;
 
-use crate::graphics::{Image, Size};
+use crate::graphics::{Image, IntSize};
 use crate::window::Window;
 
 #[cfg(feature = "std")]
@@ -63,7 +63,7 @@ pub trait Backend: Send + Sync {
     /// Send an user event to from another thread that should be run in the GUI event loop
     fn post_event(&'static self, event: Box<dyn FnOnce() + Send>);
 
-    fn image_size(&'static self, image: &Image) -> Size;
+    fn image_size(&'static self, image: &Image) -> IntSize;
 
     fn duration_since_start(&'static self) -> core::time::Duration {
         #[cfg(feature = "std")]

sixtyfps_runtime/corelib/graphics/image.rs

@@ -35,6 +35,11 @@ impl<Pixel> SharedPixelBuffer<Pixel> {
         self.height
     }
 
+    /// Returns the size of the image in pixels.
+    pub fn size(&self) -> IntSize {
+        [self.width, self.height].into()
+    }
+
     /// Returns the number of pixels per line.
     pub fn stride(&self) -> u32 {
         self.stride
@@ -158,6 +163,16 @@ impl SharedImageBuffer {
             Self::RGBA8Premultiplied(buffer) => buffer.height(),
         }
     }
+
+    /// Returns the size of the image in pixels.
+    #[inline]
+    pub fn size(&self) -> IntSize {
+        match self {
+            Self::RGB8(buffer) => buffer.size(),
+            Self::RGBA8(buffer) => buffer.size(),
+            Self::RGBA8Premultiplied(buffer) => buffer.size(),
+        }
+    }
 }
 
 impl PartialEq for SharedImageBuffer {
@@ -368,7 +383,7 @@ impl Image {
     }
 
     /// Returns the size of the Image in pixels.
-    pub fn size(&self) -> crate::graphics::Size {
+    pub fn size(&self) -> IntSize {
         match &self.0 {
             ImageInner::None => Default::default(),
             ImageInner::AbsoluteFilePath(_) |  ImageInner::EmbeddedData { .. } => {
@@ -377,8 +392,8 @@ impl Image {
                     None => panic!("sixtyfps::Image::size() called too early (before a graphics backend was chosen). You need to create a component first."),
                 }
             },
-            ImageInner::EmbeddedImage(buffer) => [buffer.width() as _, buffer.height() as _].into(),
+            ImageInner::EmbeddedImage(buffer) => buffer.size(),
-            ImageInner::StaticTextures{size, ..} => size.cast(),
+            ImageInner::StaticTextures{size, ..} => size.clone(),
 
         }
     }
@@ -411,7 +426,7 @@ fn test_image_size_from_buffer_without_backend() {
     {
         let buffer = SharedPixelBuffer::<Rgb8Pixel>::new(320, 200);
         let image = Image::from_rgb8(buffer);
-        assert_eq!(image.size(), [320., 200.].into())
+        assert_eq!(image.size(), [320, 200].into())
     }
 }
 
@@ -419,7 +434,7 @@ fn test_image_size_from_buffer_without_backend() {
 pub(crate) mod ffi {
     #![allow(unsafe_code)]
 
-    use super::super::Size;
+    use super::super::IntSize;
     use super::*;
 
     /// Expand Rgb8Pixel so that cbindgen can see it. (is in fact rgb::RGB<u8>)
@@ -441,7 +456,7 @@ pub(crate) mod ffi {
     }
 
     #[no_mangle]
-    pub unsafe extern "C" fn sixtyfps_image_size(image: &Image) -> Size {
+    pub unsafe extern "C" fn sixtyfps_image_size(image: &Image) -> IntSize {
         image.size()
     }
 

sixtyfps_runtime/corelib/items/image.rs

@@ -78,9 +78,11 @@ impl Item for ImageItem {
         let natural_size = self.source().size();
         LayoutInfo {
             preferred: match orientation {
-                _ if natural_size.width == 0. || natural_size.height == 0. => 0.,
+                _ if natural_size.width == 0 || natural_size.height == 0 => 0.,
-                Orientation::Horizontal => natural_size.width,
+                Orientation::Horizontal => natural_size.width as f32,
-                Orientation::Vertical => natural_size.height * self.width() / natural_size.width,
+                Orientation::Vertical => {
+                    natural_size.height as f32 * self.width() / natural_size.width as f32
+                }
             },
             ..Default::default()
         }
@@ -153,9 +155,11 @@ impl Item for ClippedImage {
         let natural_size = self.source().size();
         LayoutInfo {
             preferred: match orientation {
-                _ if natural_size.width == 0. || natural_size.height == 0. => 0.,
+                _ if natural_size.width == 0 || natural_size.height == 0 => 0.,
-                Orientation::Horizontal => natural_size.width,
+                Orientation::Horizontal => natural_size.width as f32,
-                Orientation::Vertical => natural_size.height * self.width() / natural_size.width,
+                Orientation::Vertical => {
+                    natural_size.height as f32 * self.width() / natural_size.width as f32
+                }
             },
             ..Default::default()
         }

sixtyfps_runtime/rendering_backends/gl/images.rs

@@ -5,7 +5,7 @@ use std::cell::RefCell;
 use std::collections::HashMap;
 use std::rc::Rc;
 
-use sixtyfps_corelib::graphics::{SharedImageBuffer, Size};
+use sixtyfps_corelib::graphics::{IntSize, SharedImageBuffer};
 #[cfg(target_arch = "wasm32")]
 use sixtyfps_corelib::Property;
 use sixtyfps_corelib::{items::ImageRendering, slice::Slice, ImageInner, SharedString};
@@ -18,11 +18,11 @@ struct Texture {
 }
 
 impl Texture {
-    fn size(&self) -> Option<Size> {
+    fn size(&self) -> Option<IntSize> {
         self.canvas
             .borrow()
             .image_info(self.id)
-            .map(|info| [info.width() as f32, info.height() as f32].into())
+            .map(|info| [info.width() as u32, info.height() as u32].into())
             .ok()
     }
 }
@@ -77,10 +77,10 @@ impl HTMLImage {
         Self { dom_element, image_load_pending }
     }
 
-    fn size(&self) -> Option<Size> {
+    fn size(&self) -> Option<IntSize> {
         match self.image_load_pending.as_ref().get() {
             true => None,
-            false => Some(Size::new(self.dom_element.width() as _, self.dom_element.height() as _)),
+            false => Some(IntSize::new(self.dom_element.width(), self.dom_element.height())),
         }
     }
 }
@@ -137,15 +137,15 @@ impl CachedImage {
         Self(RefCell::new(Texture { id: image_id, canvas: canvas.clone() }.into()))
     }
 
-    pub fn new_empty_on_gpu(canvas: &CanvasRc, width: usize, height: usize) -> Option<Self> {
+    pub fn new_empty_on_gpu(canvas: &CanvasRc, width: u32, height: u32) -> Option<Self> {
         if width == 0 || height == 0 {
             return None;
         }
         let image_id = canvas
             .borrow_mut()
             .create_image_empty(
-                width,
+                width as usize,
-                height,
+                height as usize,
                 femtovg::PixelFormat::Rgba8,
                 femtovg::ImageFlags::PREMULTIPLIED | femtovg::ImageFlags::FLIP_Y,
             )
@@ -337,23 +337,18 @@ impl CachedImage {
         }
     }
 
-    pub fn size(&self) -> Option<Size> {
+    pub fn size(&self) -> Option<IntSize> {
         use image::GenericImageView;
 
         match &*self.0.borrow() {
             ImageData::Texture(texture) => texture.size(),
-            ImageData::DecodedImage(decoded_image) => {
+            ImageData::DecodedImage(decoded_image) => Some(decoded_image.dimensions().into()),
-                let (width, height) = decoded_image.dimensions();
+            ImageData::EmbeddedImage(buffer) => Some(buffer.size()),
-                Some([width as f32, height as f32].into())
-            }
-            ImageData::EmbeddedImage(buffer) => {
-                Some([buffer.width() as f32, buffer.height() as f32].into())
-            }
 
             #[cfg(feature = "svg")]
             ImageData::Svg(tree) => {
                 let size = tree.svg_node().size.to_screen_size();
-                Some([size.width() as f32, size.height() as f32].into())
+                Some([size.width(), size.height()].into())
             }
 
             #[cfg(target_arch = "wasm32")]
@@ -377,12 +372,7 @@ impl CachedImage {
         }
         .expect("internal error: Cannot filter non-GPU images");
 
-        let filtered_image = Self::new_empty_on_gpu(
+        let filtered_image = Self::new_empty_on_gpu(canvas, size.width, size.height).expect(
-            canvas,
-            size.width.ceil() as usize,
-            size.height.ceil() as usize,
-        )
-        .expect(
             "internal error: this can only fail if the filtered image was zero width or height",
         );
 
@@ -402,7 +392,7 @@ impl CachedImage {
                 let size = tex
                     .size()
                     .expect("internal error: CachedImage::as_paint() called on zero-sized texture");
-                femtovg::Paint::image(tex.id, 0., 0., size.width, size.height, 0., 1.)
+                femtovg::Paint::image(tex.id, 0., 0., size.width as f32, size.height as f32, 0., 1.)
             }
             _ => panic!("internal error: CachedImage::as_paint() called on non-texture image"),
         }

sixtyfps_runtime/rendering_backends/gl/lib.rs

@@ -20,7 +20,7 @@ use std::rc::Rc;
 use euclid::approxeq::ApproxEq;
 use event_loop::WinitWindow;
 use sixtyfps_corelib::graphics::{
-    Brush, Color, Image, ImageInner, IntRect, Point, Rect, RenderingCache, Size,
+    Brush, Color, Image, ImageInner, IntRect, IntSize, Point, Rect, RenderingCache, Size,
 };
 use sixtyfps_corelib::item_rendering::{CachedRenderingData, ItemRenderer};
 use sixtyfps_corelib::items::{FillRule, ImageFit, ImageRendering};
@@ -546,8 +546,8 @@ impl ItemRenderer for GLItemRenderer {
                     let shadow_rect: euclid::Rect<f32, euclid::UnknownUnit> =
                         euclid::rect(0., 0., width + 2. * blur, height + 2. * blur);
 
-                    let shadow_image_width = shadow_rect.width().ceil() as usize;
+                    let shadow_image_width = shadow_rect.width().ceil() as u32;
-                    let shadow_image_height = shadow_rect.height().ceil() as usize;
+                    let shadow_image_height = shadow_rect.height().ceil() as u32;
 
                     let shadow_image = CachedImage::new_empty_on_gpu(
                         &self.canvas,
@@ -632,7 +632,12 @@ impl ItemRenderer for GLItemRenderer {
         let shadow_image_paint = shadow_image.as_paint();
 
         let mut shadow_image_rect = femtovg::Path::new();
-        shadow_image_rect.rect(0., 0., shadow_image_size.width, shadow_image_size.height);
+        shadow_image_rect.rect(
+            0.,
+            0.,
+            shadow_image_size.width as f32,
+            shadow_image_size.height as f32,
+        );
 
         self.canvas.borrow_mut().save_with(|canvas| {
             let blur = box_shadow.blur() * self.scale_factor;
@@ -844,13 +849,15 @@ impl GLItemRenderer {
             .canvas
             .borrow_mut()
             .create_image_empty(
-                image_size.width as _,
+                image_size.width as usize,
-                image_size.height as _,
+                image_size.height as usize,
                 femtovg::PixelFormat::Rgba8,
                 femtovg::ImageFlags::PREMULTIPLIED | scaling_flags,
             )
             .expect("internal error allocating temporary texture for image colorization");
 
+        let image_size: Size = image_size.cast();
+
         let mut image_rect = femtovg::Path::new();
         image_rect.rect(0., 0., image_size.width, image_size.height);
         let brush_paint = self.brush_to_paint(colorize_brush, &mut image_rect).unwrap();
@@ -977,7 +984,7 @@ impl GLItemRenderer {
         };
 
         let image_id = cached_image.ensure_uploaded_to_gpu(self, Some(image_rendering));
-        let image_size = cached_image.size().unwrap_or_default();
+        let image_size = cached_image.size().unwrap_or_default().cast();
 
         let (source_width, source_height) = if source_clip_rect.is_empty() {
             (image_size.width, image_size.height)
@@ -1261,7 +1268,7 @@ impl sixtyfps_corelib::backend::Backend for Backend {
         }
     }
 
-    fn image_size(&'static self, image: &Image) -> Size {
+    fn image_size(&'static self, image: &Image) -> IntSize {
         IMAGE_CACHE.with(|image_cache| {
             image_cache
                 .borrow_mut()

sixtyfps_runtime/rendering_backends/mcu/lib.rs

@@ -296,17 +296,15 @@ mod the_backend {
         fn image_size(
             &'static self,
             image: &sixtyfps_corelib::graphics::Image,
-        ) -> sixtyfps_corelib::graphics::Size {
+        ) -> sixtyfps_corelib::graphics::IntSize {
             let inner: &ImageInner = image.into();
             match inner {
                 ImageInner::None => Default::default(),
                 ImageInner::AbsoluteFilePath(_) | ImageInner::EmbeddedData { .. } => {
                     unimplemented!()
                 }
-                ImageInner::EmbeddedImage(buffer) => {
+                ImageInner::EmbeddedImage(buffer) => buffer.size(),
-                    [buffer.width() as f32, buffer.height() as f32].into()
+                ImageInner::StaticTextures { size, .. } => size,
-                }
-                ImageInner::StaticTextures { size, .. } => size.cast(),
             }
         }
 

sixtyfps_runtime/rendering_backends/mcu/simulator.rs

@@ -368,15 +368,13 @@ impl sixtyfps_corelib::backend::Backend for SimulatorBackend {
             .send_event(self::event_loop::CustomEvent::UserEvent(event));
     }
 
-    fn image_size(&'static self, image: &Image) -> sixtyfps_corelib::graphics::Size {
+    fn image_size(&'static self, image: &Image) -> sixtyfps_corelib::graphics::IntSize {
         let inner: &ImageInner = image.into();
         match inner {
             ImageInner::None => Default::default(),
             ImageInner::AbsoluteFilePath(_) | ImageInner::EmbeddedData { .. } => unimplemented!(),
-            ImageInner::EmbeddedImage(buffer) => {
+            ImageInner::EmbeddedImage(buffer) => buffer.size(),
-                [buffer.width() as f32, buffer.height() as f32].into()
+            ImageInner::StaticTextures { size, .. } => size,
-            }
-            ImageInner::StaticTextures { size, .. } => size.cast(),
         }
     }
 }

sixtyfps_runtime/rendering_backends/qt/lib.rs

@@ -16,7 +16,7 @@ only be used with `version = "=x.y.z"` in Cargo.toml.
 
 extern crate alloc;
 
-use sixtyfps_corelib::graphics::{Image, Size};
+use sixtyfps_corelib::graphics::{Image, IntSize};
 #[cfg(not(no_qt))]
 use sixtyfps_corelib::items::ImageFit;
 use sixtyfps_corelib::window::Window;
@@ -267,19 +267,17 @@ impl sixtyfps_corelib::backend::Backend for Backend {
         };
     }
 
-    fn image_size(&'static self, _image: &Image) -> Size {
+    fn image_size(&'static self, _image: &Image) -> IntSize {
         #[cfg(not(no_qt))]
         {
             let inner: &ImageInner = _image.into();
             match inner {
                 sixtyfps_corelib::ImageInner::None => Default::default(),
-                sixtyfps_corelib::ImageInner::EmbeddedImage(buffer) => {
+                sixtyfps_corelib::ImageInner::EmbeddedImage(buffer) => buffer.size(),
-                    [buffer.width() as _, buffer.height() as _].into()
-                }
                 _ => qt_window::load_image_from_resource(inner, None, ImageFit::fill)
                     .map(|img| {
                         let qsize = img.size();
-                        euclid::size2(qsize.width as f32, qsize.height as f32)
+                        euclid::size2(qsize.width, qsize.height)
                     })
                     .unwrap_or_default(),
             }

sixtyfps_runtime/rendering_backends/testing/lib.rs

@@ -12,7 +12,7 @@ You should use the `sixtyfps` crate instead.
 
 use image::GenericImageView;
 use sixtyfps_corelib::component::ComponentRc;
-use sixtyfps_corelib::graphics::{Image, Point, Size};
+use sixtyfps_corelib::graphics::{Image, IntSize, Point, Size};
 use sixtyfps_corelib::window::{PlatformWindow, Window};
 use sixtyfps_corelib::ImageInner;
 use std::path::Path;
@@ -62,30 +62,22 @@ impl sixtyfps_corelib::backend::Backend for TestingBackend {
         unimplemented!("event with the testing backend");
     }
 
-    fn image_size(&'static self, image: &Image) -> Size {
+    fn image_size(&'static self, image: &Image) -> IntSize {
         let inner: &ImageInner = image.into();
         match inner {
             ImageInner::None => Default::default(),
-            ImageInner::EmbeddedImage(buffer) => {
+            ImageInner::EmbeddedImage(buffer) => buffer.size(),
-                Size::new(buffer.width() as _, buffer.height() as _)
-            }
             ImageInner::AbsoluteFilePath(path) => image::open(Path::new(path.as_str()))
-                .map(|img| {
+                .map(|img| img.dimensions().into())
-                    let dim = img.dimensions();
-                    Size::new(dim.0 as _, dim.1 as _)
-                })
                 .unwrap_or_default(),
             ImageInner::EmbeddedData { data, format } => image::load_from_memory_with_format(
                 data.as_slice(),
                 image::ImageFormat::from_extension(std::str::from_utf8(format.as_slice()).unwrap())
                     .unwrap(),
             )
-            .map(|img| {
+            .map(|img| img.dimensions().into())
-                let dim = img.dimensions();
-                Size::new(dim.0 as _, dim.1 as _)
-            })
             .unwrap_or_default(),
-            ImageInner::StaticTextures { size, .. } => size.cast(),
+            ImageInner::StaticTextures { size, .. } => size.clone(),
         }
     }
 }