From c648e24cff18dfd6d94af3de6dedeade0bbfc18b Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Fri, 12 May 2023 07:17:01 +0200 Subject: [PATCH] LibWeb: Don't force HTMLImageElement to have a legacy ImageLoader We achieve this by adding a new Layout::ImageProvider class and having both HTMLImageElement and HTMLObjectElement inherit from it. The HTML spec is vague on how object image loading should work, which is why this first pass is focusing on image elements. --- .../LibWeb/HTML/HTMLImageElement.cpp | 31 +++++---- .../Libraries/LibWeb/HTML/HTMLImageElement.h | 11 ++-- .../LibWeb/HTML/HTMLObjectElement.cpp | 14 +++- .../Libraries/LibWeb/HTML/HTMLObjectElement.h | 8 ++- Userland/Libraries/LibWeb/Layout/ImageBox.cpp | 64 ++++--------------- Userland/Libraries/LibWeb/Layout/ImageBox.h | 10 ++- .../Libraries/LibWeb/Layout/ImageProvider.h | 19 ++++++ Userland/Libraries/LibWeb/Loader/Resource.cpp | 1 + .../LibWeb/Painting/ImagePaintable.cpp | 14 +--- 9 files changed, 84 insertions(+), 88 deletions(-) create mode 100644 Userland/Libraries/LibWeb/Layout/ImageProvider.h diff --git a/Userland/Libraries/LibWeb/HTML/HTMLImageElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLImageElement.cpp index f43601585ec..d53d72c9fa4 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLImageElement.cpp +++ b/Userland/Libraries/LibWeb/HTML/HTMLImageElement.cpp @@ -34,7 +34,6 @@ namespace Web::HTML { HTMLImageElement::HTMLImageElement(DOM::Document& document, DOM::QualifiedName qualified_name) : HTMLElement(document, move(qualified_name)) - , m_image_loader(*this) { m_animation_timer = Core::Timer::try_create().release_value_but_fixme_should_propagate_errors(); m_animation_timer->on_timeout = [this] { animate(); }; @@ -104,16 +103,26 @@ void HTMLImageElement::did_remove_attribute(DeprecatedFlyString const& name) JS::GCPtr HTMLImageElement::create_layout_node(NonnullRefPtr style) { - return heap().allocate_without_realm(document(), *this, move(style), m_image_loader); + return heap().allocate_without_realm(document(), *this, move(style), *this); } RefPtr HTMLImageElement::bitmap() const +{ + return current_image_bitmap(); +} + +RefPtr HTMLImageElement::current_image_bitmap() const { if (auto data = m_current_request->image_data()) return data->bitmap(m_current_frame_index); return nullptr; } +void HTMLImageElement::set_visible_in_viewport(bool) +{ + // FIXME: Loosen grip on image data when it's not visible, e.g via volatile memory. +} + // https://html.spec.whatwg.org/multipage/embedded-content.html#dom-img-width unsigned HTMLImageElement::width() const { @@ -130,8 +139,8 @@ unsigned HTMLImageElement::width() const // ...or else the density-corrected intrinsic width and height of the image, in CSS pixels, // if the image has intrinsic dimensions and is available but not being rendered. - if (m_image_loader.has_image()) - return m_image_loader.width(); + if (auto bitmap = current_image_bitmap()) + return bitmap->width(); // ...or else 0, if the image is not available or does not have intrinsic dimensions. return 0; @@ -158,8 +167,8 @@ unsigned HTMLImageElement::height() const // ...or else the density-corrected intrinsic height and height of the image, in CSS pixels, // if the image has intrinsic dimensions and is available but not being rendered. - if (m_image_loader.has_image()) - return m_image_loader.height(); + if (auto bitmap = current_image_bitmap()) + return bitmap->height(); // ...or else 0, if the image is not available or does not have intrinsic dimensions. return 0; @@ -175,8 +184,8 @@ unsigned HTMLImageElement::natural_width() const { // Return the density-corrected intrinsic width of the image, in CSS pixels, // if the image has intrinsic dimensions and is available. - if (m_image_loader.has_image()) - return m_image_loader.width(); + if (auto bitmap = current_image_bitmap()) + return bitmap->width(); // ...or else 0. return 0; @@ -187,8 +196,8 @@ unsigned HTMLImageElement::natural_height() const { // Return the density-corrected intrinsic height of the image, in CSS pixels, // if the image has intrinsic dimensions and is available. - if (m_image_loader.has_image()) - return m_image_loader.height(); + if (auto bitmap = current_image_bitmap()) + return bitmap->height(); // ...or else 0. return 0; @@ -210,7 +219,7 @@ bool HTMLImageElement::complete() const // - The img element's current request's state is completely available and its pending request is null. // - The img element's current request's state is broken and its pending request is null. // FIXME: This is ad-hoc and should be updated once we are loading images via the Fetch mechanism. - if (m_image_loader.has_loaded_or_failed()) + if (auto bitmap = current_image_bitmap()) return true; return false; diff --git a/Userland/Libraries/LibWeb/HTML/HTMLImageElement.h b/Userland/Libraries/LibWeb/HTML/HTMLImageElement.h index c8ff4c83fc6..9f408c080d1 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLImageElement.h +++ b/Userland/Libraries/LibWeb/HTML/HTMLImageElement.h @@ -14,13 +14,14 @@ #include #include #include -#include +#include namespace Web::HTML { class HTMLImageElement final : public HTMLElement - , public FormAssociatedElement { + , public FormAssociatedElement + , public Layout::ImageProvider { WEB_PLATFORM_OBJECT(HTMLImageElement, HTMLElement); FORM_ASSOCIATED_ELEMENT(HTMLElement, HTMLImageElement) @@ -76,6 +77,10 @@ public: // https://html.spec.whatwg.org/multipage/images.html#upgrade-the-pending-request-to-the-current-request void upgrade_pending_request_to_current_request(); + // ^Layout::ImageProvider + virtual RefPtr current_image_bitmap() const override; + virtual void set_visible_in_viewport(bool) override; + private: HTMLImageElement(DOM::Document&, DOM::QualifiedName); @@ -94,8 +99,6 @@ private: size_t m_current_frame_index { 0 }; size_t m_loops_completed { 0 }; - ImageLoader m_image_loader; - Optional m_load_event_delayer; CORSSettingAttribute m_cors_setting { CORSSettingAttribute::NoCORS }; diff --git a/Userland/Libraries/LibWeb/HTML/HTMLObjectElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLObjectElement.cpp index 7dd9496755c..5f131ef349a 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLObjectElement.cpp +++ b/Userland/Libraries/LibWeb/HTML/HTMLObjectElement.cpp @@ -76,7 +76,7 @@ JS::GCPtr HTMLObjectElement::create_layout_node(NonnullRefPtrhas_image()) - return heap().allocate_without_realm(document(), *this, move(style), *m_image_loader); + return heap().allocate_without_realm(document(), *this, move(style), *this); break; default: break; @@ -350,4 +350,16 @@ i32 HTMLObjectElement::default_tab_index_value() const return 0; } +RefPtr HTMLObjectElement::current_image_bitmap() const +{ + if (m_image_loader.has_value()) + return m_image_loader->bitmap(m_image_loader->current_frame_index()); + return nullptr; +} + +void HTMLObjectElement::set_visible_in_viewport(bool) +{ + // FIXME: Loosen grip on image data when it's not visible, e.g via volatile memory. +} + } diff --git a/Userland/Libraries/LibWeb/HTML/HTMLObjectElement.h b/Userland/Libraries/LibWeb/HTML/HTMLObjectElement.h index 631b5b19331..b06d4d07905 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLObjectElement.h +++ b/Userland/Libraries/LibWeb/HTML/HTMLObjectElement.h @@ -11,6 +11,7 @@ #include #include #include +#include #include namespace Web::HTML { @@ -18,7 +19,8 @@ namespace Web::HTML { class HTMLObjectElement final : public NavigableContainer , public FormAssociatedElement - , public ResourceClient { + , public ResourceClient + , public Layout::ImageProvider { WEB_PLATFORM_OBJECT(HTMLObjectElement, NavigableContainer) FORM_ASSOCIATED_ELEMENT(NavigableContainer, HTMLObjectElement) @@ -67,6 +69,10 @@ private: // ^DOM::Element virtual i32 default_tab_index_value() const override; + // ^Layout::ImageProvider + virtual RefPtr current_image_bitmap() const override; + virtual void set_visible_in_viewport(bool) override; + Representation m_representation { Representation::Unknown }; Optional m_image_loader; }; diff --git a/Userland/Libraries/LibWeb/Layout/ImageBox.cpp b/Userland/Libraries/LibWeb/Layout/ImageBox.cpp index 4e83ed0ff98..22770d00e6a 100644 --- a/Userland/Libraries/LibWeb/Layout/ImageBox.cpp +++ b/Userland/Libraries/LibWeb/Layout/ImageBox.cpp @@ -13,69 +13,27 @@ namespace Web::Layout { -ImageBox::ImageBox(DOM::Document& document, DOM::Element& element, NonnullRefPtr style, ImageLoader const& image_loader) +ImageBox::ImageBox(DOM::Document& document, DOM::Element& element, NonnullRefPtr style, ImageProvider const& image_provider) : ReplacedBox(document, element, move(style)) - , m_image_loader(image_loader) + , m_image_provider(image_provider) { } ImageBox::~ImageBox() = default; -int ImageBox::preferred_width() const -{ - return dom_node().attribute(HTML::AttributeNames::width).to_int().value_or(m_image_loader.width()); -} - -int ImageBox::preferred_height() const -{ - return dom_node().attribute(HTML::AttributeNames::height).to_int().value_or(m_image_loader.height()); -} - void ImageBox::prepare_for_replaced_layout() { - HTML::ImageRequest const* image_request = nullptr; - if (is(dom_node())) { - image_request = &static_cast(dom_node()).current_request(); - } + auto bitmap = m_image_provider.current_image_bitmap(); - if (image_request) { - if (!image_request->is_available()) { - set_intrinsic_width(0); - set_intrinsic_height(0); - } else if (auto data = image_request->image_data()) { - auto width = data->natural_width(); - if (width.has_value()) { - set_intrinsic_width(width.value()); - } - auto height = data->natural_height(); - if (height.has_value()) { - set_intrinsic_height(height.value()); - } - - if (width.has_value() && height.has_value() && height.value() != 0) { - set_intrinsic_aspect_ratio(static_cast(width.value()) / static_cast(height.value())); - } else { - set_intrinsic_aspect_ratio({}); - } - } + if (!bitmap) { + set_intrinsic_width(0); + set_intrinsic_height(0); } else { - if (!m_image_loader.has_loaded_or_failed()) { - set_intrinsic_width(0); - set_intrinsic_height(0); - } else { - if (m_image_loader.width()) { - set_intrinsic_width(m_image_loader.width()); - } - if (m_image_loader.height()) { - set_intrinsic_height(m_image_loader.height()); - } - - if (m_image_loader.width() && m_image_loader.height()) { - set_intrinsic_aspect_ratio((float)m_image_loader.width() / (float)m_image_loader.height()); - } else { - set_intrinsic_aspect_ratio({}); - } - } + auto width = bitmap->width(); + auto height = bitmap->height(); + set_intrinsic_width(width); + set_intrinsic_height(height); + set_intrinsic_aspect_ratio(static_cast(width) / static_cast(height)); } if (renders_as_alt_text()) { diff --git a/Userland/Libraries/LibWeb/Layout/ImageBox.h b/Userland/Libraries/LibWeb/Layout/ImageBox.h index f67cde4902a..0d3b2e9718c 100644 --- a/Userland/Libraries/LibWeb/Layout/ImageBox.h +++ b/Userland/Libraries/LibWeb/Layout/ImageBox.h @@ -15,7 +15,7 @@ class ImageBox final : public ReplacedBox { JS_CELL(ImageBox, ReplacedBox); public: - ImageBox(DOM::Document&, DOM::Element&, NonnullRefPtr, ImageLoader const&); + ImageBox(DOM::Document&, DOM::Element&, NonnullRefPtr, ImageProvider const&); virtual ~ImageBox() override; virtual void prepare_for_replaced_layout() override; @@ -26,15 +26,13 @@ public: virtual JS::GCPtr create_paintable() const override; - auto const& image_loader() const { return m_image_loader; } + auto const& image_provider() const { return m_image_provider; } + auto& image_provider() { return m_image_provider; } void dom_node_did_update_alt_text(Badge); private: - int preferred_width() const; - int preferred_height() const; - - ImageLoader const& m_image_loader; + ImageProvider const& m_image_provider; Optional m_cached_alt_text_width; }; diff --git a/Userland/Libraries/LibWeb/Layout/ImageProvider.h b/Userland/Libraries/LibWeb/Layout/ImageProvider.h new file mode 100644 index 00000000000..39537295c27 --- /dev/null +++ b/Userland/Libraries/LibWeb/Layout/ImageProvider.h @@ -0,0 +1,19 @@ +/* + * Copyright (c) 2023, Andreas Kling + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#pragma once + +namespace Web::Layout { + +class ImageProvider { +public: + virtual ~ImageProvider() { } + + virtual RefPtr current_image_bitmap() const = 0; + virtual void set_visible_in_viewport(bool) = 0; +}; + +} diff --git a/Userland/Libraries/LibWeb/Loader/Resource.cpp b/Userland/Libraries/LibWeb/Loader/Resource.cpp index 71ef8cf9a0f..48707a47260 100644 --- a/Userland/Libraries/LibWeb/Loader/Resource.cpp +++ b/Userland/Libraries/LibWeb/Loader/Resource.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include diff --git a/Userland/Libraries/LibWeb/Painting/ImagePaintable.cpp b/Userland/Libraries/LibWeb/Painting/ImagePaintable.cpp index 280108001cc..6b0190b25c7 100644 --- a/Userland/Libraries/LibWeb/Painting/ImagePaintable.cpp +++ b/Userland/Libraries/LibWeb/Painting/ImagePaintable.cpp @@ -61,17 +61,7 @@ void ImagePaintable::paint(PaintContext& context, PaintPhase phase) const if (alt.is_empty()) alt = image_element.src(); context.painter().draw_text(enclosing_rect, alt, Gfx::TextAlignment::Center, computed_values().color(), Gfx::TextElision::Right); - } else if (is(layout_box().dom_node())) { - auto& image_element = static_cast(layout_box().dom_node()); - auto& image_request = image_element.current_request(); - if (auto data = image_request.image_data()) { - auto bitmap = data->bitmap(image_element.current_frame_index()); - VERIFY(bitmap); - auto image_rect = context.rounded_device_rect(absolute_rect()); - ScopedCornerRadiusClip corner_clip { context, context.painter(), image_rect, normalized_border_radii_data(ShrinkRadiiForBorders::Yes) }; - context.painter().draw_scaled_bitmap(image_rect.to_type(), *bitmap, bitmap->rect(), 1.0f, to_gfx_scaling_mode(computed_values().image_rendering())); - } - } else if (auto bitmap = layout_box().image_loader().bitmap(layout_box().image_loader().current_frame_index())) { + } else if (auto bitmap = layout_box().image_provider().current_image_bitmap()) { auto image_rect = context.rounded_device_rect(absolute_rect()); ScopedCornerRadiusClip corner_clip { context, context.painter(), image_rect, normalized_border_radii_data(ShrinkRadiiForBorders::Yes) }; context.painter().draw_scaled_bitmap(image_rect.to_type(), *bitmap, bitmap->rect(), 1.0f, to_gfx_scaling_mode(computed_values().image_rendering())); @@ -81,7 +71,7 @@ void ImagePaintable::paint(PaintContext& context, PaintPhase phase) const void ImagePaintable::browsing_context_did_set_viewport_rect(CSSPixelRect const& viewport_rect) { - layout_box().image_loader().set_visible_in_viewport(viewport_rect.intersects(absolute_rect())); + const_cast(layout_box().image_provider()).set_visible_in_viewport(viewport_rect.intersects(absolute_rect())); } }