mirrors/ladybird
0
0
Fork 0
mirror of https://github.com/LadybirdBrowser/ladybird.git synced 2025-04-22 04:25:13 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 9

LibWeb: Save ScrollState snapshot in DisplayList to avoid race condition

Browse source 
With this change we save a copy of of scroll state at the time of
recording a display list, instead of actual ScrollState pointer that
could be modifed by the main thread while display list is beings
rasterized on the rendering thread, which leads to a frame painted with
inconsistent scroll state.

Fixes https://github.com/LadybirdBrowser/ladybird/issues/4288
This commit is contained in:
Aliaksandr Kalenik 2025-04-11 20:00:56 +02:00
parent eb0a51faf0
commit 0aafd22b5f
18 changed files with 85 additions and 29 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
Libraries/LibWeb/CMakeLists.txt
View file

@ -691,6 +691,7 @@ set(SOURCES
Painting/SVGPaintable.cpp
Painting/SVGSVGPaintable.cpp
Painting/ScrollFrame.cpp
Painting/ScrollState.cpp
Painting/ShadowPainting.cpp
Painting/StackingContext.cpp
Painting/TableBordersPainting.cpp

3
Libraries/LibWeb/CSS/StyleValues/CursorStyleValue.cpp
View file

@ -94,7 +94,8 @@ Optional<Gfx::ImageCursor> CursorStyleValue::make_image_cursor(Layout::NodeWithS
case DisplayListPlayerType::SkiaCPU: {
auto painting_surface = Gfx::PaintingSurface::wrap_bitmap(bitmap);
Painting::DisplayListPlayerSkia display_list_player;
display_list_player.execute(*display_list, painting_surface);
Painting::ScrollStateSnapshot scroll_state_snapshot;
display_list_player.execute(*display_list, scroll_state_snapshot, painting_surface);
break;
}
}

4
Libraries/LibWeb/DOM/Document.cpp
View file

@ -6296,8 +6296,9 @@ void Document::invalidate_display_list()
RefPtr<Painting::DisplayList> Document::record_display_list(PaintConfig config)
{
if (m_cached_display_list && m_cached_display_list_paint_config == config)
if (m_cached_display_list && m_cached_display_list_paint_config == config) {
return m_cached_display_list;
}
auto display_list = Painting::DisplayList::create();
Painting::DisplayListRecorder display_list_recorder(display_list);
@ -6354,7 +6355,6 @@ RefPtr<Painting::DisplayList> Document::record_display_list(PaintConfig config)
viewport_paintable.paint_all_phases(context);
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_cached_display_list = display_list;
m_cached_display_list_paint_config = config;

6
Libraries/LibWeb/HTML/RenderingThread.cpp
View file

@ -57,17 +57,17 @@ void RenderingThread::rendering_thread_loop()
}
auto painting_surface = painting_surface_for_backing_store(task->backing_store);
m_skia_player->execute(*task->display_list, painting_surface);
m_skia_player->execute(*task->display_list, task->scroll_state_snapshot, painting_surface);
m_main_thread_event_loop.deferred_invoke([callback = move(task->callback)] {
callback();
});
}
}
void RenderingThread::enqueue_rendering_task(NonnullRefPtr<Painting::DisplayList> display_list, NonnullRefPtr<Painting::BackingStore> backing_store, Function<void()>&& callback)
void RenderingThread::enqueue_rendering_task(NonnullRefPtr<Painting::DisplayList> display_list, Painting::ScrollStateSnapshot&& scroll_state_snapshot, NonnullRefPtr<Painting::BackingStore> backing_store, Function<void()>&& callback)
{
Threading::MutexLocker const locker { m_rendering_task_mutex };
m_rendering_tasks.enqueue(Task { move(display_list), move(backing_store), move(callback) });
m_rendering_tasks.enqueue(Task { move(display_list), move(scroll_state_snapshot), move(backing_store), move(callback) });
m_rendering_task_ready_wake_condition.signal();
}

3
Libraries/LibWeb/HTML/RenderingThread.h
View file

@ -28,7 +28,7 @@ public:
void start(DisplayListPlayerType);
void set_skia_player(OwnPtr<Painting::DisplayListPlayerSkia>&& player) { m_skia_player = move(player); }
void set_skia_backend_context(RefPtr<Gfx::SkiaBackendContext> context) { m_skia_backend_context = move(context); }
void enqueue_rendering_task(NonnullRefPtr<Painting::DisplayList>, NonnullRefPtr<Painting::BackingStore>, Function<void()>&& callback);
void enqueue_rendering_task(NonnullRefPtr<Painting::DisplayList>, Painting::ScrollStateSnapshot&&, NonnullRefPtr<Painting::BackingStore>, Function<void()>&& callback);
void clear_bitmap_to_surface_cache();
private:
@ -46,6 +46,7 @@ private:
struct Task {
NonnullRefPtr<Painting::DisplayList> display_list;
Painting::ScrollStateSnapshot scroll_state_snapshot;
NonnullRefPtr<Painting::BackingStore> backing_store;
Function<void()> callback;
};

3
Libraries/LibWeb/HTML/TraversableNavigable.cpp
View file

@ -1426,7 +1426,8 @@ RefPtr<Painting::DisplayList> TraversableNavigable::record_display_list(DevicePi
void TraversableNavigable::start_display_list_rendering(NonnullRefPtr<Painting::DisplayList> display_list, NonnullRefPtr<Painting::BackingStore> backing_store, Function<void()>&& callback)
{
m_rendering_thread.enqueue_rendering_task(move(display_list), move(backing_store), move(callback));
auto scroll_state_snapshot = active_document()->paintable()->scroll_state().snapshot();
m_rendering_thread.enqueue_rendering_task(move(display_list), move(scroll_state_snapshot), move(backing_store), move(callback));
}
}

4
Libraries/LibWeb/Painting/Command.h
View file

@ -8,13 +8,11 @@
#include <AK/Forward.h>
#include <AK/NonnullRefPtr.h>
#include <AK/SegmentedVector.h>
#include <AK/Utf8View.h>
#include <AK/Vector.h>
#include <LibGfx/Color.h>
#include <LibGfx/CompositingAndBlendingOperator.h>
#include <LibGfx/Forward.h>
#include <LibGfx/Gradients.h>
#include <LibGfx/ImmutableBitmap.h>
#include <LibGfx/LineStyle.h>
#include <LibGfx/PaintStyle.h>
@ -34,6 +32,7 @@
#include <LibWeb/Painting/GradientData.h>
#include <LibWeb/Painting/PaintBoxShadowParams.h>
#include <LibWeb/Painting/PaintStyle.h>
#include <LibWeb/Painting/ScrollState.h>
namespace Web::Painting {
@ -392,6 +391,7 @@ struct AddMask {
struct PaintNestedDisplayList {
RefPtr<DisplayList> display_list;
ScrollStateSnapshot scroll_state_snapshot;
Gfx::IntRect rect;
[[nodiscard]] Gfx::IntRect bounding_rect() const { return rect; }

7
Libraries/LibWeb/Painting/DisplayList.cpp
View file

@ -35,18 +35,18 @@ static bool command_is_clip_or_mask(Command const& command)
});
}
void DisplayListPlayer::execute(DisplayList& display_list, RefPtr<Gfx::PaintingSurface> surface)
void DisplayListPlayer::execute(DisplayList& display_list, ScrollStateSnapshot const& scroll_state, RefPtr<Gfx::PaintingSurface> surface)
{
if (surface) {
surface->lock_context();
}
execute_impl(display_list, surface);
execute_impl(display_list, scroll_state, surface);
if (surface) {
surface->unlock_context();
}
}
void DisplayListPlayer::execute_impl(DisplayList& display_list, RefPtr<Gfx::PaintingSurface> surface)
void DisplayListPlayer::execute_impl(DisplayList& display_list, ScrollStateSnapshot const& scroll_state, RefPtr<Gfx::PaintingSurface> surface)
{
if (surface)
m_surfaces.append(*surface);
@ -56,7 +56,6 @@ void DisplayListPlayer::execute_impl(DisplayList& display_list, RefPtr<Gfx::Pain
};
auto const& commands = display_list.commands();
auto const& scroll_state = display_list.scroll_state();
auto device_pixels_per_css_pixel = display_list.device_pixels_per_css_pixel();
VERIFY(!m_surfaces.is_empty());

8
Libraries/LibWeb/Painting/DisplayList.h
View file

@ -26,11 +26,11 @@ class DisplayListPlayer {
public:
virtual ~DisplayListPlayer() = default;
void execute(DisplayList&, RefPtr<Gfx::PaintingSurface>);
void execute(DisplayList&, ScrollStateSnapshot const&, RefPtr<Gfx::PaintingSurface>);
protected:
Gfx::PaintingSurface& surface() const { return m_surfaces.last(); }
void execute_impl(DisplayList&, RefPtr<Gfx::PaintingSurface>);
void execute_impl(DisplayList&, ScrollStateSnapshot const& scroll_state, RefPtr<Gfx::PaintingSurface>);
private:
virtual void flush() = 0;
@ -93,9 +93,6 @@ public:
AK::SegmentedVector<CommandListItem, 512> const& commands() const { return m_commands; }
void set_scroll_state(ScrollState scroll_state) { m_scroll_state = move(scroll_state); }
ScrollState const& scroll_state() const { return m_scroll_state; }
void set_device_pixels_per_css_pixel(double device_pixels_per_css_pixel) { m_device_pixels_per_css_pixel = device_pixels_per_css_pixel; }
double device_pixels_per_css_pixel() const { return m_device_pixels_per_css_pixel; }
@ -103,7 +100,6 @@ private:
DisplayList() = default;
AK::SegmentedVector<CommandListItem, 512> m_commands;
ScrollState m_scroll_state;
double m_device_pixels_per_css_pixel;
};

5
Libraries/LibWeb/Painting/DisplayListPlayerSkia.cpp
View file

@ -972,7 +972,8 @@ void DisplayListPlayerSkia::add_mask(AddMask const& command)
auto mask_surface = Gfx::PaintingSurface::create_with_size(m_context, rect.size(), Gfx::BitmapFormat::BGRA8888, Gfx::AlphaType::Premultiplied);
execute_impl(*command.display_list, mask_surface);
ScrollStateSnapshot scroll_state_snapshot;
execute_impl(*command.display_list, scroll_state_snapshot, mask_surface);
SkMatrix mask_matrix;
mask_matrix.setTranslate(rect.x(), rect.y());
@ -985,7 +986,7 @@ void DisplayListPlayerSkia::paint_nested_display_list(PaintNestedDisplayList con
{
auto& canvas = surface().canvas();
canvas.translate(command.rect.x(), command.rect.y());
execute_impl(*command.display_list, {});
execute_impl(*command.display_list, command.scroll_state_snapshot, {});
}
void DisplayListPlayerSkia::paint_scrollbar(PaintScrollBar const& command)

4
Libraries/LibWeb/Painting/DisplayListRecorder.cpp
View file

@ -24,9 +24,9 @@ void DisplayListRecorder::append(Command&& command)
m_command_list.append(move(command), scroll_frame_id);
}
void DisplayListRecorder::paint_nested_display_list(RefPtr<DisplayList> display_list, Gfx::IntRect rect)
void DisplayListRecorder::paint_nested_display_list(RefPtr<DisplayList> display_list, ScrollStateSnapshot&& scroll_state_snapshot, Gfx::IntRect rect)
{
append(PaintNestedDisplayList { move(display_list), rect });
append(PaintNestedDisplayList { move(display_list), move(scroll_state_snapshot), rect });
}
void DisplayListRecorder::add_rounded_rect_clip(CornerRadii corner_radii, Gfx::IntRect border_rect, CornerClip corner_clip)

2
Libraries/LibWeb/Painting/DisplayListRecorder.h
View file

@ -129,7 +129,7 @@ public:
void push_stacking_context(PushStackingContextParams params);
void pop_stacking_context();
void paint_nested_display_list(RefPtr<DisplayList> display_list, Gfx::IntRect rect);
void paint_nested_display_list(RefPtr<DisplayList> display_list, ScrollStateSnapshot&&, Gfx::IntRect rect);
void add_rounded_rect_clip(CornerRadii corner_radii, Gfx::IntRect border_rect, CornerClip corner_clip);
void add_mask(RefPtr<DisplayList> display_list, Gfx::IntRect rect);

3
Libraries/LibWeb/Painting/NavigableContainerViewportPaintable.cpp
View file

@ -60,7 +60,8 @@ void NavigableContainerViewportPaintable::paint(PaintContext& context, PaintPhas
paint_config.should_show_line_box_borders = context.should_show_line_box_borders();
paint_config.has_focus = context.has_focus();
auto display_list = const_cast<DOM::Document*>(hosted_document)->record_display_list(paint_config);
context.display_list_recorder().paint_nested_display_list(display_list, context.enclosing_device_rect(absolute_rect).to_type<int>());
auto scroll_state_snapshot = hosted_document->paintable()->scroll_state().snapshot();
context.display_list_recorder().paint_nested_display_list(display_list, move(scroll_state_snapshot), context.enclosing_device_rect(absolute_rect).to_type<int>());
context.display_list_recorder().restore();

3
Libraries/LibWeb/Painting/SVGMaskable.cpp
View file

@ -100,7 +100,8 @@ RefPtr<Gfx::ImmutableBitmap> SVGMaskable::calculate_mask_of_svg(PaintContext& co
StackingContext::paint_svg(paint_context, paintable, PaintPhase::Foreground);
auto painting_surface = Gfx::PaintingSurface::wrap_bitmap(*mask_bitmap);
DisplayListPlayerSkia display_list_player;
display_list_player.execute(display_list, painting_surface);
ScrollStateSnapshot scroll_state_snapshot;
display_list_player.execute(display_list, scroll_state_snapshot, painting_surface);
return mask_bitmap;
};
RefPtr<Gfx::Bitmap> mask_bitmap = {};

1
Libraries/LibWeb/Painting/ScrollFrame.h
View file

@ -6,6 +6,7 @@
#pragma once
#include <AK/WeakPtr.h>
#include <LibWeb/Forward.h>
#include <LibWeb/PixelUnits.h>

20
Libraries/LibWeb/Painting/ScrollState.cpp Normal file
View file

@ -0,0 +1,20 @@
/*
* Copyright (c) 2025, Aliaksandr Kalenik <kalenik.aliaksandr@gmail.com>
*
* SPDX-License-Identifier: BSD-2-Clause
*/
#include <LibWeb/Painting/ScrollState.h>
namespace Web::Painting {
ScrollStateSnapshot ScrollStateSnapshot::create(Vector<NonnullRefPtr<ScrollFrame>> const& scroll_frames)
{
ScrollStateSnapshot snapshot;
snapshot.entries.ensure_capacity(scroll_frames.size());
for (auto const& scroll_frame : scroll_frames)
snapshot.entries.append({ scroll_frame->cumulative_offset(), scroll_frame->own_offset() });
return snapshot;
}
}

34
Libraries/LibWeb/Painting/ScrollState.h
View file

@ -1,15 +1,42 @@
/*
* Copyright (c) 2024, Aliaksandr Kalenik <kalenik.aliaksandr@gmail.com>
* Copyright (c) 2024-2025, Aliaksandr Kalenik <kalenik.aliaksandr@gmail.com>
*
* SPDX-License-Identifier: BSD-2-Clause
*/
#pragma once
#include <AK/NonnullOwnPtr.h>
#include <LibWeb/Painting/ScrollFrame.h>
namespace Web::Painting {
class ScrollStateSnapshot {
public:
static ScrollStateSnapshot create(Vector<NonnullRefPtr<ScrollFrame>> const& scroll_frames);
CSSPixelPoint cumulative_offset_for_frame_with_id(size_t id) const
{
if (id >= entries.size())
return {};
return entries[id].cumulative_offset;
}
CSSPixelPoint own_offset_for_frame_with_id(size_t id) const
{
if (id >= entries.size())
return {};
return entries[id].own_offset;
}
private:
struct Entry {
CSSPixelPoint cumulative_offset;
CSSPixelPoint own_offset;
};
Vector<Entry> entries;
};
class ScrollState {
public:
NonnullRefPtr<ScrollFrame> create_scroll_frame_for(PaintableBox const& paintable_box, RefPtr<ScrollFrame const> parent)
@ -56,6 +83,11 @@ public:
}
}
ScrollStateSnapshot snapshot() const
{
return ScrollStateSnapshot::create(m_scroll_frames);
}
private:
Vector<NonnullRefPtr<ScrollFrame>> m_scroll_frames;
};

3
Libraries/LibWeb/SVG/SVGDecodedImageData.cpp
View file

@ -106,7 +106,8 @@ RefPtr<Gfx::Bitmap> SVGDecodedImageData::render(Gfx::IntSize size) const
case DisplayListPlayerType::SkiaCPU: {
auto painting_surface = Gfx::PaintingSurface::wrap_bitmap(*bitmap);
Painting::DisplayListPlayerSkia display_list_player;
display_list_player.execute(*display_list, painting_surface);
Painting::ScrollStateSnapshot scroll_state_snapshot;
display_list_player.execute(*display_list, scroll_state_snapshot, painting_surface);
break;
}
default: