mirror of
https://github.com/LadybirdBrowser/ladybird.git
synced 2025-04-21 20:15:17 +00:00
LibWeb: Fix infinite repaint loop when cached display list is used
Before this change, `m_needs_repaint` was reset in `Document::record_display_list()` only when the cached display list was absent. This meant that if the last triggered repaint used the cached display list, we would keep repainting indefinitely until the display list was invalidated (We schedule a task that checks if repainting is required 60/s). This change also moves `m_needs_repaint` from Document to TraversableNavigable as we only ever need to repaint a document that belongs to traversable.
This commit is contained in:
parent
0cfe90b59e
commit
0c5b61b7e1
Notes:
github-actions[bot]
2025-02-01 22:32:14 +00:00
Author: https://github.com/kalenikaliaksandr Commit: https://github.com/LadybirdBrowser/ladybird/commit/0c5b61b7e14 Pull-request: https://github.com/LadybirdBrowser/ladybird/pull/3425 Reviewed-by: https://github.com/awesomekling ✅
9 changed files with 17 additions and 25 deletions
|
@ -6093,8 +6093,6 @@ void Document::set_needs_display(CSSPixelRect const&, InvalidateDisplayList shou
|
|||
// FIXME: Ignore updates outside the visible viewport rect.
|
||||
// This requires accounting for fixed-position elements in the input rect, which we don't do yet.
|
||||
|
||||
m_needs_repaint = true;
|
||||
|
||||
if (should_invalidate_display_list == InvalidateDisplayList::Yes) {
|
||||
invalidate_display_list();
|
||||
}
|
||||
|
@ -6104,6 +6102,7 @@ void Document::set_needs_display(CSSPixelRect const&, InvalidateDisplayList shou
|
|||
return;
|
||||
|
||||
if (navigable->is_traversable()) {
|
||||
navigable->traversable_navigable()->set_needs_repaint();
|
||||
Web::HTML::main_thread_event_loop().schedule();
|
||||
return;
|
||||
}
|
||||
|
@ -6188,8 +6187,6 @@ RefPtr<Painting::DisplayList> Document::record_display_list(PaintConfig config)
|
|||
display_list->set_device_pixels_per_css_pixel(page().client().device_pixels_per_css_pixel());
|
||||
display_list->set_scroll_state(viewport_paintable.scroll_state());
|
||||
|
||||
m_needs_repaint = false;
|
||||
|
||||
m_cached_display_list = display_list;
|
||||
m_cached_display_list_paint_config = config;
|
||||
|
||||
|
|
|
@ -735,7 +735,6 @@ public:
|
|||
GC::Ptr<HTML::Navigable> cached_navigable();
|
||||
void set_cached_navigable(GC::Ptr<HTML::Navigable>);
|
||||
|
||||
[[nodiscard]] bool needs_repaint() const { return m_needs_repaint; }
|
||||
void set_needs_display(InvalidateDisplayList = InvalidateDisplayList::Yes);
|
||||
void set_needs_display(CSSPixelRect const&, InvalidateDisplayList = InvalidateDisplayList::Yes);
|
||||
|
||||
|
@ -1101,8 +1100,6 @@ private:
|
|||
// NOTE: This is WeakPtr, not GCPtr, on purpose. We don't want the document to keep some old detached navigable alive.
|
||||
WeakPtr<HTML::Navigable> m_cached_navigable;
|
||||
|
||||
bool m_needs_repaint { false };
|
||||
|
||||
bool m_enable_cookies_on_file_domains { false };
|
||||
|
||||
Optional<PaintConfig> m_cached_display_list_paint_config;
|
||||
|
|
|
@ -411,13 +411,13 @@ void EventLoop::update_the_rendering()
|
|||
for (auto& document : docs) {
|
||||
document->page().client().process_screenshot_requests();
|
||||
auto navigable = document->navigable();
|
||||
if (navigable && document->needs_repaint()) {
|
||||
auto* browsing_context = document->browsing_context();
|
||||
auto& page = browsing_context->page();
|
||||
if (navigable->is_traversable()) {
|
||||
VERIFY(page.client().is_ready_to_paint());
|
||||
page.client().paint_next_frame();
|
||||
}
|
||||
if (!navigable->is_traversable())
|
||||
continue;
|
||||
auto traversable = navigable->traversable_navigable();
|
||||
if (traversable && traversable->needs_repaint()) {
|
||||
auto& page = traversable->page();
|
||||
VERIFY(page.client().is_ready_to_paint());
|
||||
page.client().paint_next_frame();
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -2198,13 +2198,6 @@ void Navigable::perform_scroll_of_viewport(CSSPixelPoint new_position)
|
|||
HTML::main_thread_event_loop().schedule();
|
||||
}
|
||||
|
||||
void Navigable::set_needs_display(InvalidateDisplayList should_invalidate_display_list)
|
||||
{
|
||||
if (auto document = active_document(); document) {
|
||||
document->set_needs_display(should_invalidate_display_list);
|
||||
}
|
||||
}
|
||||
|
||||
// https://html.spec.whatwg.org/#rendering-opportunity
|
||||
bool Navigable::has_a_rendering_opportunity() const
|
||||
{
|
||||
|
|
|
@ -173,8 +173,6 @@ public:
|
|||
virtual void set_viewport_size(CSSPixelSize);
|
||||
void perform_scroll_of_viewport(CSSPixelPoint position);
|
||||
|
||||
void set_needs_display(InvalidateDisplayList = InvalidateDisplayList::Yes);
|
||||
|
||||
// https://html.spec.whatwg.org/#rendering-opportunity
|
||||
[[nodiscard]] bool has_a_rendering_opportunity() const;
|
||||
|
||||
|
|
|
@ -1429,6 +1429,8 @@ NonnullRefPtr<Gfx::PaintingSurface> TraversableNavigable::painting_surface_for_b
|
|||
|
||||
void TraversableNavigable::paint(DevicePixelRect const& content_rect, Painting::BackingStore& target, PaintOptions paint_options)
|
||||
{
|
||||
m_needs_repaint = false;
|
||||
|
||||
auto document = active_document();
|
||||
if (!document)
|
||||
return;
|
||||
|
|
|
@ -113,6 +113,9 @@ public:
|
|||
|
||||
void set_viewport_size(CSSPixelSize) override;
|
||||
|
||||
bool needs_repaint() const { return m_needs_repaint; }
|
||||
void set_needs_repaint() { m_needs_repaint = true; }
|
||||
|
||||
private:
|
||||
TraversableNavigable(GC::Ref<Page>);
|
||||
|
||||
|
@ -161,6 +164,8 @@ private:
|
|||
RefPtr<Gfx::SkiaBackendContext> m_skia_backend_context;
|
||||
OwnPtr<Painting::DisplayListPlayerSkia> m_skia_player;
|
||||
HashMap<Gfx::Bitmap*, NonnullRefPtr<Gfx::PaintingSurface>> m_bitmap_to_surface;
|
||||
|
||||
bool m_needs_repaint { true };
|
||||
};
|
||||
|
||||
struct BrowsingContextAndDocument {
|
||||
|
|
|
@ -374,7 +374,7 @@ void ConnectionFromClient::debug_request(u64 page_id, ByteString const& request,
|
|||
if (request == "set-line-box-borders") {
|
||||
bool state = argument == "on";
|
||||
page->set_should_show_line_box_borders(state);
|
||||
page->page().top_level_traversable()->set_needs_display();
|
||||
page->page().top_level_traversable()->set_needs_repaint();
|
||||
return;
|
||||
}
|
||||
|
||||
|
|
|
@ -879,6 +879,6 @@ Web::DisplayListPlayerType PageClient::display_list_player_type() const
|
|||
void PageClient::queue_screenshot_task(Optional<Web::UniqueNodeID> node_id)
|
||||
{
|
||||
m_screenshot_tasks.enqueue({ node_id });
|
||||
page().top_level_traversable()->set_needs_display();
|
||||
page().top_level_traversable()->set_needs_repaint();
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Add table
Reference in a new issue