From 91bcd502243f83dbde0a1099cf88c3a2241a1426 Mon Sep 17 00:00:00 2001 From: Aliaksandr Kalenik Date: Wed, 17 Jun 2026 02:38:33 +0200 Subject: [PATCH] LibGfx+LibWeb: Store Skia images with display list resources DisplayListPlayerSkia kept a separate DecodedImageFrameSkiaImageCache that was pruned during flushes. That made Skia image lifetime independent of display list resource lifetime, even though resource storage is what knows when image frames and compositor surfaces are no longer needed. Make DisplayListResourceStorage own an opaque stored image-frame resource that holds the decoded frame and its lazily-created SkImage. Removing image frames or compositor surfaces now drops the decoded frame and Skia image together, while transactions still carry only Skia-free decoded frames. --- Libraries/LibGfx/CMakeLists.txt | 1 - .../DecodedImageFrameSkiaImageCache.cpp | 145 ------------------ .../LibGfx/DecodedImageFrameSkiaImageCache.h | 34 ---- .../LibWeb/Painting/DisplayListPlayerSkia.cpp | 14 +- .../LibWeb/Painting/DisplayListPlayerSkia.h | 4 +- .../Painting/DisplayListResourceStorage.cpp | 78 +++++++++- .../Painting/DisplayListResourceStorage.h | 23 ++- 7 files changed, 96 insertions(+), 203 deletions(-) delete mode 100644 Libraries/LibGfx/DecodedImageFrameSkiaImageCache.cpp delete mode 100644 Libraries/LibGfx/DecodedImageFrameSkiaImageCache.h diff --git a/Libraries/LibGfx/CMakeLists.txt b/Libraries/LibGfx/CMakeLists.txt index 852a590cdc..12b917e178 100644 --- a/Libraries/LibGfx/CMakeLists.txt +++ b/Libraries/LibGfx/CMakeLists.txt @@ -32,7 +32,6 @@ set(SOURCES ImageFormats/WebPSharedLossless.cpp ImageFormats/WebPWriter.cpp ImageFormats/WebPWriterLossless.cpp - DecodedImageFrameSkiaImageCache.cpp PaintStyle.cpp Painter.cpp PainterSkia.cpp diff --git a/Libraries/LibGfx/DecodedImageFrameSkiaImageCache.cpp b/Libraries/LibGfx/DecodedImageFrameSkiaImageCache.cpp deleted file mode 100644 index 68a78e6613..0000000000 --- a/Libraries/LibGfx/DecodedImageFrameSkiaImageCache.cpp +++ /dev/null @@ -1,145 +0,0 @@ -/* - * Copyright (c) 2026, Aliaksandr Kalenik - * - * SPDX-License-Identifier: BSD-2-Clause - */ - -#include -#include -#include -#include -#include -#include - -#include -#include -#include -#include -#include - -namespace Gfx { - -static constexpr size_t image_cache_max_entries = 128; -static constexpr size_t image_cache_max_bytes = 64 * MiB; - -struct DecodedImageFrameSkiaImageCache::Impl { - Impl() = default; - - explicit Impl(RefPtr skia_backend_context) - : skia_backend_context(move(skia_backend_context)) - { - } - - struct DecodedImageFrameKeyTraits : public Traits { - static unsigned hash(DecodedImageFrame const& frame) - { - return pair_int_hash( - ptr_hash(&frame.bitmap()), - ptr_hash(color_space_pointer(frame))); - } - - static bool equals(DecodedImageFrame const& a, DecodedImageFrame const& b) - { - return &a.bitmap() == &b.bitmap() - && color_space_pointer(a) == color_space_pointer(b); - } - - static constexpr bool may_have_slow_equality_check() { return false; } - - private: - static SkColorSpace const* color_space_pointer(DecodedImageFrame const& frame) - { - return frame.color_space().color_space>().get(); - } - }; - - struct CachedImage { - sk_sp image; - u64 last_used_sequence_number { 0 }; - size_t approximate_byte_size { 0 }; - }; - - u64 next_use_sequence_number() - { - return ++use_sequence_number; - } - - void prune_to_limits() - { - while (images.size() > image_cache_max_entries || approximate_byte_size > image_cache_max_bytes) { - Optional least_recently_used_frame; - Optional least_recently_used_sequence_number; - for (auto const& image : images) { - if (!least_recently_used_sequence_number.has_value() - || image.value.last_used_sequence_number < least_recently_used_sequence_number.value()) { - least_recently_used_frame = image.key; - least_recently_used_sequence_number = image.value.last_used_sequence_number; - } - } - - if (!least_recently_used_frame.has_value()) - break; - - auto cached_image = images.get(least_recently_used_frame.value()).release_value(); - approximate_byte_size -= min(approximate_byte_size, cached_image.approximate_byte_size); - images.remove(least_recently_used_frame.value()); - } - } - - RefPtr skia_backend_context; - HashMap images; - size_t approximate_byte_size { 0 }; - u64 use_sequence_number { 0 }; -}; - -DecodedImageFrameSkiaImageCache::DecodedImageFrameSkiaImageCache() - : m_impl(make()) -{ -} - -DecodedImageFrameSkiaImageCache::DecodedImageFrameSkiaImageCache(RefPtr skia_backend_context) - : m_impl(make(move(skia_backend_context))) -{ -} - -DecodedImageFrameSkiaImageCache::~DecodedImageFrameSkiaImageCache() = default; - -sk_sp DecodedImageFrameSkiaImageCache::image_for_frame(DecodedImageFrame const& frame) -{ - auto const& bitmap = frame.bitmap(); - if (auto it = m_impl->images.find(frame); it != m_impl->images.end()) { - it->value.last_used_sequence_number = m_impl->next_use_sequence_number(); - return it->value.image; - } - - auto raster_image = sk_image_from_bitmap(bitmap, frame.color_space()); - sk_sp image; - auto* gr_context = m_impl->skia_backend_context ? m_impl->skia_backend_context->sk_context() : nullptr; - if (gr_context) { - image = SkImages::TextureFromImage(gr_context, raster_image.get(), skgpu::Mipmapped::kNo, skgpu::Budgeted::kYes); - if (!image) - image = move(raster_image); - } else { - image = move(raster_image); - } - - if (!image) - return nullptr; - - Impl::CachedImage cached_image { - .image = image, - .last_used_sequence_number = m_impl->next_use_sequence_number(), - .approximate_byte_size = bitmap.size_in_bytes(), - }; - m_impl->approximate_byte_size += cached_image.approximate_byte_size; - m_impl->images.set(frame, move(cached_image)); - m_impl->prune_to_limits(); - return image; -} - -void DecodedImageFrameSkiaImageCache::prune() -{ - m_impl->prune_to_limits(); -} - -} diff --git a/Libraries/LibGfx/DecodedImageFrameSkiaImageCache.h b/Libraries/LibGfx/DecodedImageFrameSkiaImageCache.h deleted file mode 100644 index 9b76443258..0000000000 --- a/Libraries/LibGfx/DecodedImageFrameSkiaImageCache.h +++ /dev/null @@ -1,34 +0,0 @@ -/* - * Copyright (c) 2026, Aliaksandr Kalenik - * - * SPDX-License-Identifier: BSD-2-Clause - */ - -#pragma once - -#include -#include -#include - -class SkImage; - -template -class sk_sp; - -namespace Gfx { - -class DecodedImageFrameSkiaImageCache final { -public: - DecodedImageFrameSkiaImageCache(); - explicit DecodedImageFrameSkiaImageCache(RefPtr); - ~DecodedImageFrameSkiaImageCache(); - - sk_sp image_for_frame(DecodedImageFrame const&); - void prune(); - -private: - struct Impl; - OwnPtr m_impl; -}; - -} diff --git a/Libraries/LibWeb/Painting/DisplayListPlayerSkia.cpp b/Libraries/LibWeb/Painting/DisplayListPlayerSkia.cpp index b14316a6d3..99852730df 100644 --- a/Libraries/LibWeb/Painting/DisplayListPlayerSkia.cpp +++ b/Libraries/LibWeb/Painting/DisplayListPlayerSkia.cpp @@ -50,7 +50,6 @@ DisplayListPlayerSkia::DisplayListPlayerSkia() DisplayListPlayerSkia::DisplayListPlayerSkia(RefPtr skia_backend_context) : m_skia_backend_context(move(skia_backend_context)) - , m_image_cache(m_skia_backend_context) { } @@ -107,7 +106,6 @@ static SkM44 to_skia_matrix4x4(Gfx::FloatMatrix4x4 const& matrix) void DisplayListPlayerSkia::flush(Gfx::PaintingSurface& surface) { - m_image_cache.prune(); if (auto context = surface.skia_backend_context()) context->flush_and_submit(&surface.sk_surface()); surface.flush(); @@ -115,7 +113,6 @@ void DisplayListPlayerSkia::flush(Gfx::PaintingSurface& surface) void DisplayListPlayerSkia::flush_async(Gfx::PaintingSurface& surface, Function&& callback) { - m_image_cache.prune(); if (auto context = surface.skia_backend_context()) context->flush_and_submit_async(&surface.sk_surface(), move(callback)); else @@ -210,11 +207,7 @@ void DisplayListPlayerSkia::play_command(FillRect const& command) void DisplayListPlayerSkia::play_command(DrawCompositorSurface const& command) { - auto frame = resource_storage().compositor_surface(command.surface_id); - if (!frame.has_value()) - return; - - auto image = m_image_cache.image_for_frame(frame.value()); + auto image = resource_storage().skia_image_for_compositor_surface(command.surface_id, m_skia_backend_context); if (!image) return; @@ -293,8 +286,7 @@ void DisplayListPlayerSkia::play_command(DrawVideoFrame const& command) void DisplayListPlayerSkia::play_command(DrawScaledDecodedImageFrame const& command) { - auto const& frame = resource_storage().image_frame(command.frame_id); - auto image = m_image_cache.image_for_frame(frame); + auto image = resource_storage().skia_image_for_image_frame(command.frame_id, m_skia_backend_context); if (!image) return; @@ -311,7 +303,7 @@ void DisplayListPlayerSkia::play_command(DrawScaledDecodedImageFrame const& comm void DisplayListPlayerSkia::play_command(DrawRepeatedDecodedImageFrame const& command) { auto const& frame = resource_storage().image_frame(command.frame_id); - auto image = m_image_cache.image_for_frame(frame); + auto image = resource_storage().skia_image_for_image_frame(command.frame_id, m_skia_backend_context); if (!image) return; diff --git a/Libraries/LibWeb/Painting/DisplayListPlayerSkia.h b/Libraries/LibWeb/Painting/DisplayListPlayerSkia.h index 408ef1e4c8..40a6cad545 100644 --- a/Libraries/LibWeb/Painting/DisplayListPlayerSkia.h +++ b/Libraries/LibWeb/Painting/DisplayListPlayerSkia.h @@ -7,7 +7,8 @@ #pragma once #include -#include +#include +#include #include #include #include @@ -45,7 +46,6 @@ private: ReadonlySpan gradient_positions(DisplayListGradientColorStops) const; RefPtr m_skia_backend_context; - Gfx::DecodedImageFrameSkiaImageCache m_image_cache; }; } diff --git a/Libraries/LibWeb/Painting/DisplayListResourceStorage.cpp b/Libraries/LibWeb/Painting/DisplayListResourceStorage.cpp index ae0f5c4e23..049cb064c3 100644 --- a/Libraries/LibWeb/Painting/DisplayListResourceStorage.cpp +++ b/Libraries/LibWeb/Painting/DisplayListResourceStorage.cpp @@ -7,12 +7,52 @@ #include #include #include +#include +#include #include #include #include +#include +#include +#include + namespace Web::Painting { +struct DisplayListStoredImageFrameResource { + explicit DisplayListStoredImageFrameResource(Gfx::DecodedImageFrame frame) + : frame(move(frame)) + { + } + + Gfx::DecodedImageFrame frame; + mutable sk_sp skia_image; + mutable RefPtr skia_backend_context; +}; + +static sk_sp create_skia_image(Gfx::DecodedImageFrame const& frame, RefPtr const& skia_backend_context) +{ + auto raster_image = Gfx::sk_image_from_bitmap(frame.bitmap(), frame.color_space()); + auto* gr_context = skia_backend_context ? skia_backend_context->sk_context() : nullptr; + if (!gr_context) + return raster_image; + + auto texture_image = SkImages::TextureFromImage(gr_context, raster_image.get(), skgpu::Mipmapped::kNo, skgpu::Budgeted::kYes); + if (texture_image) + return texture_image; + return raster_image; +} + +static sk_sp skia_image_for_stored_image_frame(DisplayListStoredImageFrameResource const& resource, RefPtr const& skia_backend_context) +{ + if (resource.skia_image && resource.skia_backend_context.ptr() == skia_backend_context.ptr()) + return resource.skia_image; + + resource.skia_image = create_skia_image(resource.frame, skia_backend_context); + resource.skia_backend_context = skia_backend_context; + return resource.skia_image; +} + bool DisplayListResourceSet::is_empty() const { return fonts.is_empty() @@ -51,6 +91,9 @@ DisplayListResource::DisplayListResource(DisplayList const& display_list, Accumu { } +DisplayListResourceStorage::DisplayListResourceStorage() = default; +DisplayListResourceStorage::DisplayListResourceStorage(DisplayListResourceStorage&&) = default; +DisplayListResourceStorage& DisplayListResourceStorage::operator=(DisplayListResourceStorage&&) = default; DisplayListResourceStorage::~DisplayListResourceStorage() = default; FontResourceId DisplayListResourceStorage::add_font(Gfx::Font const& font) @@ -63,7 +106,7 @@ FontResourceId DisplayListResourceStorage::add_font(Gfx::Font const& font) ImageFrameResourceId DisplayListResourceStorage::add_image_frame(Gfx::DecodedImageFrame const& frame) { auto id = frame.id(); - m_image_frames.ensure(id, [&] { return frame; }); + m_image_frames.ensure(id, [&] { return make(frame); }); return { id }; } @@ -96,7 +139,17 @@ void DisplayListResourceStorage::set_font(FontResourceId id, NonnullRefPtr(move(frame))); +} + +Gfx::DecodedImageFrame const& DisplayListResourceStorage::image_frame(ImageFrameResourceId id) const +{ + return m_image_frames.get(id.value()).value()->frame; +} + +sk_sp DisplayListResourceStorage::skia_image_for_image_frame(ImageFrameResourceId id, RefPtr const& skia_backend_context) const +{ + return skia_image_for_stored_image_frame(*m_image_frames.get(id.value()).value(), skia_backend_context); } static ReadonlyBytes inline_data(ReadonlyBytes payload, DisplayListDataSpan span) @@ -286,7 +339,8 @@ void DisplayListResourceStorage::retain_only(DisplayListResourceSet const& resou && !m_font_cache_reference_counts.contains(id); }); m_image_frames.remove_all_matching([&](auto id, auto const&) { - return !resource_set.image_frames.contains(ImageFrameResourceId { id }) + auto image_frame_id = ImageFrameResourceId { id }; + return !resource_set.image_frames.contains(image_frame_id) && !m_image_frame_cache_reference_counts.contains(id); }); m_video_frames.remove_all_matching([&](auto id, auto const&) { @@ -313,7 +367,7 @@ void DisplayListResourceStorage::clear_video_frame(VideoFrameResourceId frame_id void DisplayListResourceStorage::update_compositor_surface(CompositorSurfaceId surface_id, Gfx::SharedImage&& shared_image) { auto shared_image_buffer = Gfx::SharedImageBuffer::import_from_shared_image(move(shared_image)); - m_compositor_surfaces.set(surface_id.value(), Gfx::DecodedImageFrame { *shared_image_buffer.bitmap() }); + m_compositor_surfaces.set(surface_id.value(), make(Gfx::DecodedImageFrame { *shared_image_buffer.bitmap() })); } void DisplayListResourceStorage::clear_compositor_surface(CompositorSurfaceId surface_id) @@ -321,4 +375,20 @@ void DisplayListResourceStorage::clear_compositor_surface(CompositorSurfaceId su m_compositor_surfaces.remove(surface_id.value()); } +Optional DisplayListResourceStorage::compositor_surface(CompositorSurfaceId id) const +{ + auto frame = m_compositor_surfaces.get(id.value()); + if (!frame.has_value()) + return {}; + return frame.value()->frame; +} + +sk_sp DisplayListResourceStorage::skia_image_for_compositor_surface(CompositorSurfaceId id, RefPtr const& skia_backend_context) const +{ + auto resource = m_compositor_surfaces.get(id.value()); + if (!resource.has_value()) + return nullptr; + return skia_image_for_stored_image_frame(*resource.value(), skia_backend_context); +} + } diff --git a/Libraries/LibWeb/Painting/DisplayListResourceStorage.h b/Libraries/LibWeb/Painting/DisplayListResourceStorage.h index ab9e7e5064..cca4879698 100644 --- a/Libraries/LibWeb/Painting/DisplayListResourceStorage.h +++ b/Libraries/LibWeb/Painting/DisplayListResourceStorage.h @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -24,6 +25,11 @@ #include #include +class SkImage; + +template +class sk_sp; + namespace Web::Painting { struct DisplayListResourceSet { @@ -51,6 +57,8 @@ struct DisplayListVideoFrameResource { RefPtr frame; }; +struct DisplayListStoredImageFrameResource; + struct DisplayListResource { DisplayListResource(NonnullRefPtr, AccumulatedVisualContextTree); DisplayListResource(NonnullRefPtr, AccumulatedVisualContextTree); @@ -74,10 +82,11 @@ struct DisplayListResourceTransaction { class WEB_API DisplayListResourceStorage { AK_MAKE_NONCOPYABLE(DisplayListResourceStorage); - AK_MAKE_DEFAULT_MOVABLE(DisplayListResourceStorage); public: - DisplayListResourceStorage() = default; + DisplayListResourceStorage(); + DisplayListResourceStorage(DisplayListResourceStorage&&); + DisplayListResourceStorage& operator=(DisplayListResourceStorage&&); ~DisplayListResourceStorage(); FontResourceId add_font(Gfx::Font const&); @@ -101,21 +110,23 @@ public: void clear_compositor_surface(CompositorSurfaceId); Gfx::Font const& font(FontResourceId id) const { return *m_fonts.get(id.value()).value(); } - Gfx::DecodedImageFrame const& image_frame(ImageFrameResourceId id) const { return m_image_frames.get(id.value()).value(); } + Gfx::DecodedImageFrame const& image_frame(ImageFrameResourceId) const; + sk_sp skia_image_for_image_frame(ImageFrameResourceId, RefPtr const&) const; RefPtr video_frame(VideoFrameResourceId id) const { return m_video_frames.get(id.value()).value(); } DisplayListResource const& display_list_resource(DisplayListResourceId id) const { return m_display_lists.get(id.value()).value(); } DisplayList const& display_list(DisplayListResourceId id) const { return *display_list_resource(id).display_list; } AccumulatedVisualContextTree const& display_list_visual_context_tree(DisplayListResourceId id) const { return display_list_resource(id).visual_context_tree; } - Optional compositor_surface(CompositorSurfaceId id) const { return m_compositor_surfaces.get(id.value()); } + Optional compositor_surface(CompositorSurfaceId) const; + sk_sp skia_image_for_compositor_surface(CompositorSurfaceId, RefPtr const&) const; private: void collect_referenced_resources(ReadonlyBytes command_bytes, DisplayListResourceSet&) const; HashMap> m_fonts; - HashMap m_image_frames; + HashMap> m_image_frames; HashMap> m_video_frames; HashMap m_display_lists; - HashMap m_compositor_surfaces; + HashMap> m_compositor_surfaces; HashMap m_font_cache_reference_counts; HashMap m_image_frame_cache_reference_counts;