From d27d19f012dde284539a5c96becca29eace4015e Mon Sep 17 00:00:00 2001 From: Karol Kosek Date: Mon, 13 Feb 2023 18:42:45 +0100 Subject: [PATCH] PixelPaint: Set Tool on_*_color_change logic using virtual functions Previously, we were rewriting the on_primary_color_change in the Text Tool and Gradient, which made the palette widget no longer update after picking a color from an image. Additionally, it also crashed the program after leaving the Gradient tool and trying to change color. --- Userland/Applications/PixelPaint/MainWidget.cpp | 11 +++++++++++ Userland/Applications/PixelPaint/PaletteWidget.cpp | 8 -------- .../Applications/PixelPaint/Tools/GradientTool.cpp | 11 ++++++----- Userland/Applications/PixelPaint/Tools/GradientTool.h | 1 + Userland/Applications/PixelPaint/Tools/TextTool.cpp | 6 ++---- Userland/Applications/PixelPaint/Tools/TextTool.h | 2 +- Userland/Applications/PixelPaint/Tools/Tool.h | 2 ++ 7 files changed, 23 insertions(+), 18 deletions(-) diff --git a/Userland/Applications/PixelPaint/MainWidget.cpp b/Userland/Applications/PixelPaint/MainWidget.cpp index f54f99f264..8978249a92 100644 --- a/Userland/Applications/PixelPaint/MainWidget.cpp +++ b/Userland/Applications/PixelPaint/MainWidget.cpp @@ -1229,6 +1229,17 @@ ImageEditor& MainWidget::create_new_editor(NonnullRefPtr image) }, 100); + image_editor.on_primary_color_change = [&](Color color) { + m_palette_widget->set_primary_color(color); + if (image_editor.active_tool()) + image_editor.active_tool()->on_primary_color_change(color); + }; + image_editor.on_secondary_color_change = [&](Color color) { + m_palette_widget->set_secondary_color(color); + if (image_editor.active_tool()) + image_editor.active_tool()->on_secondary_color_change(color); + }; + if (image->layer_count()) image_editor.set_active_layer(&image->layer(0)); diff --git a/Userland/Applications/PixelPaint/PaletteWidget.cpp b/Userland/Applications/PixelPaint/PaletteWidget.cpp index aab595cb64..b5dd9b0833 100644 --- a/Userland/Applications/PixelPaint/PaletteWidget.cpp +++ b/Userland/Applications/PixelPaint/PaletteWidget.cpp @@ -150,14 +150,6 @@ void PaletteWidget::set_image_editor(ImageEditor* editor) set_primary_color(editor->primary_color()); set_secondary_color(editor->secondary_color()); - - editor->on_primary_color_change = [this](Color color) { - set_primary_color(color); - }; - - editor->on_secondary_color_change = [this](Color color) { - set_secondary_color(color); - }; } void PaletteWidget::set_primary_color(Color color) diff --git a/Userland/Applications/PixelPaint/Tools/GradientTool.cpp b/Userland/Applications/PixelPaint/Tools/GradientTool.cpp index 9d035f00ae..ac7faf6a8c 100644 --- a/Userland/Applications/PixelPaint/Tools/GradientTool.cpp +++ b/Userland/Applications/PixelPaint/Tools/GradientTool.cpp @@ -163,13 +163,14 @@ void GradientTool::on_second_paint(Layer const* layer, GUI::PaintEvent& event) draw_gradient(painter, true, m_editor->content_to_frame_position(Gfx::IntPoint(0, 0)), m_editor->scale(), m_editor->content_rect()); } +void GradientTool::on_primary_color_change(Color) +{ + if (m_gradient_end.has_value()) + m_editor->update(); +} + void GradientTool::on_tool_activation() { - m_editor->on_primary_color_change = [this](Color) { - if (m_gradient_end.has_value()) - m_editor->update(); - }; - reset(); } diff --git a/Userland/Applications/PixelPaint/Tools/GradientTool.h b/Userland/Applications/PixelPaint/Tools/GradientTool.h index 883086f068..6e1b47a683 100644 --- a/Userland/Applications/PixelPaint/Tools/GradientTool.h +++ b/Userland/Applications/PixelPaint/Tools/GradientTool.h @@ -20,6 +20,7 @@ public: virtual void on_mouseup(Layer*, MouseEvent&) override; virtual bool on_keydown(GUI::KeyEvent&) override; virtual void on_keyup(GUI::KeyEvent&) override; + virtual void on_primary_color_change(Color) override; virtual void on_tool_activation() override; virtual GUI::Widget* get_properties_widget() override; diff --git a/Userland/Applications/PixelPaint/Tools/TextTool.cpp b/Userland/Applications/PixelPaint/Tools/TextTool.cpp index f03a0d30f5..67a8c89787 100644 --- a/Userland/Applications/PixelPaint/Tools/TextTool.cpp +++ b/Userland/Applications/PixelPaint/Tools/TextTool.cpp @@ -41,11 +41,9 @@ TextTool::TextTool() }).release_value_but_fixme_should_propagate_errors(); } -void TextTool::on_tool_activation() +void TextTool::on_primary_color_change(Color color) { - m_editor->on_primary_color_change = [this](auto color) { - m_text_color = color; - }; + m_text_color = color; } void TextTool::on_tool_deactivation() diff --git a/Userland/Applications/PixelPaint/Tools/TextTool.h b/Userland/Applications/PixelPaint/Tools/TextTool.h index 3d9f777ee9..e94573dc4d 100644 --- a/Userland/Applications/PixelPaint/Tools/TextTool.h +++ b/Userland/Applications/PixelPaint/Tools/TextTool.h @@ -37,7 +37,7 @@ public: virtual void on_mousedown(Layer*, MouseEvent&) override; virtual bool on_keydown(GUI::KeyEvent&) override; virtual void on_second_paint(Layer const*, GUI::PaintEvent&) override; - virtual void on_tool_activation() override; + virtual void on_primary_color_change(Color) override; virtual void on_tool_deactivation() override; virtual Variant> cursor() override; virtual GUI::Widget* get_properties_widget() override; diff --git a/Userland/Applications/PixelPaint/Tools/Tool.h b/Userland/Applications/PixelPaint/Tools/Tool.h index b61c83ede8..0c7d093040 100644 --- a/Userland/Applications/PixelPaint/Tools/Tool.h +++ b/Userland/Applications/PixelPaint/Tools/Tool.h @@ -63,6 +63,8 @@ public: virtual void on_second_paint(Layer const*, GUI::PaintEvent&) { } virtual bool on_keydown(GUI::KeyEvent&); virtual void on_keyup(GUI::KeyEvent&) { } + virtual void on_primary_color_change(Color) { } + virtual void on_secondary_color_change(Color) { } virtual void on_tool_activation() { } virtual void on_tool_deactivation() { } virtual GUI::Widget* get_properties_widget() { return nullptr; }