From b4435bd50c69639cce961578c7afda6ecf5b46d0 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Thu, 24 Jul 2025 12:36:47 +0200 Subject: [PATCH] LibWeb: Don't clip descendants outside stacking context root rect Skia allows you to pass a bounding rect to its saveLayer() function as an optimization when you know that you won't paint outside those bounds. Unfortunately, we were passing a too-small rectangle that didn't take into account transformed descendants, etc. For now, simply pass null instead of a bounding rect. This way, Skia figures it out internally. It may allocate larger temporary bitmaps than needed this way, but at least we get more correct results. I've left re-enabling the optimization as a FIXME in the code. This fixes unwanted clipping in various parts of the Discord UI. --- Libraries/LibWeb/Painting/Command.h | 3 --- .../LibWeb/Painting/DisplayListPlayerSkia.cpp | 9 ++++----- Libraries/LibWeb/Painting/DisplayListRecorder.cpp | 1 - Libraries/LibWeb/Painting/DisplayListRecorder.h | 1 - Libraries/LibWeb/Painting/StackingContext.cpp | 1 - ...-with-unclipped-transformed-descendant-ref.html | 11 +++++++++++ ...text-with-unclipped-transformed-descendant.html | 14 ++++++++++++++ 7 files changed, 29 insertions(+), 11 deletions(-) create mode 100644 Tests/LibWeb/Ref/expected/css/stacking-context-with-unclipped-transformed-descendant-ref.html create mode 100644 Tests/LibWeb/Ref/input/css/stacking-context-with-unclipped-transformed-descendant.html diff --git a/Libraries/LibWeb/Painting/Command.h b/Libraries/LibWeb/Painting/Command.h index 25a7289da72..ef23cad21b4 100644 --- a/Libraries/LibWeb/Painting/Command.h +++ b/Libraries/LibWeb/Painting/Command.h @@ -130,15 +130,12 @@ struct PushStackingContext { float opacity; Gfx::CompositingAndBlendingOperator compositing_and_blending_operator; bool isolate; - // The bounding box of the source paintable (pre-transform). - Gfx::IntRect source_paintable_rect; // A translation to be applied after the stacking context has been transformed. StackingContextTransform transform; Optional clip_path = {}; void translate_by(Gfx::IntPoint const& offset) { - source_paintable_rect.translate_by(offset); transform.origin.translate_by(offset.to_type()); if (clip_path.has_value()) { clip_path.value().transform(Gfx::AffineTransform().translate(offset.to_type())); diff --git a/Libraries/LibWeb/Painting/DisplayListPlayerSkia.cpp b/Libraries/LibWeb/Painting/DisplayListPlayerSkia.cpp index b8676352dbe..e426e23ad49 100644 --- a/Libraries/LibWeb/Painting/DisplayListPlayerSkia.cpp +++ b/Libraries/LibWeb/Painting/DisplayListPlayerSkia.cpp @@ -211,14 +211,13 @@ void DisplayListPlayerSkia::push_stacking_context(PushStackingContext const& com auto matrix = to_skia_matrix(new_transform); if (command.opacity < 1 || command.compositing_and_blending_operator != Gfx::CompositingAndBlendingOperator::Normal || command.isolate) { - auto source_paintable_rect = to_skia_rect(command.source_paintable_rect); - SkRect dest; - matrix.mapRect(&dest, source_paintable_rect); - SkPaint paint; paint.setAlphaf(command.opacity); paint.setBlender(Gfx::to_skia_blender(command.compositing_and_blending_operator)); - canvas.saveLayer(&dest, &paint); + + // FIXME: If we knew the bounds of the stacking context including any transformed descendants etc, + // we could use saveLayer with a bounds rect. For now, we pass nullptr and let Skia figure it out. + canvas.saveLayer(nullptr, &paint); } else { canvas.save(); } diff --git a/Libraries/LibWeb/Painting/DisplayListRecorder.cpp b/Libraries/LibWeb/Painting/DisplayListRecorder.cpp index acc1de2ac6f..19f239fa9ee 100644 --- a/Libraries/LibWeb/Painting/DisplayListRecorder.cpp +++ b/Libraries/LibWeb/Painting/DisplayListRecorder.cpp @@ -330,7 +330,6 @@ void DisplayListRecorder::push_stacking_context(PushStackingContextParams params .opacity = params.opacity, .compositing_and_blending_operator = params.compositing_and_blending_operator, .isolate = params.isolate, - .source_paintable_rect = params.source_paintable_rect, .transform = { .origin = params.transform.origin, .matrix = params.transform.matrix, diff --git a/Libraries/LibWeb/Painting/DisplayListRecorder.h b/Libraries/LibWeb/Painting/DisplayListRecorder.h index 9443fcc4b53..4667c922096 100644 --- a/Libraries/LibWeb/Painting/DisplayListRecorder.h +++ b/Libraries/LibWeb/Painting/DisplayListRecorder.h @@ -124,7 +124,6 @@ public: Gfx::CompositingAndBlendingOperator compositing_and_blending_operator; bool isolate; bool is_fixed_position; - Gfx::IntRect source_paintable_rect; StackingContextTransform transform; Optional clip_path = {}; }; diff --git a/Libraries/LibWeb/Painting/StackingContext.cpp b/Libraries/LibWeb/Painting/StackingContext.cpp index acb0f1569f7..f205d401d4e 100644 --- a/Libraries/LibWeb/Painting/StackingContext.cpp +++ b/Libraries/LibWeb/Painting/StackingContext.cpp @@ -324,7 +324,6 @@ void StackingContext::paint(PaintContext& context) const .compositing_and_blending_operator = compositing_and_blending_operator, .isolate = paintable_box().computed_values().isolation() == CSS::Isolation::Isolate, .is_fixed_position = paintable_box().is_fixed_position(), - .source_paintable_rect = source_paintable_rect, .transform = { .origin = transform_origin.scaled(to_device_pixels_scale), .matrix = matrix_with_scaled_translation(transform_matrix, to_device_pixels_scale), diff --git a/Tests/LibWeb/Ref/expected/css/stacking-context-with-unclipped-transformed-descendant-ref.html b/Tests/LibWeb/Ref/expected/css/stacking-context-with-unclipped-transformed-descendant-ref.html new file mode 100644 index 00000000000..8542c81624c --- /dev/null +++ b/Tests/LibWeb/Ref/expected/css/stacking-context-with-unclipped-transformed-descendant-ref.html @@ -0,0 +1,11 @@ + diff --git a/Tests/LibWeb/Ref/input/css/stacking-context-with-unclipped-transformed-descendant.html b/Tests/LibWeb/Ref/input/css/stacking-context-with-unclipped-transformed-descendant.html new file mode 100644 index 00000000000..65816424a0b --- /dev/null +++ b/Tests/LibWeb/Ref/input/css/stacking-context-with-unclipped-transformed-descendant.html @@ -0,0 +1,14 @@ + + +