Browse Source

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.
Andreas Kling 2 years ago
parent
commit
c648e24cff

+ 20 - 11
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<Layout::Node> HTMLImageElement::create_layout_node(NonnullRefPtr<CSS::StyleProperties> style)
 {
-    return heap().allocate_without_realm<Layout::ImageBox>(document(), *this, move(style), m_image_loader);
+    return heap().allocate_without_realm<Layout::ImageBox>(document(), *this, move(style), *this);
 }
 
 RefPtr<Gfx::Bitmap const> HTMLImageElement::bitmap() const
+{
+    return current_image_bitmap();
+}
+
+RefPtr<Gfx::Bitmap const> 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;

+ 7 - 4
Userland/Libraries/LibWeb/HTML/HTMLImageElement.h

@@ -14,13 +14,14 @@
 #include <LibWeb/HTML/FormAssociatedElement.h>
 #include <LibWeb/HTML/HTMLElement.h>
 #include <LibWeb/HTML/SourceSet.h>
-#include <LibWeb/Loader/ImageLoader.h>
+#include <LibWeb/Layout/ImageProvider.h>
 
 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<Gfx::Bitmap const> 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<DOM::DocumentLoadEventDelayer> m_load_event_delayer;
 
     CORSSettingAttribute m_cors_setting { CORSSettingAttribute::NoCORS };

+ 13 - 1
Userland/Libraries/LibWeb/HTML/HTMLObjectElement.cpp

@@ -76,7 +76,7 @@ JS::GCPtr<Layout::Node> HTMLObjectElement::create_layout_node(NonnullRefPtr<CSS:
         return nullptr;
     case Representation::Image:
         if (m_image_loader.has_value() && m_image_loader->has_image())
-            return heap().allocate_without_realm<Layout::ImageBox>(document(), *this, move(style), *m_image_loader);
+            return heap().allocate_without_realm<Layout::ImageBox>(document(), *this, move(style), *this);
         break;
     default:
         break;
@@ -350,4 +350,16 @@ i32 HTMLObjectElement::default_tab_index_value() const
     return 0;
 }
 
+RefPtr<Gfx::Bitmap const> 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.
+}
+
 }

+ 7 - 1
Userland/Libraries/LibWeb/HTML/HTMLObjectElement.h

@@ -11,6 +11,7 @@
 #include <LibWeb/HTML/FormAssociatedElement.h>
 #include <LibWeb/HTML/HTMLElement.h>
 #include <LibWeb/HTML/NavigableContainer.h>
+#include <LibWeb/Layout/ImageProvider.h>
 #include <LibWeb/Loader/ImageLoader.h>
 
 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<Gfx::Bitmap const> current_image_bitmap() const override;
+    virtual void set_visible_in_viewport(bool) override;
+
     Representation m_representation { Representation::Unknown };
     Optional<ImageLoader> m_image_loader;
 };

+ 11 - 53
Userland/Libraries/LibWeb/Layout/ImageBox.cpp

@@ -13,69 +13,27 @@
 
 namespace Web::Layout {
 
-ImageBox::ImageBox(DOM::Document& document, DOM::Element& element, NonnullRefPtr<CSS::StyleProperties> style, ImageLoader const& image_loader)
+ImageBox::ImageBox(DOM::Document& document, DOM::Element& element, NonnullRefPtr<CSS::StyleProperties> 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<HTML::HTMLImageElement>(dom_node())) {
-        image_request = &static_cast<HTML::HTMLImageElement const&>(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<float>(width.value()) / static_cast<float>(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<float>(width) / static_cast<float>(height));
     }
 
     if (renders_as_alt_text()) {

+ 4 - 6
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<CSS::StyleProperties>, ImageLoader const&);
+    ImageBox(DOM::Document&, DOM::Element&, NonnullRefPtr<CSS::StyleProperties>, ImageProvider const&);
     virtual ~ImageBox() override;
 
     virtual void prepare_for_replaced_layout() override;
@@ -26,15 +26,13 @@ public:
 
     virtual JS::GCPtr<Painting::Paintable> 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<HTML::HTMLImageElement>);
 
 private:
-    int preferred_width() const;
-    int preferred_height() const;
-
-    ImageLoader const& m_image_loader;
+    ImageProvider const& m_image_provider;
 
     Optional<CSSPixels> m_cached_alt_text_width;
 };

+ 19 - 0
Userland/Libraries/LibWeb/Layout/ImageProvider.h

@@ -0,0 +1,19 @@
+/*
+ * Copyright (c) 2023, Andreas Kling <kling@serenityos.org>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#pragma once
+
+namespace Web::Layout {
+
+class ImageProvider {
+public:
+    virtual ~ImageProvider() { }
+
+    virtual RefPtr<Gfx::Bitmap const> current_image_bitmap() const = 0;
+    virtual void set_visible_in_viewport(bool) = 0;
+};
+
+}

+ 1 - 0
Userland/Libraries/LibWeb/Loader/Resource.cpp

@@ -9,6 +9,7 @@
 #include <LibCore/MimeData.h>
 #include <LibTextCodec/Decoder.h>
 #include <LibWeb/HTML/HTMLImageElement.h>
+#include <LibWeb/Loader/ImageResource.h>
 #include <LibWeb/Loader/Resource.h>
 #include <LibWeb/Loader/ResourceLoader.h>
 #include <LibWeb/Platform/EventLoopPlugin.h>

+ 2 - 12
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<HTML::HTMLImageElement>(layout_box().dom_node())) {
-            auto& image_element = static_cast<HTML::HTMLImageElement const&>(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<int>(), *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<int>(), *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::ImageProvider&>(layout_box().image_provider()).set_visible_in_viewport(viewport_rect.intersects(absolute_rect()));
 }
 
 }