From 142a8dc185e3010b29eb604e3d2324f7627ff15a Mon Sep 17 00:00:00 2001 From: Olivier Goffart Date: Fri, 28 May 2021 16:17:42 +0200 Subject: [PATCH] Rename ImageReference to ImageInner and make Immage.0 private --- api/sixtyfps-cpp/include/sixtyfps_image.h | 2 +- api/sixtyfps-node/native/lib.rs | 12 ++++--- api/sixtyfps-rs/lib.rs | 2 +- sixtyfps_compiler/generator/rust.rs | 4 +-- sixtyfps_runtime/corelib/graphics/image.rs | 18 +++++++---- sixtyfps_runtime/corelib/items/image.rs | 6 ++-- sixtyfps_runtime/corelib/lib.rs | 2 +- sixtyfps_runtime/corelib/window.rs | 4 +-- .../rendering_backends/gl/graphics_window.rs | 2 +- .../rendering_backends/gl/images.rs | 15 ++++----- sixtyfps_runtime/rendering_backends/gl/lib.rs | 18 +++++------ .../rendering_backends/qt/qt_window.rs | 32 +++++++++---------- .../rendering_backends/testing/lib.rs | 12 +++---- xtask/src/cbindgen.rs | 4 +-- 14 files changed, 68 insertions(+), 65 deletions(-) diff --git a/api/sixtyfps-cpp/include/sixtyfps_image.h b/api/sixtyfps-cpp/include/sixtyfps_image.h index 7e16e8fc3..d82bda13a 100644 --- a/api/sixtyfps-cpp/include/sixtyfps_image.h +++ b/api/sixtyfps-cpp/include/sixtyfps_image.h @@ -43,7 +43,7 @@ public: } private: - using Tag = cbindgen_private::types::ImageReference::Tag; + using Tag = cbindgen_private::types::ImageInner::Tag; using Data = cbindgen_private::types::Image; Data data; }; diff --git a/api/sixtyfps-node/native/lib.rs b/api/sixtyfps-node/native/lib.rs index 1be3115df..8232f4b8e 100644 --- a/api/sixtyfps-node/native/lib.rs +++ b/api/sixtyfps-node/native/lib.rs @@ -11,7 +11,7 @@ use core::cell::RefCell; use neon::prelude::*; use rand::RngCore; use sixtyfps_compilerlib::langtype::Type; -use sixtyfps_corelib::{ImageReference, SharedVector}; +use sixtyfps_corelib::{ImageInner, SharedVector}; mod js_model; mod persistent_context; @@ -247,10 +247,12 @@ fn to_js_value<'cx>( Value::Number(n) => JsNumber::new(cx, n).as_value(cx), Value::String(s) => JsString::new(cx, s.as_str()).as_value(cx), Value::Bool(b) => JsBoolean::new(cx, b).as_value(cx), - Value::Image(r) => match r.0 { - ImageReference::None => JsUndefined::new().as_value(cx), - ImageReference::AbsoluteFilePath(path) => JsString::new(cx, path.as_str()).as_value(cx), - ImageReference::EmbeddedData { .. } | ImageReference::EmbeddedRgbaImage { .. } => { + Value::Image(r) => match (&r).into() { + &ImageInner::None => JsUndefined::new().as_value(cx), + &ImageInner::AbsoluteFilePath(ref path) => { + JsString::new(cx, path.as_str()).as_value(cx) + } + &ImageInner::EmbeddedData { .. } | &ImageInner::EmbeddedRgbaImage { .. } => { JsNull::new().as_value(cx) } // TODO: maybe pass around node buffers? }, diff --git a/api/sixtyfps-rs/lib.rs b/api/sixtyfps-rs/lib.rs index 477976075..716221d6f 100644 --- a/api/sixtyfps-rs/lib.rs +++ b/api/sixtyfps-rs/lib.rs @@ -226,7 +226,7 @@ pub mod re_exports { init_component_items, Component, ComponentRefPin, ComponentVTable, }; pub use sixtyfps_corelib::graphics::{ - Brush, GradientStop, Image, ImageReference, LinearGradientBrush, PathArcTo, PathCubicTo, + Brush, GradientStop, Image, ImageInner, LinearGradientBrush, PathArcTo, PathCubicTo, PathData, PathElement, PathEvent, PathLineTo, PathMoveTo, PathQuadraticTo, Point, Rect, Size, }; diff --git a/sixtyfps_compiler/generator/rust.rs b/sixtyfps_compiler/generator/rust.rs index c77b15dbd..1ad7f2deb 100644 --- a/sixtyfps_compiler/generator/rust.rs +++ b/sixtyfps_compiler/generator/rust.rs @@ -1343,8 +1343,8 @@ fn compile_expression(expr: &Expression, component: &Rc) -> TokenStre crate::expression_tree::ImageReference::EmbeddedData(resource_id) => { let symbol = format_ident!("SFPS_EMBEDDED_RESOURCE_{}", resource_id); quote!( - sixtyfps::re_exports::Image( - sixtyfps::re_exports::ImageReference::EmbeddedData(#symbol.into()) + sixtyfps::re_exports::Image::from( + sixtyfps::re_exports::ImageInner::EmbeddedData(#symbol.into()) ) ) } diff --git a/sixtyfps_runtime/corelib/graphics/image.rs b/sixtyfps_runtime/corelib/graphics/image.rs index d968412a7..23459950c 100644 --- a/sixtyfps_runtime/corelib/graphics/image.rs +++ b/sixtyfps_runtime/corelib/graphics/image.rs @@ -16,7 +16,7 @@ use crate::{SharedString, SharedVector}; /// is necessary before they can be used. #[derive(Clone, PartialEq, Debug)] #[repr(u8)] -pub enum ImageReference { +pub enum ImageInner { /// A resource that does not represent any data. None, /// A resource that points to a file in the file system @@ -29,9 +29,15 @@ pub enum ImageReference { EmbeddedRgbaImage { width: u32, height: u32, data: SharedVector }, } -impl Default for ImageReference { +impl Default for ImageInner { fn default() -> Self { - ImageReference::None + ImageInner::None + } +} + +impl<'a> From<&'a Image> for &'a ImageInner { + fn from(other: &'a Image) -> Self { + &other.0 } } @@ -41,14 +47,14 @@ pub struct LoadImageError(()); /// An image type that can be displayed by the Image element #[repr(transparent)] -#[derive(Default, Clone, Debug, PartialEq)] +#[derive(Default, Clone, Debug, PartialEq, derive_more::From)] // FIXME: the inner should be private -pub struct Image(pub ImageReference); +pub struct Image(ImageInner); impl Image { /// Load an Image from a path to a file containing an image pub fn load_from_path(path: &std::path::Path) -> Result { - Ok(Image(ImageReference::AbsoluteFilePath(path.to_str().ok_or(LoadImageError(()))?.into()))) + Ok(Image(ImageInner::AbsoluteFilePath(path.to_str().ok_or(LoadImageError(()))?.into()))) } /* /// Create an image from argb pixels. diff --git a/sixtyfps_runtime/corelib/items/image.rs b/sixtyfps_runtime/corelib/items/image.rs index 107b0086f..fc78bda97 100644 --- a/sixtyfps_runtime/corelib/items/image.rs +++ b/sixtyfps_runtime/corelib/items/image.rs @@ -72,7 +72,7 @@ impl Item for ImageItem { } fn layouting_info(self: Pin<&Self>, window: &ComponentWindow) -> LayoutInfo { - let natural_size = window.0.image_size(&self.source().0); + let natural_size = window.0.image_size((&self.source()).into()); LayoutInfo { preferred_width: natural_size.width, preferred_height: natural_size.height, @@ -143,7 +143,7 @@ impl Item for ClippedImage { } fn layouting_info(self: Pin<&Self>, window: &ComponentWindow) -> LayoutInfo { - let natural_size = window.0.image_size(&self.source().0); + let natural_size = window.0.image_size((&self.source()).into()); LayoutInfo { preferred_width: natural_size.width, preferred_height: natural_size.height, @@ -185,4 +185,4 @@ impl ItemConsts for ClippedImage { ClippedImage, CachedRenderingData, > = ClippedImage::FIELD_OFFSETS.cached_rendering_data.as_unpinned_projection(); -} \ No newline at end of file +} diff --git a/sixtyfps_runtime/corelib/lib.rs b/sixtyfps_runtime/corelib/lib.rs index 0ee063348..b20d28824 100644 --- a/sixtyfps_runtime/corelib/lib.rs +++ b/sixtyfps_runtime/corelib/lib.rs @@ -49,7 +49,7 @@ pub use string::SharedString; pub use sharedvector::SharedVector; #[doc(inline)] -pub use graphics::ImageReference; +pub use graphics::ImageInner; #[doc(inline)] pub use properties::Property; diff --git a/sixtyfps_runtime/corelib/window.rs b/sixtyfps_runtime/corelib/window.rs index bb911817d..8c7af0340 100644 --- a/sixtyfps_runtime/corelib/window.rs +++ b/sixtyfps_runtime/corelib/window.rs @@ -15,7 +15,7 @@ use crate::input::{KeyEvent, MouseEvent, MouseInputState, TextCursorBlinker}; use crate::items::{ItemRc, ItemRef, ItemWeak}; use crate::properties::PropertyTracker; use crate::slice::Slice; -use crate::ImageReference; +use crate::ImageInner; use crate::{ component::{ComponentRc, ComponentWeak}, SharedString, @@ -69,7 +69,7 @@ pub trait PlatformWindow { /// Return the size of the image referenced by the specified resource, multiplied by the window /// scale factor. - fn image_size(&self, source: &ImageReference) -> crate::graphics::Size; + fn image_size(&self, source: &ImageInner) -> crate::graphics::Size; /// Return self as any so the backend can upcast fn as_any(&self) -> &dyn core::any::Any; diff --git a/sixtyfps_runtime/rendering_backends/gl/graphics_window.rs b/sixtyfps_runtime/rendering_backends/gl/graphics_window.rs index 3a52507d7..da2d7daee 100644 --- a/sixtyfps_runtime/rendering_backends/gl/graphics_window.rs +++ b/sixtyfps_runtime/rendering_backends/gl/graphics_window.rs @@ -507,7 +507,7 @@ impl PlatformWindow for GraphicsWindow { )) } - fn image_size(&self, source: &ImageReference) -> sixtyfps_corelib::graphics::Size { + fn image_size(&self, source: &ImageInner) -> sixtyfps_corelib::graphics::Size { match &*self.map_state.borrow() { GraphicsWindowBackendState::Unmapped => { // Temporary: Register this internal property to notify the caller when diff --git a/sixtyfps_runtime/rendering_backends/gl/images.rs b/sixtyfps_runtime/rendering_backends/gl/images.rs index 89ce46fa7..3f369884c 100644 --- a/sixtyfps_runtime/rendering_backends/gl/images.rs +++ b/sixtyfps_runtime/rendering_backends/gl/images.rs @@ -11,7 +11,7 @@ LICENSE END */ use std::cell::RefCell; use std::rc::Rc; -use sixtyfps_corelib::{graphics::Size, slice::Slice, ImageReference, Property, SharedString}; +use sixtyfps_corelib::{graphics::Size, slice::Slice, ImageInner, Property, SharedString}; use super::{CanvasRc, GLItemRenderer, GLRendererData, ItemGraphicsCacheEntry}; @@ -115,15 +115,12 @@ impl CachedImage { Self(RefCell::new(ImageData::SVG(tree))) } - pub fn new_from_resource( - resource: &ImageReference, - renderer: &GLRendererData, - ) -> Option> { + pub fn new_from_resource(resource: &ImageInner, renderer: &GLRendererData) -> Option> { match resource { - ImageReference::None => None, - ImageReference::AbsoluteFilePath(path) => Self::new_from_path(path, renderer), - ImageReference::EmbeddedData(data) => Self::new_from_data(data).map(Rc::new), - ImageReference::EmbeddedRgbaImage { .. } => todo!(), + ImageInner::None => None, + ImageInner::AbsoluteFilePath(path) => Self::new_from_path(path, renderer), + ImageInner::EmbeddedData(data) => Self::new_from_data(data).map(Rc::new), + ImageInner::EmbeddedRgbaImage { .. } => todo!(), } } diff --git a/sixtyfps_runtime/rendering_backends/gl/lib.rs b/sixtyfps_runtime/rendering_backends/gl/lib.rs index 7f203733a..3f7aa84e4 100644 --- a/sixtyfps_runtime/rendering_backends/gl/lib.rs +++ b/sixtyfps_runtime/rendering_backends/gl/lib.rs @@ -31,7 +31,7 @@ use sixtyfps_corelib::items::{ use sixtyfps_corelib::properties::Property; use sixtyfps_corelib::window::ComponentWindow; -use sixtyfps_corelib::{ImageReference, SharedString}; +use sixtyfps_corelib::{ImageInner, SharedString}; mod graphics_window; use graphics_window::*; @@ -53,19 +53,19 @@ enum ImageCacheKey { } impl ImageCacheKey { - fn new(resource: &ImageReference) -> Option { + fn new(resource: &ImageInner) -> Option { Some(match resource { - ImageReference::None => return None, - ImageReference::AbsoluteFilePath(path) => { + ImageInner::None => return None, + ImageInner::AbsoluteFilePath(path) => { if path.is_empty() { return None; } Self::Path(path.to_string()) } - ImageReference::EmbeddedData(data) => { + ImageInner::EmbeddedData(data) => { Self::EmbeddedData(by_address::ByAddress(data.as_slice())) } - ImageReference::EmbeddedRgbaImage { .. } => return None, + ImageInner::EmbeddedRgbaImage { .. } => return None, }) } } @@ -364,7 +364,7 @@ impl GLRendererData { } // Try to load the image the given resource points to - fn load_image_resource(&self, resource: &ImageReference) -> Option> { + fn load_image_resource(&self, resource: &ImageInner) -> Option> { let cache_key = ImageCacheKey::new(resource)?; self.lookup_image_in_cache_or_create(cache_key, || { @@ -474,7 +474,7 @@ impl GLRenderer { /// Returns the size of image referenced by the specified resource. These are image pixels, not adjusted /// to the window scale factor. - fn image_size(&self, source: &ImageReference) -> sixtyfps_corelib::graphics::Size { + fn image_size(&self, source: &ImageInner) -> sixtyfps_corelib::graphics::Size { self.shared_data .load_image_resource(source) .and_then(|image| image.size()) @@ -1376,7 +1376,7 @@ impl GLItemRenderer { .borrow_mut() .load_item_graphics_cache_with_function(item_cache, || { self.shared_data - .load_image_resource(&source_property.get().0) + .load_image_resource((&source_property.get()).into()) .and_then(|cached_image| { cached_image.as_renderable( // The condition at the entry of the function ensures that width/height are positive diff --git a/sixtyfps_runtime/rendering_backends/qt/qt_window.rs b/sixtyfps_runtime/rendering_backends/qt/qt_window.rs index 690e6b0ad..884780374 100644 --- a/sixtyfps_runtime/rendering_backends/qt/qt_window.rs +++ b/sixtyfps_runtime/rendering_backends/qt/qt_window.rs @@ -17,7 +17,7 @@ use sixtyfps_corelib::items::{self, FillRule, ItemRef, TextOverflow, TextWrap}; use sixtyfps_corelib::slice::Slice; use sixtyfps_corelib::window::PlatformWindow; use sixtyfps_corelib::{component::ComponentRc, SharedString}; -use sixtyfps_corelib::{ImageReference, PathData, Property}; +use sixtyfps_corelib::{ImageInner, PathData, Property}; use std::cell::RefCell; use std::pin::Pin; @@ -719,15 +719,15 @@ impl ItemRenderer for QtItemRenderer<'_> { } fn load_image_from_resource( - resource: &ImageReference, + resource: &ImageInner, source_size: Option, image_fit: ImageFit, ) -> Option { let (is_path, data) = match resource { - ImageReference::None => return None, - ImageReference::AbsoluteFilePath(path) => (true, qttypes::QByteArray::from(path.as_str())), - ImageReference::EmbeddedData(data) => (false, qttypes::QByteArray::from(data.as_slice())), - ImageReference::EmbeddedRgbaImage { .. } => todo!(), + ImageInner::None => return None, + ImageInner::AbsoluteFilePath(path) => (true, qttypes::QByteArray::from(path.as_str())), + ImageInner::EmbeddedData(data) => (false, qttypes::QByteArray::from(data.as_slice())), + ImageInner::EmbeddedRgbaImage { .. } => todo!(), }; let size_requested = is_svg(resource) && source_size.is_some(); let source_size = source_size.unwrap_or_default(); @@ -807,12 +807,12 @@ fn adjust_to_image_fit( } /// Return true if this image is a SVG that is scalable -fn is_svg(resource: &ImageReference) -> bool { +fn is_svg(resource: &ImageInner) -> bool { match resource { - ImageReference::None => false, - ImageReference::AbsoluteFilePath(path) => path.as_str().ends_with(".svg"), - ImageReference::EmbeddedData(data) => data.starts_with(b" false, + ImageInner::None => false, + ImageInner::AbsoluteFilePath(path) => path.as_str().ends_with(".svg"), + ImageInner::EmbeddedData(data) => data.starts_with(b" false, } } @@ -851,9 +851,8 @@ impl QtItemRenderer<'_> { None }; - load_image_from_resource(&source_property.get().0, source_size, image_fit).map_or( - QtRenderingCacheItem::Invalid, - |mut pixmap: qttypes::QPixmap| { + load_image_from_resource((&source_property.get()).into(), source_size, image_fit) + .map_or(QtRenderingCacheItem::Invalid, |mut pixmap: qttypes::QPixmap| { let colorize = colorize_property.map_or(Brush::default(), |c| c.get()); if !colorize.is_transparent() { let brush: QBrush = colorize.into(); @@ -864,8 +863,7 @@ impl QtItemRenderer<'_> { }); } QtRenderingCacheItem::Pixmap(pixmap) - }, - ) + }) }); let pixmap: &qttypes::QPixmap = match &cached { QtRenderingCacheItem::Pixmap(pixmap) => pixmap, @@ -1204,7 +1202,7 @@ impl PlatformWindow for QtWindow { Box::new(get_font(unresolved_font_request_getter().merge(&self.default_font_properties()))) } - fn image_size(&self, source: &ImageReference) -> sixtyfps_corelib::graphics::Size { + fn image_size(&self, source: &ImageInner) -> sixtyfps_corelib::graphics::Size { load_image_from_resource(source, None, ImageFit::fill) .map(|img| { let qsize = img.size(); diff --git a/sixtyfps_runtime/rendering_backends/testing/lib.rs b/sixtyfps_runtime/rendering_backends/testing/lib.rs index 614408576..a5f42e5d5 100644 --- a/sixtyfps_runtime/rendering_backends/testing/lib.rs +++ b/sixtyfps_runtime/rendering_backends/testing/lib.rs @@ -21,7 +21,7 @@ use sixtyfps_corelib::component::ComponentRc; use sixtyfps_corelib::graphics::{FontMetrics, Size}; use sixtyfps_corelib::slice::Slice; use sixtyfps_corelib::window::{ComponentWindow, PlatformWindow, Window}; -use sixtyfps_corelib::{ImageReference, Property}; +use sixtyfps_corelib::{ImageInner, Property}; use std::path::Path; use std::pin::Pin; use std::rc::Rc; @@ -125,19 +125,19 @@ impl PlatformWindow for TestingWindow { Box::new(TestingFontMetrics::default()) } - fn image_size(&self, source: &ImageReference) -> Size { + fn image_size(&self, source: &ImageInner) -> Size { match source { - ImageReference::None => Default::default(), - ImageReference::EmbeddedRgbaImage { width, height, .. } => { + ImageInner::None => Default::default(), + ImageInner::EmbeddedRgbaImage { width, height, .. } => { Size::new(*width as _, *height as _) } - ImageReference::AbsoluteFilePath(path) => image::open(Path::new(path.as_str())) + ImageInner::AbsoluteFilePath(path) => image::open(Path::new(path.as_str())) .map(|img| { let dim = img.dimensions(); Size::new(dim.0 as _, dim.1 as _) }) .unwrap_or_default(), - ImageReference::EmbeddedData(data) => image::load_from_memory(data.as_slice()) + ImageInner::EmbeddedData(data) => image::load_from_memory(data.as_slice()) .map(|img| { let dim = img.dimensions(); Size::new(dim.0 as _, dim.1 as _) diff --git a/xtask/src/cbindgen.rs b/xtask/src/cbindgen.rs index 7d37d30f2..82692f02b 100644 --- a/xtask/src/cbindgen.rs +++ b/xtask/src/cbindgen.rs @@ -109,7 +109,7 @@ fn gen_corelib(root_dir: &Path, include_dir: &Path) -> anyhow::Result<()> { config.export.exclude = [ "SharedString", "SharedVector", - "ImageReference", + "ImageInner", "Image", "Color", "PathData", @@ -180,7 +180,7 @@ fn gen_corelib(root_dir: &Path, include_dir: &Path) -> anyhow::Result<()> { .write_to_file(include_dir.join("sixtyfps_properties_internal.h")); for (rust_types, extra_excluded_types, internal_header) in [ - (vec!["ImageReference", "Image"], vec![], "sixtyfps_image_internal.h"), + (vec!["ImageInner", "Image"], vec![], "sixtyfps_image_internal.h"), ( vec!["Color", "sixtyfps_color_brighter", "sixtyfps_color_darker"], vec![],