dmaax/ladybird
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Compositor: Tighten internal state invariants

Browse source 
Several compositor helpers still treated internally-owned state as
optional after their callers had already established the necessary
preconditions. That made invalid parent/child surface state, screenshot
traversal state, and scrollbar capture indices look like normal no-op
paths.
This commit is contained in:
Aliaksandr Kalenik 2026-05-30 22:08:20 +02:00 committed by Alexander Kalenik
parent 894b9098fe
commit 3fe1b97ad0
4 changed files with 12 additions and 27 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

15
Services/Compositor/CompositorState.cpp
View file

@ -342,8 +342,7 @@ void CompositorState::flush_descendant_surfaces_for_screenshot(Web::Compositor::
// list, whose embedded-content commands read those child surfaces. So, flush any descendant with a deferred
// present synchronously (deepest-first) — to capture a complete frame instead of a stale/blank iframe.
auto* context = context_if_present(context_id);
if (!context)
return;
VERIFY(context);
for (auto& child : context->child_contexts())
present_subtree_for_screenshot(child.child_context_id);
}
@ -351,8 +350,7 @@ void CompositorState::flush_descendant_surfaces_for_screenshot(Web::Compositor::
bool CompositorState::present_subtree_for_screenshot(Web::Compositor::CompositorContextId context_id)
{
auto* context = context_if_present(context_id);
if (!context)
return false;
VERIFY(context);
bool needs_present = context->needs_synchronous_present_for_screenshot();
for (auto& child : context->child_contexts()) {
@ -383,7 +381,8 @@ bool CompositorState::request_screenshot(Web::Compositor::CompositorContextId co
return false;
flush_descendant_surfaces_for_screenshot(context_id);
return context->paint_screenshot(*m_display_list_player, target_bitmap);
context->paint_screenshot(*m_display_list_player, target_bitmap);
return true;
}
void CompositorState::presented_bitmap_ready_to_paint(Web::Compositor::CompositorContextId context_id, i32 bitmap_id)
@ -445,8 +444,9 @@ void CompositorState::cancel_pending_async_presents_for_context(Web::Compositor:
void CompositorState::schedule_gpu_completion_check()
{
if (!m_skia_backend_context || m_pending_async_presents.is_empty())
if (!m_skia_backend_context)
return;
VERIFY(!m_pending_async_presents.is_empty());
if (!m_gpu_completion_timer) {
m_gpu_completion_timer = Core::Timer::create_repeating(gpu_completion_check_interval_ms, [this] {
@ -572,8 +572,7 @@ bool CompositorState::apply_context_update_result(
void CompositorState::publish_backing_stores(Web::Compositor::CompositorContextId context_id, ContextState& context, BackingStoreManager::Publication&& publication)
{
VERIFY(m_client);
if (!context.presents_to_client())
return;
VERIFY(context.presents_to_client());
m_client->did_allocate_backing_stores(context_id, publication.front_bitmap_id, move(publication.front_shared_image), publication.back_bitmap_id, move(publication.back_shared_image));
}

9
Services/Compositor/ContextState.cpp
View file

@ -513,8 +513,7 @@ void ContextState::did_submit_prepared_frame(Gfx::IntRect viewport_rect)
Optional<Web::Compositor::PublishToCompositorSurface> ContextState::present_synchronously(Web::Painting::DisplayListPlayerSkia& display_list_player)
{
auto* publish_mode = m_presentation_mode.get_pointer<Web::Compositor::PublishToCompositorSurface>();
if (!publish_mode)
return {};
VERIFY(publish_mode);
if (!can_render_frame())
return {};
// Don't race an async present already in flight for this context; its own completion will publish.
@ -546,15 +545,13 @@ bool ContextState::can_paint_screenshot(Gfx::ShareableBitmap& target_bitmap) con
return m_display_list && target_bitmap.is_valid() && target_bitmap.bitmap();
}
bool ContextState::paint_screenshot(Web::Painting::DisplayListPlayerSkia& display_list_player, Gfx::ShareableBitmap& target_bitmap)
void ContextState::paint_screenshot(Web::Painting::DisplayListPlayerSkia& display_list_player, Gfx::ShareableBitmap& target_bitmap)
{
if (!can_paint_screenshot(target_bitmap))
return false;
VERIFY(can_paint_screenshot(target_bitmap));
auto target_surface = Gfx::PaintingSurface::wrap_bitmap(*target_bitmap.bitmap());
paint_current_display_list(display_list_player, *target_surface);
display_list_player.flush(*target_surface);
return true;
}
bool ContextState::acknowledge_presented_bitmap(i32 bitmap_id)

2
Services/Compositor/ContextState.h
View file

@ -153,7 +153,7 @@ public:
void did_submit_prepared_frame(Gfx::IntRect);
Optional<Web::Compositor::PublishToCompositorSurface> present_synchronously(Web::Painting::DisplayListPlayerSkia&);
bool can_paint_screenshot(Gfx::ShareableBitmap&) const;
bool paint_screenshot(Web::Painting::DisplayListPlayerSkia&, Gfx::ShareableBitmap&);
void paint_screenshot(Web::Painting::DisplayListPlayerSkia&, Gfx::ShareableBitmap&);
bool acknowledge_presented_bitmap(i32 bitmap_id);
void did_finish_gpu_present(i32 bitmap_id);

13
Services/Compositor/ViewportScrollbarController.cpp
View file

@ -29,7 +29,7 @@ static ViewportScrollbarIdentity viewport_scrollbar_identity(Web::Compositor::Vi
static Optional<ViewportScrollbarIdentity> viewport_scrollbar_identity_at(ReadonlySpan<Web::Compositor::ViewportScrollbar> scrollbars, Optional<size_t> scrollbar_index)
{
if (!scrollbar_index.has_value() || *scrollbar_index >= scrollbars.size())
if (!scrollbar_index.has_value())
return {};
return viewport_scrollbar_identity(scrollbars[*scrollbar_index]);
}
@ -159,10 +159,6 @@ Optional<ViewportScrollbarController::Drag> ViewportScrollbarController::capture
if (!m_captured_scrollbar_index.has_value())
return {};
auto scrollbar_index = *m_captured_scrollbar_index;
if (scrollbar_index >= m_scrollbars.size()) {
m_captured_scrollbar_index.clear();
return {};
}
auto const& scrollbar = m_scrollbars[scrollbar_index];
auto primary_position = position.primary_offset_for_orientation(orientation_for_scrollbar(scrollbar));
return Drag { scrollbar_index, primary_position, m_thumb_grab_position };
@ -174,10 +170,6 @@ Optional<ViewportScrollbarController::Drag> ViewportScrollbarController::release
return {};
auto scrollbar_index = *m_captured_scrollbar_index;
auto thumb_grab_position = m_thumb_grab_position;
if (scrollbar_index >= m_scrollbars.size()) {
m_captured_scrollbar_index.clear();
return {};
}
auto const& scrollbar = m_scrollbars[scrollbar_index];
auto primary_position = position.primary_offset_for_orientation(orientation_for_scrollbar(scrollbar));
m_captured_scrollbar_index.clear();
@ -196,9 +188,6 @@ bool ViewportScrollbarController::set_hovered_scrollbar(Optional<size_t> scrollb
Optional<ViewportScrollbarController::ScrollDelta> ViewportScrollbarController::scroll_delta_for_drag(Web::Compositor::AsyncScrollTree const& async_scroll_tree, Web::Painting::ScrollStateSnapshot const& scroll_state_snapshot, Drag const& drag) const
{
if (drag.scrollbar_index >= m_scrollbars.size())
return {};
auto const& scrollbar = m_scrollbars[drag.scrollbar_index];
auto expanded = is_expanded(drag.scrollbar_index);
auto scroll_size = scrollbar_scroll_size(scrollbar, expanded);