From 24da32c88433264aaf7a32969b3e48647f52efd9 Mon Sep 17 00:00:00 2001
From: Aliaksandr Kalenik <kalenik.aliaksandr@gmail.com>
Date: Wed, 29 Nov 2023 18:18:00 +0100
Subject: [PATCH] LibAccelGfx+LibWeb: Store state of all stacking contexts in
 GPU painter

This change ensures that the GPU painting executor follows the pattern
of the CPU executor, where the state is stored for each stacking
context, but a painter is created only for those with opacity.

Fixes crashing on apple.com because now save() and restore() are called
on correct painters.
---
 Userland/Libraries/LibAccelGfx/Painter.cpp      |  2 +-
 Userland/Libraries/LibAccelGfx/Painter.h        |  2 +-
 .../Painting/PaintingCommandExecutorGPU.cpp     | 17 ++++++++++++-----
 .../Painting/PaintingCommandExecutorGPU.h       | 10 ++++++----
 4 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/Userland/Libraries/LibAccelGfx/Painter.cpp b/Userland/Libraries/LibAccelGfx/Painter.cpp
index 0a3c90658e..d463787fa4 100644
--- a/Userland/Libraries/LibAccelGfx/Painter.cpp
+++ b/Userland/Libraries/LibAccelGfx/Painter.cpp
@@ -137,7 +137,7 @@ void main() {
 
 HashMap<u32, GL::Texture> s_immutable_bitmap_texture_cache;
 
-OwnPtr<Painter> Painter::create()
+NonnullOwnPtr<Painter> Painter::create()
 {
     auto& context = Context::the();
     return make<Painter>(context);
diff --git a/Userland/Libraries/LibAccelGfx/Painter.h b/Userland/Libraries/LibAccelGfx/Painter.h
index cde3f519b3..adf25beb2f 100644
--- a/Userland/Libraries/LibAccelGfx/Painter.h
+++ b/Userland/Libraries/LibAccelGfx/Painter.h
@@ -29,7 +29,7 @@ class Painter {
     AK_MAKE_NONMOVABLE(Painter);
 
 public:
-    static OwnPtr<Painter> create();
+    static NonnullOwnPtr<Painter> create();
 
     Painter(Context&);
     ~Painter();
diff --git a/Userland/Libraries/LibWeb/Painting/PaintingCommandExecutorGPU.cpp b/Userland/Libraries/LibWeb/Painting/PaintingCommandExecutorGPU.cpp
index 08384fda2b..6a70322259 100644
--- a/Userland/Libraries/LibWeb/Painting/PaintingCommandExecutorGPU.cpp
+++ b/Userland/Libraries/LibWeb/Painting/PaintingCommandExecutorGPU.cpp
@@ -15,7 +15,7 @@ PaintingCommandExecutorGPU::PaintingCommandExecutorGPU(Gfx::Bitmap& bitmap)
     auto painter = AccelGfx::Painter::create();
     auto canvas = AccelGfx::Canvas::create(bitmap.size());
     painter->set_target_canvas(canvas);
-    stacking_contexts.append({ .canvas = canvas,
+    m_stacking_contexts.append({ .canvas = canvas,
         .painter = move(painter),
         .opacity = 1.0f,
         .destination = {} });
@@ -23,7 +23,7 @@ PaintingCommandExecutorGPU::PaintingCommandExecutorGPU(Gfx::Bitmap& bitmap)
 
 PaintingCommandExecutorGPU::~PaintingCommandExecutorGPU()
 {
-    VERIFY(stacking_contexts.size() == 1);
+    VERIFY(m_stacking_contexts.size() == 1);
     painter().flush(m_target_bitmap);
 }
 
@@ -92,6 +92,7 @@ CommandResult PaintingCommandExecutorGPU::set_font(Gfx::Font const&)
 
 CommandResult PaintingCommandExecutorGPU::push_stacking_context(float opacity, bool is_fixed_position, Gfx::IntRect const& source_paintable_rect, Gfx::IntPoint post_transform_translation, CSS::ImageRendering, StackingContextTransform transform, Optional<StackingContextMask>)
 {
+    m_stacking_contexts.last().stacking_context_depth++;
     painter().save();
     if (is_fixed_position) {
         auto const& translation = painter().transform().translation();
@@ -110,23 +111,29 @@ CommandResult PaintingCommandExecutorGPU::push_stacking_context(float opacity, b
         auto transformed_destination_rect = affine_transform.map(source_rect).translated(transform.origin);
         auto destination_rect = transformed_destination_rect.to_rounded<int>();
 
-        stacking_contexts.append({ .canvas = canvas,
+        m_stacking_contexts.append({ .canvas = canvas,
             .painter = move(painter),
             .opacity = opacity,
             .destination = destination_rect });
     } else {
         painter().translate(affine_transform.translation() + post_transform_translation.to_type<float>());
+        m_stacking_contexts.append({ .canvas = {},
+            .painter = MaybeOwned(painter()),
+            .opacity = opacity,
+            .destination = {} });
     }
     return CommandResult::Continue;
 }
 
 CommandResult PaintingCommandExecutorGPU::pop_stacking_context()
 {
-    if (stacking_contexts.last().opacity < 1) {
-        auto stacking_context = stacking_contexts.take_last();
+    auto stacking_context = m_stacking_contexts.take_last();
+    VERIFY(stacking_context.stacking_context_depth == 0);
+    if (stacking_context.painter.is_owned()) {
         painter().blit_canvas(stacking_context.destination, *stacking_context.canvas, stacking_context.opacity);
     }
     painter().restore();
+    m_stacking_contexts.last().stacking_context_depth--;
     return CommandResult::Continue;
 }
 
diff --git a/Userland/Libraries/LibWeb/Painting/PaintingCommandExecutorGPU.h b/Userland/Libraries/LibWeb/Painting/PaintingCommandExecutorGPU.h
index fba5fa605b..2989a92b64 100644
--- a/Userland/Libraries/LibWeb/Painting/PaintingCommandExecutorGPU.h
+++ b/Userland/Libraries/LibWeb/Painting/PaintingCommandExecutorGPU.h
@@ -6,6 +6,7 @@
 
 #pragma once
 
+#include <AK/MaybeOwned.h>
 #include <LibAccelGfx/Painter.h>
 #include <LibWeb/Painting/RecordingPainter.h>
 
@@ -63,15 +64,16 @@ private:
 
     struct StackingContext {
         RefPtr<AccelGfx::Canvas> canvas;
-        OwnPtr<AccelGfx::Painter> painter;
+        MaybeOwned<AccelGfx::Painter> painter;
         float opacity;
         Gfx::IntRect destination;
+        int stacking_context_depth { 0 };
     };
 
-    [[nodiscard]] AccelGfx::Painter const& painter() const { return *stacking_contexts.last().painter; }
-    [[nodiscard]] AccelGfx::Painter& painter() { return *stacking_contexts.last().painter; }
+    [[nodiscard]] AccelGfx::Painter const& painter() const { return *m_stacking_contexts.last().painter; }
+    [[nodiscard]] AccelGfx::Painter& painter() { return *m_stacking_contexts.last().painter; }
 
-    Vector<StackingContext> stacking_contexts;
+    Vector<StackingContext> m_stacking_contexts;
 };
 
 }