canvas: Unify retrieving image data from the HTMLImageElement (#37809)

Currently, HTMLImageElement uses as an image source
(ImageBitmapSource, CanvasImageSource, TexImageSource)
in various canvas2D/WebGL operations, and there is a small
inconsistency in how we get the image data of the 'img' element:
usability checking and retrieving the image data from the image cache.

To simplify and avoid state inconsistency between the window's image
cache and the 'img' element, let's retrieve the image data (as a raster)
from the HTMLImageElement itself.

Testing: No expected changes in testing results, except the
'drawimage_svg_image_with_foreign_object_does_not_taint.html'
which is 'false' passed because drawing of the non supported vector
image
is silently skip instead of throwing the 'InvalidState' exception
anymore.

Signed-off-by: Andrei Volykhin <andrei.volykhin@gmail.com>

components/script/canvas_state.rs

@@ -53,9 +53,10 @@ use crate::dom::canvasgradient::{CanvasGradient, CanvasGradientStyle, ToFillOrSt
 use crate::dom::canvaspattern::CanvasPattern;
 use crate::dom::dommatrix::DOMMatrix;
 use crate::dom::dommatrixreadonly::dommatrix2dinit_to_matrix;
-use crate::dom::element::{Element, cors_setting_for_element};
+use crate::dom::element::Element;
 use crate::dom::globalscope::GlobalScope;
 use crate::dom::htmlcanvaselement::HTMLCanvasElement;
+use crate::dom::htmlimageelement::HTMLImageElement;
 use crate::dom::htmlvideoelement::HTMLVideoElement;
 use crate::dom::imagebitmap::ImageBitmap;
 use crate::dom::imagedata::ImageData;
@@ -444,6 +445,17 @@ impl CanvasState {
         }
 
         let result = match image {
+            CanvasImageSource::HTMLImageElement(ref image) => {
+                // https://html.spec.whatwg.org/multipage/#drawing-images
+                // 2. Let usability be the result of checking the usability of image.
+                // 3. If usability is bad, then return (without drawing anything).
+                if !image.is_usable()? {
+                    return Ok(());
+                }
+
+                self.draw_html_image_element(image, htmlcanvas, sx, sy, sw, sh, dx, dy, dw, dh);
+                Ok(())
+            },
             CanvasImageSource::HTMLVideoElement(ref video) => {
                 // <https://html.spec.whatwg.org/multipage/#check-the-usability-of-the-image-argument>
                 // Step 2. Let usability be the result of checking the usability of image.
@@ -480,34 +492,6 @@ impl CanvasState {
 
                 self.draw_offscreen_canvas(canvas, htmlcanvas, sx, sy, sw, sh, dx, dy, dw, dh)
             },
-            CanvasImageSource::HTMLImageElement(ref image) => {
-                // https://html.spec.whatwg.org/multipage/#drawing-images
-                // 2. Let usability be the result of checking the usability of image.
-                // 3. If usability is bad, then return (without drawing anything).
-                if !image.is_usable()? {
-                    return Ok(());
-                }
-
-                // TODO(pylbrecht): is it possible for image.get_url() to return None after the usability check?
-                // https://html.spec.whatwg.org/multipage/#img-error
-                // If the image argument is an HTMLImageElement object that is in the broken state,
-                // then throw an InvalidStateError exception
-                let url = image.get_url().ok_or(Error::InvalidState)?;
-                let cors_setting = cors_setting_for_element(image.upcast());
-                self.fetch_and_draw_image_data(
-                    htmlcanvas,
-                    url,
-                    cors_setting,
-                    sx,
-                    sy,
-                    sw,
-                    sh,
-                    dx,
-                    dy,
-                    dw,
-                    dh,
-                )
-            },
             CanvasImageSource::CSSStyleValue(ref value) => {
                 let url = value
                     .get_url(self.base_url.clone())
@@ -524,6 +508,52 @@ impl CanvasState {
         result
     }
 
+    /// <https://html.spec.whatwg.org/multipage/#dom-context-2d-drawimage>
+    #[allow(clippy::too_many_arguments)]
+    fn draw_html_image_element(
+        &self,
+        image: &HTMLImageElement,
+        canvas: Option<&HTMLCanvasElement>,
+        sx: f64,
+        sy: f64,
+        sw: Option<f64>,
+        sh: Option<f64>,
+        dx: f64,
+        dy: f64,
+        dw: Option<f64>,
+        dh: Option<f64>,
+    ) {
+        let Some(snapshot) = image.get_raster_image_data() else {
+            return;
+        };
+
+        // Step 4. Establish the source and destination rectangles.
+        let image_size = snapshot.size();
+        let dw = dw.unwrap_or(image_size.width as f64);
+        let dh = dh.unwrap_or(image_size.height as f64);
+        let sw = sw.unwrap_or(image_size.width as f64);
+        let sh = sh.unwrap_or(image_size.height as f64);
+
+        let (source_rect, dest_rect) =
+            self.adjust_source_dest_rects(image_size, sx, sy, sw, sh, dx, dy, dw, dh);
+
+        // Step 5. If one of the sw or sh arguments is zero, then return. Nothing is painted.
+        if !is_rect_valid(source_rect) || !is_rect_valid(dest_rect) {
+            return;
+        }
+
+        let smoothing_enabled = self.state.borrow().image_smoothing_enabled;
+
+        self.send_canvas_2d_msg(Canvas2dMsg::DrawImage(
+            snapshot.as_ipc(),
+            dest_rect,
+            source_rect,
+            smoothing_enabled,
+        ));
+
+        self.mark_as_dirty(canvas);
+    }
+
     /// <https://html.spec.whatwg.org/multipage/#dom-context-2d-drawimage>
     #[allow(clippy::too_many_arguments)]
     fn draw_html_video_element(
@@ -1099,12 +1129,7 @@ impl CanvasState {
                     return Ok(None);
                 }
 
-                image
-                    .get_url()
-                    .and_then(|url| {
-                        self.fetch_image_data(url, cors_setting_for_element(image.upcast()))
-                    })
-                    .ok_or(Error::InvalidState)?
+                image.get_raster_image_data().ok_or(Error::InvalidState)?
             },
             CanvasImageSource::HTMLVideoElement(ref video) => {
                 // <https://html.spec.whatwg.org/multipage/#check-the-usability-of-the-image-argument>

components/script/dom/htmlcanvaselement.rs

@@ -749,29 +749,3 @@ impl Convert<GLContextAttributes> for WebGLContextAttributes {
         }
     }
 }
-
-pub(crate) mod utils {
-    use net_traits::image_cache::ImageResponse;
-    use net_traits::request::CorsSettings;
-    use servo_url::ServoUrl;
-
-    use crate::dom::window::Window;
-
-    pub(crate) fn request_image_from_cache(
-        window: &Window,
-        url: ServoUrl,
-        cors_setting: Option<CorsSettings>,
-    ) -> ImageResponse {
-        let image_cache = window.image_cache();
-        let result = image_cache.get_image(
-            url.clone(),
-            window.origin().immutable().clone(),
-            cors_setting,
-        );
-
-        match result {
-            Some(image) => ImageResponse::Loaded(image, url),
-            None => ImageResponse::None,
-        }
-    }
-}

components/script/dom/htmlimageelement.rs

@@ -12,7 +12,7 @@ use std::{char, mem};
 use app_units::{AU_PER_PX, Au};
 use cssparser::{Parser, ParserInput};
 use dom_struct::dom_struct;
-use euclid::Point2D;
+use euclid::default::{Point2D, Size2D};
 use html5ever::{LocalName, Prefix, QualName, local_name, ns};
 use js::jsapi::JSAutoRealm;
 use js::rust::HandleObject;
@@ -28,7 +28,9 @@ use net_traits::{
     ResourceFetchTiming, ResourceTimingType,
 };
 use num_traits::ToPrimitive;
-use pixels::{CorsStatus, ImageMetadata};
+use pixels::{
+    CorsStatus, ImageMetadata, PixelFormat, Snapshot, SnapshotAlphaMode, SnapshotPixelFormat,
+};
 use servo_url::ServoUrl;
 use servo_url::origin::MutableOrigin;
 use style::attr::{AttrValue, LengthOrPercentageOrAuto, parse_integer, parse_length};
@@ -168,9 +170,6 @@ pub(crate) struct HTMLImageElement {
 }
 
 impl HTMLImageElement {
-    pub(crate) fn get_url(&self) -> Option<ServoUrl> {
-        self.current_request.borrow().parsed_url.clone()
-    }
     // https://html.spec.whatwg.org/multipage/#check-the-usability-of-the-image-argument
     pub(crate) fn is_usable(&self) -> Fallible<bool> {
         // If image has an intrinsic width or intrinsic height (or both) equal to zero, then return bad.
@@ -193,6 +192,36 @@ impl HTMLImageElement {
     pub(crate) fn image_data(&self) -> Option<Image> {
         self.current_request.borrow().image.clone()
     }
+
+    /// Gets the copy of the raster image data.
+    pub(crate) fn get_raster_image_data(&self) -> Option<Snapshot> {
+        let Some(img) = self.image_data()?.as_raster_image() else {
+            warn!("Vector image is not supported as raster image source");
+            return None;
+        };
+
+        let size = Size2D::new(img.metadata.width, img.metadata.height);
+        let format = match img.format {
+            PixelFormat::BGRA8 => SnapshotPixelFormat::BGRA,
+            PixelFormat::RGBA8 => SnapshotPixelFormat::RGBA,
+            pixel_format => {
+                unimplemented!("unsupported pixel format ({:?})", pixel_format)
+            },
+        };
+
+        let alpha_mode = SnapshotAlphaMode::Transparent {
+            premultiplied: false,
+        };
+
+        let snapshot = Snapshot::from_vec(
+            size.cast(),
+            format,
+            alpha_mode,
+            img.first_frame().bytes.to_vec(),
+        );
+
+        Some(snapshot)
+    }
 }
 
 /// The context required for asynchronously loading an external image.

components/script/dom/imagebitmap.rs

@@ -19,7 +19,7 @@ use crate::dom::bindings::codegen::Bindings::ImageBitmapBinding::{
     ImageBitmapMethods, ImageBitmapOptions, ImageBitmapSource, ImageOrientation, PremultiplyAlpha,
     ResizeQuality,
 };
-use crate::dom::bindings::refcounted::Trusted;
+use crate::dom::bindings::refcounted::{Trusted, TrustedPromise};
 use crate::dom::bindings::reflector::{Reflector, reflect_dom_object};
 use crate::dom::bindings::root::DomRoot;
 use crate::dom::bindings::serializable::Serializable;
@@ -28,7 +28,6 @@ use crate::dom::bindings::transferable::Transferable;
 use crate::dom::globalscope::GlobalScope;
 use crate::dom::types::Promise;
 use crate::script_runtime::CanGc;
-use crate::test::TrustedPromise;
 
 #[dom_struct]
 pub(crate) struct ImageBitmap {
@@ -359,37 +358,13 @@ impl ImageBitmap {
                     return p;
                 }
 
-                // If no ImageBitmap object can be constructed, then the promise is rejected instead.
-                let Some(img) = image.image_data() else {
+                // If no ImageBitmap object can be constructed, then the promise
+                // is rejected instead.
+                let Some(snapshot) = image.get_raster_image_data() else {
                     p.reject_error(Error::InvalidState, can_gc);
                     return p;
                 };
 
-                // TODO: Support vector HTMLImageElement.
-                let Some(img) = img.as_raster_image() else {
-                    p.reject_error(Error::InvalidState, can_gc);
-                    return p;
-                };
-
-                let size = Size2D::new(img.metadata.width, img.metadata.height);
-                let format = match img.format {
-                    PixelFormat::BGRA8 => SnapshotPixelFormat::BGRA,
-                    PixelFormat::RGBA8 => SnapshotPixelFormat::RGBA,
-                    pixel_format => {
-                        unimplemented!("unsupported pixel format ({:?})", pixel_format)
-                    },
-                };
-                let alpha_mode = SnapshotAlphaMode::Transparent {
-                    premultiplied: false,
-                };
-
-                let snapshot = Snapshot::from_vec(
-                    size.cast(),
-                    format,
-                    alpha_mode,
-                    img.first_frame().bytes.to_vec(),
-                );
-
                 // Step 6.3. Set imageBitmap's bitmap data to a copy of image's media data,
                 // cropped to the source rectangle with formatting.
                 let Some(bitmap_data) =

components/script/dom/webglrenderingcontext.rs

@@ -29,7 +29,6 @@ use js::typedarray::{
     ArrayBufferView, CreateWith, Float32, Float32Array, Int32, Int32Array, TypedArray,
     TypedArrayElementCreator, Uint32Array,
 };
-use net_traits::image_cache::ImageResponse;
 use pixels::{self, PixelFormat, Snapshot, SnapshotPixelFormat};
 use serde::{Deserialize, Serialize};
 use servo_config::pref;
@@ -55,9 +54,8 @@ use crate::dom::bindings::inheritance::Castable;
 use crate::dom::bindings::reflector::{DomGlobal, DomObject, Reflector, reflect_dom_object};
 use crate::dom::bindings::root::{DomOnceCell, DomRoot, LayoutDom, MutNullableDom};
 use crate::dom::bindings::str::DOMString;
-use crate::dom::element::cors_setting_for_element;
 use crate::dom::event::{Event, EventBubbles, EventCancelable};
-use crate::dom::htmlcanvaselement::{LayoutCanvasRenderingContextHelpers, utils as canvas_utils};
+use crate::dom::htmlcanvaselement::LayoutCanvasRenderingContextHelpers;
 use crate::dom::node::{Node, NodeDamage, NodeTraits};
 #[cfg(feature = "webxr")]
 use crate::dom::promise::Promise;
@@ -652,50 +650,30 @@ impl WebGLRenderingContext {
                     return Err(Error::Security);
                 }
 
-                let img_url = match image.get_url() {
-                    Some(url) => url,
-                    None => return Ok(None),
-                };
-
-                let window = match self.canvas {
-                    HTMLCanvasElementOrOffscreenCanvas::HTMLCanvasElement(ref canvas) => {
-                        canvas.owner_window()
-                    },
-                    // This is marked as unreachable as we should have returned already
-                    HTMLCanvasElementOrOffscreenCanvas::OffscreenCanvas(_) => unreachable!(),
-                };
-                let cors_setting = cors_setting_for_element(image.upcast());
-
-                let img = match canvas_utils::request_image_from_cache(
-                    &window,
-                    img_url,
-                    cors_setting,
-                ) {
-                    ImageResponse::Loaded(image, _) => {
-                        match image.as_raster_image() {
-                            Some(image) => image,
-                            None => {
-                                // Vector images are not currently supported here and there are some open questions
-                                // in the specification about how to handle them:
-                                // See https://github.com/KhronosGroup/WebGL/issues/1503.
-                                warn!(
-                                    "Vector images as are not yet supported as WebGL texture source"
-                                );
+                // Vector images are not currently supported here and there are
+                // some open questions in the specification about how to handle them:
+                // See https://github.com/KhronosGroup/WebGL/issues/1503
+                let Some(snapshot) = image.get_raster_image_data() else {
                     return Ok(None);
-                            },
-                        }
-                    },
-                    ImageResponse::PlaceholderLoaded(_, _) |
-                    ImageResponse::None |
-                    ImageResponse::MetadataLoaded(_) => return Ok(None),
                 };
 
-                let size = Size2D::new(img.metadata.width, img.metadata.height);
-                let data = IpcSharedMemory::from_bytes(img.first_frame().bytes);
+                let snapshot = snapshot.as_ipc();
+                let size = snapshot.size().cast();
+                let format: PixelFormat = match snapshot.format() {
+                    SnapshotPixelFormat::RGBA => PixelFormat::RGBA8,
+                    SnapshotPixelFormat::BGRA => PixelFormat::BGRA8,
+                };
 
-                let (alpha_treatment, y_axis_treatment) = self.get_current_unpack_state(false);
+                let (alpha_treatment, y_axis_treatment) =
+                    self.get_current_unpack_state(snapshot.alpha_mode().is_premultiplied());
 
-                TexPixels::new(data, size, img.format, alpha_treatment, y_axis_treatment)
+                TexPixels::new(
+                    snapshot.to_ipc_shared_memory(),
+                    size,
+                    format,
+                    alpha_treatment,
+                    y_axis_treatment,
+                )
             },
             // TODO(emilio): Getting canvas data is implemented in CanvasRenderingContext2D,
             // but we need to refactor it moving it to `HTMLCanvasElement` and support

tests/wpt/meta/html/canvas/element/manual/drawing-images-to-the-canvas/drawimage_svg_image_with_foreign_object_does_not_taint.html.ini

@@ -1,3 +0,0 @@
-[drawimage_svg_image_with_foreign_object_does_not_taint.html]
-  [Canvas should not be tainted after drawing SVG including <foreignObject>]
-    expected: FAIL