From 0264ae23bc3e2a03ce82cf2a5c4b80142862ba82 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Tue, 5 Oct 2021 22:30:53 +0200 Subject: [PATCH] LibWeb: Make CSS layout lazier Instead of doing layout synchronously whenever something changes, we now use a basic event loop timer to defer and coalesce relayouts. If you did something that requires a relayout of the page, make sure to call Document::set_needs_layout() and it will get coalesced with all the other layout updates. There's lots of room for improvement here, but this already makes many web pages significantly snappier. :^) Also, note that this exposes a number of layout bugs where we have been relying on multiple relayouts to calculate the correct dimensions for things. Now that we only do a single layout in many cases, these kind of problems are much more noticeable. That should also make them easier to figure out and fix. :^) --- .../Debugger/EvaluateExpressionDialog.cpp | 2 -- .../Libraries/LibWeb/DOM/CharacterData.cpp | 3 +-- Userland/Libraries/LibWeb/DOM/Document.cpp | 26 ++++++++++++------- Userland/Libraries/LibWeb/DOM/Document.h | 9 ++++--- Userland/Libraries/LibWeb/DOM/Element.cpp | 3 +-- Userland/Libraries/LibWeb/DOM/Node.cpp | 1 - Userland/Libraries/LibWeb/DOM/ShadowRoot.cpp | 1 - .../Libraries/LibWeb/HTML/HTMLElement.cpp | 1 - .../LibWeb/HTML/HTMLImageElement.cpp | 4 +-- .../Libraries/LibWeb/Page/BrowsingContext.cpp | 5 ++++ 10 files changed, 32 insertions(+), 23 deletions(-) diff --git a/Userland/DevTools/HackStudio/Debugger/EvaluateExpressionDialog.cpp b/Userland/DevTools/HackStudio/Debugger/EvaluateExpressionDialog.cpp index 97501295194..23a923880a5 100644 --- a/Userland/DevTools/HackStudio/Debugger/EvaluateExpressionDialog.cpp +++ b/Userland/DevTools/HackStudio/Debugger/EvaluateExpressionDialog.cpp @@ -144,8 +144,6 @@ void EvaluateExpressionDialog::set_output(const StringView& html) paragraph->set_inner_html(html); m_output_container->append_child(paragraph); - m_output_container->document().invalidate_layout(); - m_output_container->document().update_layout(); } } diff --git a/Userland/Libraries/LibWeb/DOM/CharacterData.cpp b/Userland/Libraries/LibWeb/DOM/CharacterData.cpp index d49b0411936..ec96b5d9328 100644 --- a/Userland/Libraries/LibWeb/DOM/CharacterData.cpp +++ b/Userland/Libraries/LibWeb/DOM/CharacterData.cpp @@ -24,8 +24,7 @@ void CharacterData::set_data(String data) if (m_data == data) return; m_data = move(data); - // FIXME: This is definitely too aggressive. - document().schedule_forced_layout(); + set_needs_style_update(true); } } diff --git a/Userland/Libraries/LibWeb/DOM/Document.cpp b/Userland/Libraries/LibWeb/DOM/Document.cpp index 93e5eadb74c..2fb688d00f0 100644 --- a/Userland/Libraries/LibWeb/DOM/Document.cpp +++ b/Userland/Libraries/LibWeb/DOM/Document.cpp @@ -80,7 +80,7 @@ Document::Document(const AK::URL& url) update_style(); }); - m_forced_layout_timer = Core::Timer::create_single_shot(0, [this] { + m_layout_update_timer = Core::Timer::create_single_shot(0, [this] { force_layout(); }); } @@ -164,11 +164,11 @@ void Document::schedule_style_update() m_style_update_timer->start(); } -void Document::schedule_forced_layout() +void Document::schedule_layout_update() { - if (m_forced_layout_timer->is_active()) + if (m_layout_update_timer->is_active()) return; - m_forced_layout_timer->start(); + m_layout_update_timer->start(); } bool Document::is_child_allowed(const Node& node) const @@ -402,25 +402,31 @@ AK::URL Document::parse_url(String const& url) const return m_url.complete_url(url); } -void Document::invalidate_layout() +void Document::set_needs_layout() { - tear_down_layout_tree(); + if (m_needs_layout) + return; + m_needs_layout = true; + schedule_layout_update(); } void Document::force_layout() { - invalidate_layout(); + tear_down_layout_tree(); update_layout(); } void Document::ensure_layout() { - if (!m_layout_root) + if (m_needs_layout || !m_layout_root) update_layout(); } void Document::update_layout() { + if (!m_needs_layout && m_layout_root) + return; + if (!browsing_context()) return; @@ -438,6 +444,8 @@ void Document::update_layout() if (auto* page = this->page()) page->client().page_did_layout(); } + + m_needs_layout = false; } static void update_style_recursively(DOM::Node& node) @@ -461,7 +469,7 @@ void Document::update_style() if (!browsing_context()) return; update_style_recursively(*this); - update_layout(); + set_needs_layout(); } RefPtr Document::create_layout_node() diff --git a/Userland/Libraries/LibWeb/DOM/Document.h b/Userland/Libraries/LibWeb/DOM/Document.h index c78908f15e6..e9623c896cd 100644 --- a/Userland/Libraries/LibWeb/DOM/Document.h +++ b/Userland/Libraries/LibWeb/DOM/Document.h @@ -144,19 +144,20 @@ public: void set_visited_link_color(Color); void force_layout(); - void invalidate_layout(); void ensure_layout(); void update_style(); void update_layout(); + void set_needs_layout(); + virtual bool is_child_allowed(const Node&) const override; const Layout::InitialContainingBlock* layout_node() const; Layout::InitialContainingBlock* layout_node(); void schedule_style_update(); - void schedule_forced_layout(); + void schedule_layout_update(); NonnullRefPtr get_elements_by_name(String const&); NonnullRefPtr get_elements_by_class_name(FlyString const&); @@ -352,7 +353,7 @@ private: Optional m_visited_link_color; RefPtr m_style_update_timer; - RefPtr m_forced_layout_timer; + RefPtr m_layout_update_timer; String m_source; @@ -399,6 +400,8 @@ private: // Used by evaluate_media_queries_and_report_changes(). Vector> m_media_query_lists; + + bool m_needs_layout { false }; }; } diff --git a/Userland/Libraries/LibWeb/DOM/Element.cpp b/Userland/Libraries/LibWeb/DOM/Element.cpp index b6239dcd94c..5045d966505 100644 --- a/Userland/Libraries/LibWeb/DOM/Element.cpp +++ b/Userland/Libraries/LibWeb/DOM/Element.cpp @@ -224,7 +224,7 @@ void Element::recompute_style() return; layout_node()->apply_style(*new_specified_css_values); if (diff == StyleDifference::NeedsRelayout) { - document().schedule_forced_layout(); + document().set_needs_layout(); return; } if (diff == StyleDifference::NeedsRepaint) { @@ -270,7 +270,6 @@ ExceptionOr Element::set_inner_html(String const& markup) return result.exception(); set_needs_style_update(true); - document().invalidate_layout(); return {}; } diff --git a/Userland/Libraries/LibWeb/DOM/Node.cpp b/Userland/Libraries/LibWeb/DOM/Node.cpp index c81705ab6a8..7f76f6d3437 100644 --- a/Userland/Libraries/LibWeb/DOM/Node.cpp +++ b/Userland/Libraries/LibWeb/DOM/Node.cpp @@ -142,7 +142,6 @@ void Node::set_text_content(String const& content) } set_needs_style_update(true); - document().invalidate_layout(); } RefPtr Node::create_layout_node() diff --git a/Userland/Libraries/LibWeb/DOM/ShadowRoot.cpp b/Userland/Libraries/LibWeb/DOM/ShadowRoot.cpp index 675a8a2df29..0f2d1d82233 100644 --- a/Userland/Libraries/LibWeb/DOM/ShadowRoot.cpp +++ b/Userland/Libraries/LibWeb/DOM/ShadowRoot.cpp @@ -49,7 +49,6 @@ ExceptionOr ShadowRoot::set_inner_html(String const& markup) return result.exception(); set_needs_style_update(true); - document().invalidate_layout(); return {}; } diff --git a/Userland/Libraries/LibWeb/HTML/HTMLElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLElement.cpp index cb01c626d87..21f350445f8 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLElement.cpp +++ b/Userland/Libraries/LibWeb/HTML/HTMLElement.cpp @@ -97,7 +97,6 @@ void HTMLElement::set_inner_text(StringView text) append_child(document().create_text_node(text)); set_needs_style_update(true); - document().invalidate_layout(); } String HTMLElement::inner_text() diff --git a/Userland/Libraries/LibWeb/HTML/HTMLImageElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLImageElement.cpp index 14e79b1a875..4d6c32adb13 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLImageElement.cpp +++ b/Userland/Libraries/LibWeb/HTML/HTMLImageElement.cpp @@ -21,7 +21,7 @@ HTMLImageElement::HTMLImageElement(DOM::Document& document, QualifiedName qualif , m_image_loader(*this) { m_image_loader.on_load = [this] { - this->document().update_layout(); + this->document().schedule_style_update(); queue_an_element_task(HTML::Task::Source::DOMManipulation, [this] { dispatch_event(DOM::Event::create(EventNames::load)); }); @@ -29,7 +29,7 @@ HTMLImageElement::HTMLImageElement(DOM::Document& document, QualifiedName qualif m_image_loader.on_fail = [this] { dbgln("HTMLImageElement: Resource did fail: {}", src()); - this->document().update_layout(); + this->document().schedule_style_update(); queue_an_element_task(HTML::Task::Source::DOMManipulation, [this] { dispatch_event(DOM::Event::create(EventNames::error)); }); diff --git a/Userland/Libraries/LibWeb/Page/BrowsingContext.cpp b/Userland/Libraries/LibWeb/Page/BrowsingContext.cpp index 653f2cc1fd1..448f39d8d6e 100644 --- a/Userland/Libraries/LibWeb/Page/BrowsingContext.cpp +++ b/Userland/Libraries/LibWeb/Page/BrowsingContext.cpp @@ -82,6 +82,8 @@ void BrowsingContext::set_viewport_rect(Gfx::IntRect const& rect) if (m_size != rect.size()) { m_size = rect.size(); + if (auto* document = active_document()) + document->set_needs_layout(); did_change = true; } @@ -105,6 +107,9 @@ void BrowsingContext::set_size(Gfx::IntSize const& size) return; m_size = size; + if (auto* document = active_document()) + document->set_needs_layout(); + for (auto* client : m_viewport_clients) client->browsing_context_did_set_viewport_rect(viewport_rect());