From c98ec85fb55ed0d3cef063c0b6e6aaa48aa25da5 Mon Sep 17 00:00:00 2001
From: Tim Ledbetter <timledbetter@gmail.com>
Date: Fri, 24 Feb 2023 06:12:34 +0000
Subject: [PATCH] PixelPaint: Make "Crop Image to Selection" work with disjoint
 layers

The "Crop Image to Selection" action will now crop all layers within
the current selection.

Any layers outside of the current selection are deleted.

If there are no layers within the current selection, a new layer the
same size as the selection is created, which has the same name as the
current active layer.
---
 Userland/Applications/PixelPaint/Image.cpp    | 39 +++++++++++++------
 .../Applications/PixelPaint/MainWidget.cpp    |  4 ++
 2 files changed, 31 insertions(+), 12 deletions(-)

diff --git a/Userland/Applications/PixelPaint/Image.cpp b/Userland/Applications/PixelPaint/Image.cpp
index d07a724e1a..3929197cc0 100644
--- a/Userland/Applications/PixelPaint/Image.cpp
+++ b/Userland/Applications/PixelPaint/Image.cpp
@@ -604,21 +604,24 @@ ErrorOr<void> Image::rotate(Gfx::RotationDirection direction)
 
 ErrorOr<void> Image::crop(Gfx::IntRect const& cropped_rect)
 {
+    VERIFY(!cropped_rect.is_empty());
+
     Vector<NonnullRefPtr<Layer>> cropped_layers;
     TRY(cropped_layers.try_ensure_capacity(m_layers.size()));
 
     VERIFY(m_layers.size() > 0);
 
-    size_t selected_layer_index = 0;
-    for (size_t i = 0; i < m_layers.size(); ++i) {
-        auto& layer = m_layers[i];
-        auto new_layer = TRY(Layer::create_snapshot(*this, layer));
-
+    RefPtr<Layer> selected_layer;
+    for (auto const& layer : m_layers) {
         if (layer->is_selected())
-            selected_layer_index = i;
+            selected_layer = layer;
 
-        auto layer_location = new_layer->location();
-        auto layer_local_crop_rect = new_layer->relative_rect().intersected(cropped_rect).translated(-layer_location.x(), -layer_location.y());
+        auto layer_location = layer->location();
+        auto layer_local_crop_rect = layer->relative_rect().intersected(cropped_rect).translated(-layer_location.x(), -layer_location.y());
+        if (!layer->rect().intersects(layer_local_crop_rect))
+            continue;
+
+        auto new_layer = TRY(Layer::create_snapshot(*this, layer));
         TRY(new_layer->crop(layer_local_crop_rect, Layer::NotifyClients::No));
 
         auto new_layer_x = max(0, layer_location.x() - cropped_rect.x());
@@ -629,11 +632,23 @@ ErrorOr<void> Image::crop(Gfx::IntRect const& cropped_rect)
         cropped_layers.unchecked_append(new_layer);
     }
 
-    m_layers = move(cropped_layers);
-    for (auto& layer : m_layers)
-        layer->did_modify_bitmap({}, Layer::NotifyClients::Yes);
+    if (cropped_layers.is_empty()) {
+        auto layer_name = selected_layer ? selected_layer->name() : "Background";
+        auto new_layer = TRY(Layer::create_with_size(*this, cropped_rect.size(), layer_name));
+        new_layer->set_selected(true);
+        cropped_layers.append(new_layer);
+    }
 
-    select_layer(m_layers[selected_layer_index]);
+    auto new_selected_layer = cropped_layers.last_matching([](auto& layer) {
+        return layer->is_selected();
+    });
+    selected_layer = new_selected_layer.has_value() ? new_selected_layer.release_value() : cropped_layers.first();
+    selected_layer->set_selected(true);
+
+    m_layers = move(cropped_layers);
+
+    select_layer(selected_layer);
+    did_modify_layer_stack();
 
     m_size = { cropped_rect.width(), cropped_rect.height() };
     did_change_rect(cropped_rect);
diff --git a/Userland/Applications/PixelPaint/MainWidget.cpp b/Userland/Applications/PixelPaint/MainWidget.cpp
index dbd9caa3de..b12423dcde 100644
--- a/Userland/Applications/PixelPaint/MainWidget.cpp
+++ b/Userland/Applications/PixelPaint/MainWidget.cpp
@@ -673,6 +673,10 @@ ErrorOr<void> MainWidget::initialize_menubar(GUI::Window& window)
             if (editor->image().selection().is_empty())
                 return;
             auto crop_rect = editor->image().rect().intersected(editor->image().selection().bounding_rect());
+            // FIXME: It is only possible to hit this condition, as transforming the image (crop, rotate etc.), does not update the selection.
+            //        We should ensure that image transformations also transform the selection, so that its relative size and position are maintained.
+            if (crop_rect.is_empty())
+                return;
             auto image_crop_or_error = editor->image().crop(crop_rect);
             if (image_crop_or_error.is_error()) {
                 GUI::MessageBox::show_error(&window, MUST(String::formatted("Failed to crop image: {}", image_crop_or_error.release_error())));