From 1c012f0a4a4b8044be7b9febcd819a2471ee1816 Mon Sep 17 00:00:00 2001 From: MacDue Date: Sat, 21 Oct 2023 19:17:49 +0100 Subject: [PATCH] LibWeb: Remove "cached corner bitmap" and its use in the corner clipper With the recording painter the actual painting operations are delayed, so now if multiple corner clippers are constructed, and they use a shared bitmap they can interfere with each other. The use of this shared bitmap was somewhat questionable anyway, so this is not much of a loss. This fixes the border-radius.html test page. --- .../LibWeb/Painting/BorderPainting.cpp | 25 ------------------- .../LibWeb/Painting/BorderPainting.h | 2 -- .../Painting/BorderRadiusCornerClipper.cpp | 19 ++++---------- .../Painting/BorderRadiusCornerClipper.h | 10 ++------ .../LibWeb/Painting/PaintableBox.cpp | 2 +- 5 files changed, 8 insertions(+), 50 deletions(-) diff --git a/Userland/Libraries/LibWeb/Painting/BorderPainting.cpp b/Userland/Libraries/LibWeb/Painting/BorderPainting.cpp index 92776fc304..cd71d85fab 100644 --- a/Userland/Libraries/LibWeb/Painting/BorderPainting.cpp +++ b/Userland/Libraries/LibWeb/Painting/BorderPainting.cpp @@ -545,31 +545,6 @@ void paint_border(PaintContext& context, BorderEdge edge, DevicePixelRect const& } } -RefPtr get_cached_corner_bitmap(DevicePixelSize corners_size) -{ - auto allocate_mask_bitmap = [&]() -> RefPtr { - auto bitmap = Gfx::Bitmap::create(Gfx::BitmapFormat::BGRA8888, corners_size.to_type()); - if (!bitmap.is_error()) - return bitmap.release_value(); - return nullptr; - }; - // FIXME: Allocate per page? - static thread_local auto corner_bitmap = allocate_mask_bitmap(); - // Only reallocate the corner bitmap is the existing one is too small. - // (should mean no more allocations after the first paint -- amortised zero allocations :^)) - if (corner_bitmap && corner_bitmap->rect().size().contains(corners_size.to_type())) { - Gfx::Painter painter { *corner_bitmap }; - painter.clear_rect({ { 0, 0 }, corners_size }, Gfx::Color()); - } else { - corner_bitmap = allocate_mask_bitmap(); - if (!corner_bitmap) { - dbgln("Failed to allocate corner bitmap with size {}", corners_size); - return nullptr; - } - } - return corner_bitmap; -} - void paint_all_borders(PaintContext& context, DevicePixelRect const& border_rect, BorderRadiiData const& border_radii_data, BordersData const& borders_data) { if (borders_data.top.width <= 0 && borders_data.right.width <= 0 && borders_data.left.width <= 0 && borders_data.bottom.width <= 0) diff --git a/Userland/Libraries/LibWeb/Painting/BorderPainting.h b/Userland/Libraries/LibWeb/Painting/BorderPainting.h index ec263af3d0..494a658430 100644 --- a/Userland/Libraries/LibWeb/Painting/BorderPainting.h +++ b/Userland/Libraries/LibWeb/Painting/BorderPainting.h @@ -28,8 +28,6 @@ enum class BorderEdge { // Returns OptionalNone if there is no outline to paint. Optional borders_data_for_outline(Layout::Node const&, Color outline_color, CSS::OutlineStyle outline_style, CSSPixels outline_width); -RefPtr get_cached_corner_bitmap(DevicePixelSize corners_size); - void paint_border(PaintContext& context, BorderEdge edge, DevicePixelRect const& rect, Gfx::AntiAliasingPainter::CornerRadius const& radius, Gfx::AntiAliasingPainter::CornerRadius const& opposite_radius, BordersData const& borders_data, Gfx::Path& path, bool last); void paint_all_borders(PaintContext& context, DevicePixelRect const& border_rect, BorderRadiiData const& border_radii_data, BordersData const&); diff --git a/Userland/Libraries/LibWeb/Painting/BorderRadiusCornerClipper.cpp b/Userland/Libraries/LibWeb/Painting/BorderRadiusCornerClipper.cpp index 9f7a791000..88fc088654 100644 --- a/Userland/Libraries/LibWeb/Painting/BorderRadiusCornerClipper.cpp +++ b/Userland/Libraries/LibWeb/Painting/BorderRadiusCornerClipper.cpp @@ -11,7 +11,7 @@ namespace Web::Painting { -ErrorOr> BorderRadiusCornerClipper::create(CornerRadii const& corner_radii, DevicePixelRect const& border_rect, BorderRadiiData const& border_radii, CornerClip corner_clip, UseCachedBitmap use_cached_bitmap) +ErrorOr> BorderRadiusCornerClipper::create(CornerRadii const& corner_radii, DevicePixelRect const& border_rect, BorderRadiiData const& border_radii, CornerClip corner_clip) { VERIFY(border_radii.has_any_radius()); @@ -37,20 +37,12 @@ ErrorOr> BorderRadiusCornerClipper::cre top_right.vertical_radius + bottom_right.vertical_radius)) }; - RefPtr corner_bitmap; - if (use_cached_bitmap == UseCachedBitmap::Yes) { - corner_bitmap = get_cached_corner_bitmap(corners_bitmap_size); - if (!corner_bitmap) - return Error::from_errno(ENOMEM); - } else { - corner_bitmap = TRY(Gfx::Bitmap::create(Gfx::BitmapFormat::BGRA8888, corners_bitmap_size.to_type())); - } + RefPtr corner_bitmap = TRY(Gfx::Bitmap::create(Gfx::BitmapFormat::BGRA8888, corners_bitmap_size.to_type())); CornerData corner_data { .corner_radii = corner_radii, .page_locations = { .top_left = border_rect.top_left(), .top_right = border_rect.top_right().translated(-top_right.horizontal_radius, 0), .bottom_right = border_rect.bottom_right().translated(-bottom_right.horizontal_radius, -bottom_right.vertical_radius), .bottom_left = border_rect.bottom_left().translated(0, -bottom_left.vertical_radius) }, .bitmap_locations = { .top_left = { 0, 0 }, .top_right = { corners_bitmap_size.width() - top_right.horizontal_radius, 0 }, .bottom_right = { corners_bitmap_size.width() - bottom_right.horizontal_radius, corners_bitmap_size.height() - bottom_right.vertical_radius }, .bottom_left = { 0, corners_bitmap_size.height() - bottom_left.vertical_radius } }, - .corner_bitmap_size = corners_bitmap_size }; return try_make_ref_counted(corner_data, corner_bitmap.release_nonnull(), corner_clip, border_rect); @@ -61,8 +53,7 @@ void BorderRadiusCornerClipper::sample_under_corners(Gfx::Painter& page_painter) // Generate a mask for the corners: Gfx::Painter corner_painter { *m_corner_bitmap }; Gfx::AntiAliasingPainter corner_aa_painter { corner_painter }; - Gfx::IntRect corner_rect { { 0, 0 }, m_data.corner_bitmap_size }; - corner_aa_painter.fill_rect_with_rounded_corners(corner_rect, Color::NamedColor::Black, + corner_aa_painter.fill_rect_with_rounded_corners(m_corner_bitmap->rect(), Color::NamedColor::Black, m_data.corner_radii.top_left, m_data.corner_radii.top_right, m_data.corner_radii.bottom_right, m_data.corner_radii.bottom_left); auto copy_page_masked = [&](Gfx::IntRect const& mask_src, Gfx::IntPoint const& page_location) { @@ -112,7 +103,7 @@ void BorderRadiusCornerClipper::blit_corner_clipping(Gfx::Painter& painter) painter.blit(m_data.page_locations.bottom_left.to_type(), *m_corner_bitmap, m_data.corner_radii.bottom_left.as_rect().translated(m_data.bitmap_locations.bottom_left.to_type())); } -ScopedCornerRadiusClip::ScopedCornerRadiusClip(PaintContext& context, DevicePixelRect const& border_rect, BorderRadiiData const& border_radii, CornerClip corner_clip, BorderRadiusCornerClipper::UseCachedBitmap use_cached_bitmap) +ScopedCornerRadiusClip::ScopedCornerRadiusClip(PaintContext& context, DevicePixelRect const& border_rect, BorderRadiiData const& border_radii, CornerClip corner_clip) : m_context(context) { if (border_radii.has_any_radius()) { @@ -122,7 +113,7 @@ ScopedCornerRadiusClip::ScopedCornerRadiusClip(PaintContext& context, DevicePixe .bottom_right = border_radii.bottom_right.as_corner(context), .bottom_left = border_radii.bottom_left.as_corner(context) }; - auto clipper = BorderRadiusCornerClipper::create(corner_radii, border_rect, border_radii, corner_clip, use_cached_bitmap); + auto clipper = BorderRadiusCornerClipper::create(corner_radii, border_rect, border_radii, corner_clip); if (!clipper.is_error()) { m_corner_clipper = clipper.release_value(); m_context.painter().sample_under_corners(*m_corner_clipper); diff --git a/Userland/Libraries/LibWeb/Painting/BorderRadiusCornerClipper.h b/Userland/Libraries/LibWeb/Painting/BorderRadiusCornerClipper.h index 50aacbd812..01233bbde7 100644 --- a/Userland/Libraries/LibWeb/Painting/BorderRadiusCornerClipper.h +++ b/Userland/Libraries/LibWeb/Painting/BorderRadiusCornerClipper.h @@ -18,12 +18,7 @@ enum class CornerClip { class BorderRadiusCornerClipper : public RefCounted { public: - enum class UseCachedBitmap { - Yes, - No - }; - - static ErrorOr> create(CornerRadii const&, DevicePixelRect const& border_rect, BorderRadiiData const& border_radii, CornerClip corner_clip = CornerClip::Outside, UseCachedBitmap use_cached_bitmap = UseCachedBitmap::Yes); + static ErrorOr> create(CornerRadii const&, DevicePixelRect const& border_rect, BorderRadiiData const& border_radii, CornerClip corner_clip = CornerClip::Outside); void sample_under_corners(Gfx::Painter& page_painter); void blit_corner_clipping(Gfx::Painter& page_painter); @@ -38,7 +33,6 @@ public: }; CornerLocations page_locations; CornerLocations bitmap_locations; - DevicePixelSize corner_bitmap_size; } m_data; DevicePixelRect border_rect() const { return m_border_rect; } @@ -59,7 +53,7 @@ private: }; struct ScopedCornerRadiusClip { - ScopedCornerRadiusClip(PaintContext& context, DevicePixelRect const& border_rect, BorderRadiiData const& border_radii, CornerClip corner_clip = CornerClip::Outside, BorderRadiusCornerClipper::UseCachedBitmap use_cached_bitmap = BorderRadiusCornerClipper::UseCachedBitmap::Yes); + ScopedCornerRadiusClip(PaintContext& context, DevicePixelRect const& border_rect, BorderRadiiData const& border_radii, CornerClip corner_clip = CornerClip::Outside); ~ScopedCornerRadiusClip(); diff --git a/Userland/Libraries/LibWeb/Painting/PaintableBox.cpp b/Userland/Libraries/LibWeb/Painting/PaintableBox.cpp index 535080ded5..01c3c98994 100644 --- a/Userland/Libraries/LibWeb/Painting/PaintableBox.cpp +++ b/Userland/Libraries/LibWeb/Painting/PaintableBox.cpp @@ -484,7 +484,7 @@ void PaintableBox::apply_clip_overflow_rect(PaintContext& context, PaintPhase ph .bottom_left = border_radii_data.bottom_left.as_corner(context) }; if (border_radii_data.has_any_radius()) { - auto corner_clipper = BorderRadiusCornerClipper::create(corner_radii, context.rounded_device_rect(*clip_rect), border_radii_data, CornerClip::Outside, BorderRadiusCornerClipper::UseCachedBitmap::No); + auto corner_clipper = BorderRadiusCornerClipper::create(corner_radii, context.rounded_device_rect(*clip_rect), border_radii_data, CornerClip::Outside); if (corner_clipper.is_error()) { dbgln("Failed to create overflow border-radius corner clipper: {}", corner_clipper.error()); return;