From 7c2713c14f1a83325bb61eb25926122f29cba908 Mon Sep 17 00:00:00 2001 From: Aliaksandr Kalenik Date: Sun, 14 Jan 2024 13:46:52 +0100 Subject: [PATCH] LibWeb: Move set_needs_display() from layout node to paintable For this method, there is no need to go through the layout node when we can directly reach the paintable. --- Userland/Libraries/LibWeb/DOM/Document.cpp | 20 ++++++------- Userland/Libraries/LibWeb/DOM/Element.cpp | 4 +-- Userland/Libraries/LibWeb/DOM/Range.cpp | 5 ++-- Userland/Libraries/LibWeb/HTML/AudioTrack.cpp | 5 ++-- .../Libraries/LibWeb/HTML/BrowsingContext.cpp | 16 +++++----- .../LibWeb/HTML/CanvasRenderingContext2D.cpp | 5 ++-- .../Libraries/LibWeb/HTML/HTMLBodyElement.cpp | 5 ++-- .../LibWeb/HTML/HTMLImageElement.cpp | 4 +-- .../LibWeb/HTML/HTMLMediaElement.cpp | 21 ++++++------- .../LibWeb/HTML/HTMLVideoElement.cpp | 5 ++-- Userland/Libraries/LibWeb/HTML/Navigable.cpp | 4 +-- Userland/Libraries/LibWeb/Layout/Box.cpp | 9 ------ Userland/Libraries/LibWeb/Layout/Box.h | 2 -- Userland/Libraries/LibWeb/Layout/Node.cpp | 25 ---------------- Userland/Libraries/LibWeb/Layout/Node.h | 2 -- .../Libraries/LibWeb/Painting/Paintable.cpp | 30 +++++++++++++++++++ .../Libraries/LibWeb/Painting/Paintable.h | 4 ++- .../LibWeb/Painting/PaintableBox.cpp | 8 ++++- .../Libraries/LibWeb/Painting/PaintableBox.h | 2 ++ .../LibWeb/Painting/PaintableFragment.cpp | 3 +- .../WebGL/WebGLRenderingContextBase.cpp | 5 ++-- 21 files changed, 97 insertions(+), 87 deletions(-) diff --git a/Userland/Libraries/LibWeb/DOM/Document.cpp b/Userland/Libraries/LibWeb/DOM/Document.cpp index bcbda66e040..2a108358592 100644 --- a/Userland/Libraries/LibWeb/DOM/Document.cpp +++ b/Userland/Libraries/LibWeb/DOM/Document.cpp @@ -1177,14 +1177,14 @@ void Document::set_inspected_node(Node* node, Optionalset_needs_display(); + if (auto layout_node = inspected_layout_node(); layout_node && layout_node->paintable()) + layout_node->paintable()->set_needs_display(); m_inspected_node = node; m_inspected_pseudo_element = pseudo_element; - if (auto layout_node = inspected_layout_node()) - layout_node->set_needs_display(); + if (auto layout_node = inspected_layout_node(); layout_node && layout_node->paintable()) + layout_node->paintable()->set_needs_display(); } Layout::Node* Document::inspected_layout_node() @@ -1736,8 +1736,8 @@ void Document::set_focused_element(Element* element) m_focused_element->set_needs_style_update(true); } - if (m_layout_root) - m_layout_root->set_needs_display(); + if (paintable()) + paintable()->set_needs_display(); // Scroll the viewport if necessary to make the newly focused element visible. if (m_focused_element) { @@ -1755,8 +1755,8 @@ void Document::set_active_element(Element* element) m_active_element = element; - if (m_layout_root) - m_layout_root->set_needs_display(); + if (paintable()) + paintable()->set_needs_display(); } void Document::set_target_element(Element* element) @@ -1772,8 +1772,8 @@ void Document::set_target_element(Element* element) if (m_target_element) m_target_element->set_needs_style_update(true); - if (m_layout_root) - m_layout_root->set_needs_display(); + if (paintable()) + paintable()->set_needs_display(); } // https://html.spec.whatwg.org/multipage/browsing-the-web.html#the-indicated-part-of-the-document diff --git a/Userland/Libraries/LibWeb/DOM/Element.cpp b/Userland/Libraries/LibWeb/DOM/Element.cpp index efcf598e2b9..9bbf105abaf 100644 --- a/Userland/Libraries/LibWeb/DOM/Element.cpp +++ b/Userland/Libraries/LibWeb/DOM/Element.cpp @@ -602,8 +602,8 @@ Element::RequiredInvalidationAfterStyleChange Element::recompute_style() if (!invalidation.rebuild_layout_tree && layout_node()) { // If we're keeping the layout tree, we can just apply the new style to the existing layout tree. layout_node()->apply_style(*m_computed_css_values); - if (invalidation.repaint) - layout_node()->set_needs_display(); + if (invalidation.repaint && paintable()) + paintable()->set_needs_display(); } return invalidation; diff --git a/Userland/Libraries/LibWeb/DOM/Range.cpp b/Userland/Libraries/LibWeb/DOM/Range.cpp index 191cfa6f03c..365dab2f0e8 100644 --- a/Userland/Libraries/LibWeb/DOM/Range.cpp +++ b/Userland/Libraries/LibWeb/DOM/Range.cpp @@ -22,6 +22,7 @@ #include #include #include +#include namespace Web::DOM { @@ -94,9 +95,9 @@ void Range::update_associated_selection() { if (!m_associated_selection) return; - if (auto* layout_root = m_associated_selection->document()->layout_node()) { + if (auto* layout_root = m_associated_selection->document()->layout_node(); layout_root && layout_root->paintable()) { layout_root->recompute_selection_states(); - layout_root->set_needs_display(); + layout_root->paintable()->set_needs_display(); } } diff --git a/Userland/Libraries/LibWeb/HTML/AudioTrack.cpp b/Userland/Libraries/LibWeb/HTML/AudioTrack.cpp index 78bec9419d7..dce51667c8b 100644 --- a/Userland/Libraries/LibWeb/HTML/AudioTrack.cpp +++ b/Userland/Libraries/LibWeb/HTML/AudioTrack.cpp @@ -16,6 +16,7 @@ #include #include #include +#include #include namespace Web::HTML { @@ -30,8 +31,8 @@ AudioTrack::AudioTrack(JS::Realm& realm, JS::NonnullGCPtr medi , m_audio_plugin(Platform::AudioCodecPlugin::create(move(loader)).release_value_but_fixme_should_propagate_errors()) { m_audio_plugin->on_playback_position_updated = [this](auto position) { - if (auto* layout_node = m_media_element->layout_node()) - layout_node->set_needs_display(); + if (auto const* paintable = m_media_element->paintable()) + paintable->set_needs_display(); auto playback_position = static_cast(position.to_milliseconds()) / 1000.0; m_media_element->set_current_playback_position(playback_position); diff --git a/Userland/Libraries/LibWeb/HTML/BrowsingContext.cpp b/Userland/Libraries/LibWeb/HTML/BrowsingContext.cpp index 2c8c1550a99..11c96990d6c 100644 --- a/Userland/Libraries/LibWeb/HTML/BrowsingContext.cpp +++ b/Userland/Libraries/LibWeb/HTML/BrowsingContext.cpp @@ -273,9 +273,9 @@ BrowsingContext::BrowsingContext(JS::NonnullGCPtr page, HTML::NavigableCon m_cursor_blink_timer = Core::Timer::create_repeating(500, [this] { if (!is_focused_context()) return; - if (m_cursor_position && m_cursor_position->node()->layout_node()) { + if (m_cursor_position && m_cursor_position->node()->layout_node() && m_cursor_position->node()->layout_node()->paintable()) { m_cursor_blink_state = !m_cursor_blink_state; - m_cursor_position->node()->layout_node()->set_needs_display(); + m_cursor_position->node()->paintable()->set_needs_display(); } }).release_value_but_fixme_should_propagate_errors(); } @@ -327,8 +327,8 @@ void BrowsingContext::reset_cursor_blink_cycle() { m_cursor_blink_state = true; m_cursor_blink_timer->restart(); - if (m_cursor_position && m_cursor_position->node()->layout_node()) - m_cursor_position->node()->layout_node()->set_needs_display(); + if (m_cursor_position && m_cursor_position->node()->paintable()) + m_cursor_position->node()->paintable()->set_needs_display(); } // https://html.spec.whatwg.org/multipage/browsers.html#top-level-browsing-context @@ -404,13 +404,13 @@ void BrowsingContext::set_cursor_position(JS::NonnullGCPtr positi if (m_cursor_position && m_cursor_position->equals(position)) return; - if (m_cursor_position && m_cursor_position->node()->layout_node()) - m_cursor_position->node()->layout_node()->set_needs_display(); + if (m_cursor_position && m_cursor_position->node()->paintable()) + m_cursor_position->node()->paintable()->set_needs_display(); m_cursor_position = position; - if (m_cursor_position && m_cursor_position->node()->layout_node()) - m_cursor_position->node()->layout_node()->set_needs_display(); + if (m_cursor_position && m_cursor_position->node()->paintable()) + m_cursor_position->node()->paintable()->set_needs_display(); reset_cursor_blink_cycle(); } diff --git a/Userland/Libraries/LibWeb/HTML/CanvasRenderingContext2D.cpp b/Userland/Libraries/LibWeb/HTML/CanvasRenderingContext2D.cpp index 54d8e47033c..7d769c20d02 100644 --- a/Userland/Libraries/LibWeb/HTML/CanvasRenderingContext2D.cpp +++ b/Userland/Libraries/LibWeb/HTML/CanvasRenderingContext2D.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include #include @@ -174,9 +175,9 @@ WebIDL::ExceptionOr CanvasRenderingContext2D::draw_image_internal(CanvasIm void CanvasRenderingContext2D::did_draw(Gfx::FloatRect const&) { // FIXME: Make use of the rect to reduce the invalidated area when possible. - if (!canvas_element().layout_node()) + if (!canvas_element().paintable()) return; - canvas_element().layout_node()->set_needs_display(); + canvas_element().paintable()->set_needs_display(); } Gfx::Painter* CanvasRenderingContext2D::painter() diff --git a/Userland/Libraries/LibWeb/HTML/HTMLBodyElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLBodyElement.cpp index 72f3df83699..b6035225e9e 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLBodyElement.cpp +++ b/Userland/Libraries/LibWeb/HTML/HTMLBodyElement.cpp @@ -12,6 +12,7 @@ #include #include #include +#include namespace Web::HTML { @@ -78,8 +79,8 @@ void HTMLBodyElement::attribute_changed(FlyString const& name, Optional } else if (name.equals_ignoring_ascii_case("background"sv)) { m_background_style_value = CSS::ImageStyleValue::create(document().parse_url(value.value_or(String {}))); m_background_style_value->on_animate = [this] { - if (layout_node()) { - layout_node()->set_needs_display(); + if (paintable()) { + paintable()->set_needs_display(); } }; } diff --git a/Userland/Libraries/LibWeb/HTML/HTMLImageElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLImageElement.cpp index ccf25fe743b..f05f33df208 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLImageElement.cpp +++ b/Userland/Libraries/LibWeb/HTML/HTMLImageElement.cpp @@ -1005,8 +1005,8 @@ void HTMLImageElement::animate() } } - if (layout_node()) - layout_node()->set_needs_display(); + if (paintable()) + paintable()->set_needs_display(); } } diff --git a/Userland/Libraries/LibWeb/HTML/HTMLMediaElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLMediaElement.cpp index 4f2b6910eb3..5fa5b269d09 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLMediaElement.cpp +++ b/Userland/Libraries/LibWeb/HTML/HTMLMediaElement.cpp @@ -35,6 +35,7 @@ #include #include #include +#include #include namespace Web::HTML { @@ -325,8 +326,8 @@ void HTMLMediaElement::set_duration(double duration) m_duration = duration; - if (auto* layout_node = this->layout_node()) - layout_node->set_needs_display(); + if (auto* paintable = this->paintable()) + paintable->set_needs_display(); } WebIDL::ExceptionOr> HTMLMediaElement::play() @@ -424,8 +425,8 @@ void HTMLMediaElement::volume_or_muted_attribute_changed() // FIXME: Then, if the media element is not allowed to play, the user agent must run the internal pause steps for the media element. - if (auto* layout_node = this->layout_node()) - layout_node->set_needs_display(); + if (auto* paintable = this->paintable()) + paintable->set_needs_display(); on_volume_change(); } @@ -1586,8 +1587,8 @@ void HTMLMediaElement::set_show_poster(bool show_poster) m_show_poster = show_poster; - if (auto* layout_node = this->layout_node()) - layout_node->set_needs_display(); + if (auto* paintable = this->paintable()) + paintable->set_needs_display(); } void HTMLMediaElement::set_paused(bool paused) @@ -1600,8 +1601,8 @@ void HTMLMediaElement::set_paused(bool paused) if (m_paused) on_paused(); - if (auto* layout_node = this->layout_node()) - layout_node->set_needs_display(); + if (auto* paintable = this->paintable()) + paintable->set_needs_display(); set_needs_style_update(true); } @@ -1928,8 +1929,8 @@ void HTMLMediaElement::set_layout_display_time(Badge, m_display_time = move(display_time); - if (auto* layout_node = this->layout_node()) - layout_node->set_needs_display(); + if (auto* paintable = this->paintable()) + paintable->set_needs_display(); } double HTMLMediaElement::layout_display_time(Badge) const diff --git a/Userland/Libraries/LibWeb/HTML/HTMLVideoElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLVideoElement.cpp index d1f9e7fbd4a..b4f8ad94e0a 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLVideoElement.cpp +++ b/Userland/Libraries/LibWeb/HTML/HTMLVideoElement.cpp @@ -16,6 +16,7 @@ #include #include #include +#include #include namespace Web::HTML { @@ -102,8 +103,8 @@ void HTMLVideoElement::set_video_track(JS::GCPtr video_track) void HTMLVideoElement::set_current_frame(Badge, RefPtr frame, double position) { m_current_frame = { move(frame), position }; - if (layout_node()) - layout_node()->set_needs_display(); + if (paintable()) + paintable()->set_needs_display(); } void HTMLVideoElement::on_playing() diff --git a/Userland/Libraries/LibWeb/HTML/Navigable.cpp b/Userland/Libraries/LibWeb/HTML/Navigable.cpp index 951845a1de6..f7a081c0344 100644 --- a/Userland/Libraries/LibWeb/HTML/Navigable.cpp +++ b/Userland/Libraries/LibWeb/HTML/Navigable.cpp @@ -1990,8 +1990,8 @@ void Navigable::set_needs_display(CSSPixelRect const& rect) return; } - if (container() && container()->layout_node()) - container()->layout_node()->set_needs_display(); + if (container() && container()->paintable()) + container()->paintable()->set_needs_display(); } // https://html.spec.whatwg.org/#rendering-opportunity diff --git a/Userland/Libraries/LibWeb/Layout/Box.cpp b/Userland/Libraries/LibWeb/Layout/Box.cpp index bb991a08cd5..9b5041c1612 100644 --- a/Userland/Libraries/LibWeb/Layout/Box.cpp +++ b/Userland/Libraries/LibWeb/Layout/Box.cpp @@ -60,15 +60,6 @@ bool Box::is_user_scrollable() const return computed_values().overflow_y() == CSS::Overflow::Scroll || computed_values().overflow_y() == CSS::Overflow::Auto; } -void Box::set_needs_display() -{ - if (!navigable()) - return; - - if (paintable_box()) - navigable()->set_needs_display(paintable_box()->absolute_rect()); -} - bool Box::is_body() const { return dom_node() && dom_node() == document().body(); diff --git a/Userland/Libraries/LibWeb/Layout/Box.h b/Userland/Libraries/LibWeb/Layout/Box.h index eff78c4b6b4..69298e271ca 100644 --- a/Userland/Libraries/LibWeb/Layout/Box.h +++ b/Userland/Libraries/LibWeb/Layout/Box.h @@ -25,8 +25,6 @@ public: Painting::PaintableBox const* paintable_box() const; Painting::PaintableBox* paintable_box(); - virtual void set_needs_display() override; - bool is_body() const; // https://www.w3.org/TR/css-images-3/#natural-dimensions diff --git a/Userland/Libraries/LibWeb/Layout/Node.cpp b/Userland/Libraries/LibWeb/Layout/Node.cpp index 51dfaf14f86..72a611bc8ef 100644 --- a/Userland/Libraries/LibWeb/Layout/Node.cpp +++ b/Userland/Libraries/LibWeb/Layout/Node.cpp @@ -226,31 +226,6 @@ Viewport& Node::root() return *document().layout_node(); } -void Node::set_needs_display() -{ - auto* containing_block = this->containing_block(); - if (!containing_block) - return; - if (!containing_block->paintable_box()) - return; - auto navigable = this->navigable(); - if (!navigable) - return; - - if (this->paintable() && is(this->paintable())) { - auto const& fragments = static_cast(this->paintable())->fragments(); - for (auto const& fragment : fragments) - navigable->set_needs_display(fragment.absolute_rect()); - } - - if (!is(*containing_block->paintable_box())) - return; - static_cast(*containing_block->paintable_box()).for_each_fragment([&](auto& fragment) { - navigable->set_needs_display(fragment.absolute_rect()); - return IterationDecision::Continue; - }); -} - bool Node::is_floating() const { if (!has_style()) diff --git a/Userland/Libraries/LibWeb/Layout/Node.h b/Userland/Libraries/LibWeb/Layout/Node.h index bd6136636e0..a1549ffaaaa 100644 --- a/Userland/Libraries/LibWeb/Layout/Node.h +++ b/Userland/Libraries/LibWeb/Layout/Node.h @@ -155,8 +155,6 @@ public: void removed_from(Node&) { } void children_changed() { } - virtual void set_needs_display(); - bool children_are_inline() const { return m_children_are_inline; } void set_children_are_inline(bool value) { m_children_are_inline = value; } diff --git a/Userland/Libraries/LibWeb/Painting/Paintable.cpp b/Userland/Libraries/LibWeb/Painting/Paintable.cpp index 97271834434..452fa2c4490 100644 --- a/Userland/Libraries/LibWeb/Painting/Paintable.cpp +++ b/Userland/Libraries/LibWeb/Painting/Paintable.cpp @@ -73,6 +73,11 @@ HTML::BrowsingContext& Paintable::browsing_context() return m_browsing_context; } +JS::GCPtr Paintable::navigable() const +{ + return document().navigable(); +} + Paintable::DispatchEventOfSameName Paintable::handle_mousedown(Badge, CSSPixelPoint, unsigned, unsigned) { return DispatchEventOfSameName::Yes; @@ -118,6 +123,31 @@ void Paintable::invalidate_stacking_context() m_stacking_context = nullptr; } +void Paintable::set_needs_display() const +{ + auto* containing_block = this->containing_block(); + if (!containing_block) + return; + if (!containing_block->paintable_box()) + return; + auto navigable = this->navigable(); + if (!navigable) + return; + + if (is(*this)) { + auto const& fragments = static_cast(this)->fragments(); + for (auto const& fragment : fragments) + navigable->set_needs_display(fragment.absolute_rect()); + } + + if (!is(*containing_block->paintable_box())) + return; + static_cast(*containing_block->paintable_box()).for_each_fragment([&](auto& fragment) { + navigable->set_needs_display(fragment.absolute_rect()); + return IterationDecision::Continue; + }); +} + PaintableBox const* Paintable::nearest_scrollable_ancestor_within_stacking_context() const { auto* ancestor = parent(); diff --git a/Userland/Libraries/LibWeb/Painting/Paintable.h b/Userland/Libraries/LibWeb/Painting/Paintable.h index 9f517e5b659..41810b17b7a 100644 --- a/Userland/Libraries/LibWeb/Painting/Paintable.h +++ b/Userland/Libraries/LibWeb/Painting/Paintable.h @@ -165,7 +165,9 @@ public: [[nodiscard]] HTML::BrowsingContext const& browsing_context() const; [[nodiscard]] HTML::BrowsingContext& browsing_context(); - void set_needs_display() const { const_cast(*m_layout_node).set_needs_display(); } + JS::GCPtr navigable() const; + + virtual void set_needs_display() const; Layout::Box const* containing_block() const { diff --git a/Userland/Libraries/LibWeb/Painting/PaintableBox.cpp b/Userland/Libraries/LibWeb/Painting/PaintableBox.cpp index 3222ff982ca..29707cb8b6c 100644 --- a/Userland/Libraries/LibWeb/Painting/PaintableBox.cpp +++ b/Userland/Libraries/LibWeb/Painting/PaintableBox.cpp @@ -80,7 +80,7 @@ void PaintableBox::set_scroll_offset(CSSPixelPoint offset) return; } - node.set_needs_display(); + set_needs_display(); } void PaintableBox::scroll_by(int delta_x, int delta_y) @@ -790,4 +790,10 @@ Optional PaintableWithLines::hit_test(CSSPixelPoint position, Hit return {}; } +void PaintableBox::set_needs_display() const +{ + if (auto navigable = this->navigable()) + navigable->set_needs_display(absolute_rect()); +} + } diff --git a/Userland/Libraries/LibWeb/Painting/PaintableBox.h b/Userland/Libraries/LibWeb/Painting/PaintableBox.h index 7ec80043442..8e89f09209b 100644 --- a/Userland/Libraries/LibWeb/Painting/PaintableBox.h +++ b/Userland/Libraries/LibWeb/Painting/PaintableBox.h @@ -124,6 +124,8 @@ public: DOM::Node const* dom_node() const { return layout_box().dom_node(); } DOM::Node* dom_node() { return layout_box().dom_node(); } + virtual void set_needs_display() const override; + virtual void apply_scroll_offset(PaintContext&, PaintPhase) const override; virtual void reset_scroll_offset(PaintContext&, PaintPhase) const override; diff --git a/Userland/Libraries/LibWeb/Painting/PaintableFragment.cpp b/Userland/Libraries/LibWeb/Painting/PaintableFragment.cpp index 828d6eb86b1..2f15bbbc2de 100644 --- a/Userland/Libraries/LibWeb/Painting/PaintableFragment.cpp +++ b/Userland/Libraries/LibWeb/Painting/PaintableFragment.cpp @@ -24,7 +24,8 @@ PaintableFragment::PaintableFragment(Layout::LineBoxFragment const& fragment) CSSPixelRect const PaintableFragment::absolute_rect() const { CSSPixelRect rect { {}, size() }; - rect.set_location(m_layout_node->containing_block()->paintable_box()->absolute_position()); + if (m_layout_node->containing_block() && m_layout_node->containing_block()->paintable_box()) + rect.set_location(m_layout_node->containing_block()->paintable_box()->absolute_position()); rect.translate_by(offset()); return rect; } diff --git a/Userland/Libraries/LibWeb/WebGL/WebGLRenderingContextBase.cpp b/Userland/Libraries/LibWeb/WebGL/WebGLRenderingContextBase.cpp index 67432ced422..ea898634de8 100644 --- a/Userland/Libraries/LibWeb/WebGL/WebGLRenderingContextBase.cpp +++ b/Userland/Libraries/LibWeb/WebGL/WebGLRenderingContextBase.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include namespace Web::WebGL { @@ -99,9 +100,9 @@ void WebGLRenderingContextBase::needs_to_present() { m_should_present = true; - if (!canvas_element().layout_node()) + if (!canvas_element().paintable()) return; - canvas_element().layout_node()->set_needs_display(); + canvas_element().paintable()->set_needs_display(); } void WebGLRenderingContextBase::set_error(GLenum error)