From 97cc34c03441cef5cb601356c474debf2e85253f Mon Sep 17 00:00:00 2001 From: Mustafa Quraish Date: Thu, 2 Sep 2021 04:26:10 -0400 Subject: [PATCH] PixelPaint: Fix the displayed order of layers in LayerListWidget Previously the background layer was shown at the top, and layers in front of it were shown below it. This was really unintuitive. This patch fixes LayerListWidget to now properly differentiate between the index of a gadget, and the index of a layer, since they are essentially mirrored. I chose not to modify the order in which layers are stored since back-to-front makes it really convenient there. --- .../PixelPaint/LayerListWidget.cpp | 75 ++++++++++++------- .../Applications/PixelPaint/LayerListWidget.h | 6 +- 2 files changed, 52 insertions(+), 29 deletions(-) diff --git a/Userland/Applications/PixelPaint/LayerListWidget.cpp b/Userland/Applications/PixelPaint/LayerListWidget.cpp index 079006b63f..a81c5a1006 100644 --- a/Userland/Applications/PixelPaint/LayerListWidget.cpp +++ b/Userland/Applications/PixelPaint/LayerListWidget.cpp @@ -1,5 +1,6 @@ /* * Copyright (c) 2020, Andreas Kling + * Copyright (c) 2021, Mustafa Quraish * * SPDX-License-Identifier: BSD-2-Clause */ @@ -27,6 +28,16 @@ LayerListWidget::~LayerListWidget() m_image->remove_client(*this); } +size_t LayerListWidget::to_gadget_index(size_t layer_index) const +{ + return m_image->layer_count() - layer_index - 1; +} + +size_t LayerListWidget::to_layer_index(size_t gadget_index) const +{ + return m_image->layer_count() - gadget_index - 1; +} + void LayerListWidget::set_image(Image* image) { if (m_image == image) @@ -44,8 +55,8 @@ void LayerListWidget::rebuild_gadgets() { m_gadgets.clear(); if (m_image) { - for (size_t layer_index = 0; layer_index < m_image->layer_count(); ++layer_index) { - m_gadgets.append({ layer_index, {}, false, {} }); + for (int layer_index = m_image->layer_count() - 1; layer_index >= 0; --layer_index) { + m_gadgets.append({ static_cast(layer_index), {}, false, {} }); } } relayout_gadgets(); @@ -140,15 +151,16 @@ void LayerListWidget::mousedown_event(GUI::MouseEvent& event) Gfx::IntPoint translated_event_point = { 0, vertical_scrollbar().value() + event.y() }; - auto gadget_index = gadget_at(translated_event_point); - if (!gadget_index.has_value()) + auto maybe_gadget_index = gadget_at(translated_event_point); + if (!maybe_gadget_index.has_value()) return; + auto gadget_index = maybe_gadget_index.value(); m_moving_gadget_index = gadget_index; - m_selected_layer_index = gadget_index.value(); + m_selected_gadget_index = gadget_index; m_moving_event_origin = translated_event_point; auto& gadget = m_gadgets[m_moving_gadget_index.value()]; - auto& layer = m_image->layer(gadget_index.value()); + auto& layer = m_image->layer(to_layer_index(gadget_index)); set_selected_layer(&layer); gadget.is_moving = true; gadget.movement_delta = {}; @@ -191,7 +203,10 @@ void LayerListWidget::mouseup_event(GUI::MouseEvent& event) new_index = m_image->layer_count() - 1; m_moving_gadget_index = {}; - m_image->change_layer_index(old_index, new_index); + + auto old_layer_index = to_layer_index(old_index); + auto new_layer_index = to_layer_index(new_index); + m_image->change_layer_index(old_layer_index, new_layer_index); } void LayerListWidget::context_menu_event(GUI::ContextMenuEvent& event) @@ -200,8 +215,8 @@ void LayerListWidget::context_menu_event(GUI::ContextMenuEvent& event) auto gadget_index = gadget_at(translated_event_point); if (gadget_index.has_value()) { - auto& layer = m_image->layer(gadget_index.value()); - m_selected_layer_index = gadget_index.value(); + m_selected_gadget_index = gadget_index.value(); + auto& layer = m_image->layer(to_layer_index(m_selected_gadget_index)); set_selected_layer(&layer); } @@ -215,8 +230,9 @@ void LayerListWidget::image_did_add_layer(size_t layer_index) m_gadgets[m_moving_gadget_index.value()].is_moving = false; m_moving_gadget_index = {}; } - Gadget gadget { layer_index, {}, false, {} }; - m_gadgets.insert(layer_index, gadget); + auto gadget_index = to_gadget_index(layer_index); + Gadget gadget { gadget_index, {}, false, {} }; + m_gadgets.insert(gadget_index, gadget); relayout_gadgets(); } @@ -226,14 +242,16 @@ void LayerListWidget::image_did_remove_layer(size_t layer_index) m_gadgets[m_moving_gadget_index.value()].is_moving = false; m_moving_gadget_index = {}; } - m_gadgets.remove(layer_index); - m_selected_layer_index = 0; + // No -1 here since a layer has already been removed. + auto gadget_index = m_image->layer_count() - layer_index; + m_gadgets.remove(gadget_index); + m_selected_gadget_index = to_gadget_index(0); relayout_gadgets(); } void LayerListWidget::image_did_modify_layer_properties(size_t layer_index) { - update(m_gadgets[layer_index].rect); + update(m_gadgets[to_gadget_index(layer_index)].rect); } void LayerListWidget::image_did_modify_layer_bitmap(size_t layer_index) @@ -241,7 +259,7 @@ void LayerListWidget::image_did_modify_layer_bitmap(size_t layer_index) Gfx::IntRect adjusted_rect; Gfx::IntRect thumbnail_rect; Gfx::IntRect text_rect; - get_gadget_rects(m_gadgets[layer_index], adjusted_rect, thumbnail_rect, text_rect); + get_gadget_rects(m_gadgets[to_gadget_index(layer_index)], adjusted_rect, thumbnail_rect, text_rect); update(thumbnail_rect); } @@ -266,7 +284,7 @@ void LayerListWidget::select_bottom_layer() { if (!m_image || !m_image->layer_count()) return; - m_selected_layer_index = 0; + m_selected_gadget_index = to_gadget_index(0); set_selected_layer(&m_image->layer(0)); } @@ -274,8 +292,8 @@ void LayerListWidget::select_top_layer() { if (!m_image || !m_image->layer_count()) return; - m_selected_layer_index = m_image->layer_count() - 1; - set_selected_layer(&m_image->layer(m_image->layer_count() - 1)); + m_selected_gadget_index = 0; + set_selected_layer(&m_image->layer(to_layer_index(0))); } void LayerListWidget::cycle_through_selection(int delta) @@ -283,16 +301,17 @@ void LayerListWidget::cycle_through_selection(int delta) if (!m_image || !m_image->layer_count()) return; - int selected_layer_index = static_cast(m_selected_layer_index); - selected_layer_index += delta; + auto current_index = static_cast(m_selected_gadget_index); + current_index += delta; - if (selected_layer_index < 0) - selected_layer_index = m_image->layer_count() - 1; - if (selected_layer_index > static_cast(m_image->layer_count()) - 1) - selected_layer_index = 0; + if (current_index < 0) + current_index = m_gadgets.size() - 1; + if (current_index > static_cast(m_gadgets.size()) - 1) + current_index = 0; - m_selected_layer_index = selected_layer_index; - set_selected_layer(&m_image->layer(m_selected_layer_index)); + m_selected_gadget_index = current_index; + auto selected_layer_index = to_layer_index(m_selected_gadget_index); + set_selected_layer(&m_image->layer(selected_layer_index)); } void LayerListWidget::relayout_gadgets() @@ -327,8 +346,8 @@ void LayerListWidget::set_selected_layer(Layer* layer) for (size_t i = 0; i < m_image->layer_count(); ++i) { if (layer == &m_image->layer(i)) { m_image->layer(i).set_selected(true); - scroll_into_view(m_gadgets[i].rect, false, true); - m_selected_layer_index = i; + m_selected_gadget_index = to_gadget_index(i); + scroll_into_view(m_gadgets[m_selected_gadget_index].rect, false, true); } else { m_image->layer(i).set_selected(false); } diff --git a/Userland/Applications/PixelPaint/LayerListWidget.h b/Userland/Applications/PixelPaint/LayerListWidget.h index 9907d24fd7..27144a3410 100644 --- a/Userland/Applications/PixelPaint/LayerListWidget.h +++ b/Userland/Applications/PixelPaint/LayerListWidget.h @@ -1,5 +1,6 @@ /* * Copyright (c) 2020, Andreas Kling + * Copyright (c) 2021, Mustafa Quraish * * SPDX-License-Identifier: BSD-2-Clause */ @@ -62,13 +63,16 @@ private: Optional gadget_at(Gfx::IntPoint const&); + size_t to_layer_index(size_t gadget_index) const; + size_t to_gadget_index(size_t layer_index) const; + Vector m_gadgets; RefPtr m_image; Optional m_moving_gadget_index; Gfx::IntPoint m_moving_event_origin; - size_t m_selected_layer_index { 0 }; + size_t m_selected_gadget_index { 0 }; }; }