From 4c147cbf0b7bf2c52b88ad30fd8a375cefe93281 Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Sun, 13 Jan 2013 10:32:13 -0800 Subject: [PATCH] Make ColourButton saner Emit a separate event for when a color is picked rather than horribly overloading the onclick event, and switch to a wxButton base since wxBitmapButton no longer does anything useful. --- aegisub/src/auto4_lua_dialog.cpp | 5 +- aegisub/src/colour_button.cpp | 93 ++++++++++------------------- aegisub/src/colour_button.h | 68 ++++++++++----------- aegisub/src/dialog_dummy_video.cpp | 7 +-- aegisub/src/dialog_style_editor.cpp | 41 ++++--------- aegisub/src/dialog_style_editor.h | 5 +- aegisub/src/preferences_base.cpp | 17 +----- 7 files changed, 81 insertions(+), 155 deletions(-) diff --git a/aegisub/src/auto4_lua_dialog.cpp b/aegisub/src/auto4_lua_dialog.cpp index 65a82f88b..446aed671 100644 --- a/aegisub/src/auto4_lua_dialog.cpp +++ b/aegisub/src/auto4_lua_dialog.cpp @@ -246,8 +246,7 @@ namespace Automation4 { wxControl *Create(wxWindow *parent) { agi::Color colour(from_wx(text)); - wxControl *cw = new ColourButton(parent, -1, wxSize(50*width,10*height), colour); - cw->SetValidator(ColorValidator(&text)); + wxControl *cw = new ColourButton(parent, wxSize(50*width,10*height), colour, ColorValidator(&text)); cw->SetToolTip(hint); return cw; } @@ -678,8 +677,6 @@ namespace Automation4 { button_pushed = evt.GetId() - 1000; // hack to make sure the dialog will be closed - // only do this for non-colour buttons - if (dynamic_cast (evt.GetEventObject())) return; evt.SetId(wxID_OK); } LOG_D("automation/lua/dialog") << "button_pushed set to: " << button_pushed; diff --git a/aegisub/src/colour_button.cpp b/aegisub/src/colour_button.cpp index fd29b7fa1..a6e68e0fb 100644 --- a/aegisub/src/colour_button.cpp +++ b/aegisub/src/colour_button.cpp @@ -1,88 +1,55 @@ -// Copyright (c) 2006, Rodrigo Braz Monteiro -// All rights reserved. +// Copyright (c) 2013, Thomas Goyne // -// Redistribution and use in source and binary forms, with or without -// modification, are permitted provided that the following conditions are met: +// Permission to use, copy, modify, and distribute this software for any +// purpose with or without fee is hereby granted, provided that the above +// copyright notice and this permission notice appear in all copies. // -// * Redistributions of source code must retain the above copyright notice, -// this list of conditions and the following disclaimer. -// * Redistributions in binary form must reproduce the above copyright notice, -// this list of conditions and the following disclaimer in the documentation -// and/or other materials provided with the distribution. -// * Neither the name of the Aegisub Group nor the names of its contributors -// may be used to endorse or promote products derived from this software -// without specific prior written permission. -// -// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" -// AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE -// IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE -// ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE -// LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR -// CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF -// SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS -// INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN -// CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) -// ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE -// POSSIBILITY OF SUCH DAMAGE. +// THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES +// WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF +// MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR +// ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES +// WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN +// ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF +// OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. // // Aegisub Project http://www.aegisub.org/ -/// @file colour_button.cpp -/// @brief Push-button that displays a colour for label, and brings up colour selection dialogue when pressed -/// @ingroup custom_control -/// - #include "config.h" -#include - #include "colour_button.h" #include "compat.h" #include "dialog_colorpicker.h" -ColourButton::ColourButton(wxWindow* parent, wxWindowID id, const wxSize& size, agi::Color col) -: wxBitmapButton(parent, id, wxBitmap(size), wxDefaultPosition, wxSize(size.GetWidth() + 6, size.GetHeight() + 6)) -, bmp(GetBitmapLabel()) +#include + +wxDEFINE_EVENT(EVT_COLOR, wxThreadEvent); + +ColourButton::ColourButton(wxWindow *parent, wxSize const& size, agi::Color col, wxValidator const& validator) +: wxButton(parent, -1, "", wxDefaultPosition, wxSize(size.GetWidth() + 6, size.GetHeight() + 6), 0, validator) +, bmp(size) , colour(col) { - Paint(); - SetBitmapLabel(bmp); - Bind(wxEVT_COMMAND_BUTTON_CLICKED, &ColourButton::OnClick, this); + UpdateBitmap(); + Bind(wxEVT_COMMAND_BUTTON_CLICKED, [=](wxCommandEvent&) { + GetColorFromUser(GetParent(), colour, this); + }); } -void ColourButton::Paint() { +void ColourButton::UpdateBitmap() { wxMemoryDC dc; dc.SelectObject(bmp); dc.SetBrush(wxBrush(to_wx(colour))); - dc.DrawRectangle(0,0,bmp.GetWidth(),bmp.GetHeight()); + dc.DrawRectangle(0, 0, bmp.GetWidth(), bmp.GetHeight()); + SetBitmapLabel(bmp); } -/// @brief Callback for the color picker dialog -/// @param col New color -void ColourButton::SetColour(agi::Color col) { - colour = col; +void ColourButton::SetColour(agi::Color new_color) { + colour = new_color; + UpdateBitmap(); - // Draw colour - Paint(); - SetBitmapLabel(bmp); - - // Trigger a click event on this as some stuff relies on that to know - // when the color has changed - wxCommandEvent evt(wxEVT_COMMAND_BUTTON_CLICKED, GetId()); - evt.SetClientData(this); + wxThreadEvent evt(EVT_COLOR, GetId()); evt.SetEventObject(this); + evt.SetPayload(colour); AddPendingEvent(evt); } - -agi::Color ColourButton::GetColor() { - return colour; -} - -void ColourButton::OnClick(wxCommandEvent &event) { - if (event.GetClientData() == this) - event.Skip(); - else { - GetColorFromUser(GetParent(), colour, this); - } -} diff --git a/aegisub/src/colour_button.h b/aegisub/src/colour_button.h index 4fa49a429..d279da1c0 100644 --- a/aegisub/src/colour_button.h +++ b/aegisub/src/colour_button.h @@ -1,51 +1,45 @@ -// Copyright (c) 2007, Rodrigo Braz Monteiro -// All rights reserved. +// Copyright (c) 2013, Thomas Goyne // -// Redistribution and use in source and binary forms, with or without -// modification, are permitted provided that the following conditions are met: +// Permission to use, copy, modify, and distribute this software for any +// purpose with or without fee is hereby granted, provided that the above +// copyright notice and this permission notice appear in all copies. // -// * Redistributions of source code must retain the above copyright notice, -// this list of conditions and the following disclaimer. -// * Redistributions in binary form must reproduce the above copyright notice, -// this list of conditions and the following disclaimer in the documentation -// and/or other materials provided with the distribution. -// * Neither the name of the Aegisub Group nor the names of its contributors -// may be used to endorse or promote products derived from this software -// without specific prior written permission. -// -// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" -// AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE -// IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE -// ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE -// LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR -// CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF -// SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS -// INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN -// CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) -// ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE -// POSSIBILITY OF SUCH DAMAGE. +// THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES +// WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF +// MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR +// ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES +// WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN +// ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF +// OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. // // Aegisub Project http://www.aegisub.org/ -/// @file colour_button.h -/// @see colour_button.cpp -/// @ingroup custom_control -/// - -#include +#include #include -class ColourButton: public wxBitmapButton { - wxBitmap bmp; /// The button's bitmap label - agi::Color colour; /// The current colour +/// Emitted by ColourButton when the user picks a new color, with the chosen +/// color set to the event payload +wxDECLARE_EVENT(EVT_COLOR, wxThreadEvent); - void Paint(); - void OnClick(wxCommandEvent &event); +/// A button which displays a currently-selected color and lets the user pick +/// a new color when clicked +class ColourButton: public wxButton { + wxBitmap bmp; ///< The button's bitmap label + agi::Color colour; ///< The current colour + + /// Update the bitmap label after the color is changed + void UpdateBitmap(); + /// Callback for the colorpicker dialog void SetColour(agi::Color colour); public: - ColourButton(wxWindow* parent, wxWindowID id, const wxSize& size, agi::Color col = agi::Color()); + /// Constructor + /// @param parent Parent window + /// @param size Size of the bitmap (note: not the size of the button) + /// @param color Initial color to display + ColourButton(wxWindow *parent, wxSize const& size, agi::Color color = agi::Color(), wxValidator const& validator = wxDefaultValidator); - agi::Color GetColor(); + /// Get the currently selected color + agi::Color GetColor() { return colour; } }; diff --git a/aegisub/src/dialog_dummy_video.cpp b/aegisub/src/dialog_dummy_video.cpp index 1ecabd7ea..75bd6c36e 100644 --- a/aegisub/src/dialog_dummy_video.cpp +++ b/aegisub/src/dialog_dummy_video.cpp @@ -104,7 +104,7 @@ DialogDummyVideo::DialogDummyVideo(wxWindow *parent) res_sizer->Add(spin_ctrl(this, 1, 10000, &height), wxSizerFlags(1).Expand()); wxBoxSizer *color_sizer = new wxBoxSizer(wxHORIZONTAL); - ColourButton *color_btn = new ColourButton(this, -1, wxSize(30, 17), color); + ColourButton *color_btn = new ColourButton(this, wxSize(30, 17), color); color_sizer->Add(color_btn, wxSizerFlags().DoubleBorder(wxRIGHT)); color_sizer->Add(new wxCheckBox(this, -1, _("Checkerboard &pattern"), wxDefaultPosition, wxDefaultSize, 0, wxGenericValidator(&pattern)), wxSizerFlags(1).Center()); @@ -130,10 +130,7 @@ DialogDummyVideo::DialogDummyVideo(wxWindow *parent) CenterOnParent(); Bind(wxEVT_COMMAND_COMBOBOX_SELECTED, &DialogDummyVideo::OnResolutionShortcut, this); - color_btn->Bind(wxEVT_COMMAND_BUTTON_CLICKED, [=](wxCommandEvent& e) { - color = color_btn->GetColor(); - e.Skip(); - }); + color_btn->Bind(EVT_COLOR, [=](wxThreadEvent& e) { color = color_btn->GetColor(); }); Bind(wxEVT_COMMAND_SPINCTRL_UPDATED, [=](wxCommandEvent&) { TransferDataFromWindow(); UpdateLengthDisplay(); diff --git a/aegisub/src/dialog_style_editor.cpp b/aegisub/src/dialog_style_editor.cpp index a329bcc01..569f17927 100644 --- a/aegisub/src/dialog_style_editor.cpp +++ b/aegisub/src/dialog_style_editor.cpp @@ -191,14 +191,10 @@ DialogStyleEditor::DialogStyleEditor(wxWindow *parent, AssStyle *style, agi::Con BoxItalic = new wxCheckBox(this, -1, _("&Italic")); BoxUnderline = new wxCheckBox(this, -1, _("&Underline")); BoxStrikeout = new wxCheckBox(this, -1, _("&Strikeout")); - colorButton[0] = new ColourButton(this, -1, wxSize(55, 16), style->primary); - colorButton[1] = new ColourButton(this, -1, wxSize(55, 16), style->secondary); - colorButton[2] = new ColourButton(this, -1, wxSize(55, 16), style->outline); - colorButton[3] = new ColourButton(this, -1, wxSize(55, 16), style->shadow); - colorAlpha[0] = spin_ctrl(this, style->primary.a, 255); - colorAlpha[1] = spin_ctrl(this, style->secondary.a, 255); - colorAlpha[2] = spin_ctrl(this, style->outline.a, 255); - colorAlpha[3] = spin_ctrl(this, style->shadow.a, 255); + colorButton[0] = new ColourButton(this, wxSize(55, 16), style->primary); + colorButton[1] = new ColourButton(this, wxSize(55, 16), style->secondary); + colorButton[2] = new ColourButton(this, wxSize(55, 16), style->outline); + colorButton[3] = new ColourButton(this, wxSize(55, 16), style->shadow); for (int i = 0; i < 3; i++) margin[i] = spin_ctrl(this, style->Margin[i], 9999); Alignment = new wxRadioBox(this, -1, _("Alignment"), wxDefaultPosition, wxDefaultSize, 9, alignValues, 3, wxRA_SPECIFY_COLS); @@ -219,7 +215,6 @@ DialogStyleEditor::DialogStyleEditor(wxWindow *parent, AssStyle *style, agi::Con colorButton[1]->SetToolTip(_("Choose secondary color")); colorButton[2]->SetToolTip(_("Choose outline color")); colorButton[3]->SetToolTip(_("Choose shadow color")); - for (int i=0;i<4;i++) colorAlpha[i]->SetToolTip(_("Set opacity, from 0 (opaque) to 255 (transparent)")); margin[0]->SetToolTip(_("Distance from left edge, in pixels")); margin[1]->SetToolTip(_("Distance from right edge, in pixels")); margin[2]->SetToolTip(_("Distance from top/bottom edge, in pixels")); @@ -282,7 +277,6 @@ DialogStyleEditor::DialogStyleEditor(wxWindow *parent, AssStyle *style, agi::Con ColorSizer[i] = new wxBoxSizer(wxVERTICAL); ColorSizer[i]->Add(new wxStaticText(this, -1, colorLabels[i]), 0, wxBOTTOM | wxALIGN_CENTER, 5); ColorSizer[i]->Add(colorButton[i], 0, wxBOTTOM | wxALIGN_CENTER, 5); - ColorSizer[i]->Add(colorAlpha[i], 0, wxALIGN_CENTER, 0); ColorsSizer->Add(ColorSizer[i], 0, wxLEFT, i?5:0); } ColorsSizer->AddStretchSpacer(1); @@ -330,7 +324,7 @@ DialogStyleEditor::DialogStyleEditor(wxWindow *parent, AssStyle *style, agi::Con ColourButton *previewButton = 0; if (!SubtitlesProviderFactory::GetClasses().empty()) { PreviewText = new wxTextCtrl(this, -1, to_wx(OPT_GET("Tool/Style Editor/Preview Text")->GetString())); - previewButton = new ColourButton(this, -1, wxSize(45, 16), OPT_GET("Colour/Style Editor/Background/Preview")->GetColor()); + previewButton = new ColourButton(this, wxSize(45, 16), OPT_GET("Colour/Style Editor/Background/Preview")->GetColor()); SubsPreview = new SubtitlesPreview(this, wxSize(100, 60), wxSUNKEN_BORDER, OPT_GET("Colour/Style Editor/Background/Preview")->GetColor()); SubsPreview->SetToolTip(_("Preview of current style")); @@ -405,7 +399,7 @@ DialogStyleEditor::DialogStyleEditor(wxWindow *parent, AssStyle *style, agi::Con Bind(wxEVT_COMMAND_BUTTON_CLICKED, std::bind(&HelpButton::OpenPage, "Style Editor"), wxID_HELP); for (int i = 0; i < 4; ++i) - colorButton[i]->Bind(wxEVT_COMMAND_BUTTON_CLICKED, std::bind(&DialogStyleEditor::OnSetColor, this, i + 1, std::placeholders::_1)); + colorButton[i]->Bind(EVT_COLOR, std::bind(&DialogStyleEditor::OnSetColor, this, i + 1, std::placeholders::_1)); } DialogStyleEditor::~DialogStyleEditor() { @@ -505,31 +499,18 @@ void DialogStyleEditor::UpdateWorkStyle() { for (size_t i = 0; i < 3; ++i) work->Margin[i] = margin[i]->GetValue(); - work->primary.a = colorAlpha[0]->GetValue(); - work->secondary.a = colorAlpha[1]->GetValue(); - work->outline.a = colorAlpha[2]->GetValue(); - work->shadow.a = colorAlpha[3]->GetValue(); - work->bold = BoxBold->IsChecked(); work->italic = BoxItalic->IsChecked(); work->underline = BoxUnderline->IsChecked(); work->strikeout = BoxStrikeout->IsChecked(); } -/// @brief Sets color for one of the four color buttons -/// @param n Colour to set -void DialogStyleEditor::OnSetColor(int n, wxCommandEvent& evt) { - ColourButton *btn = static_cast(evt.GetClientData()); - if (!btn) { - evt.Skip(); - return; - } - +void DialogStyleEditor::OnSetColor(int n, wxThreadEvent& evt) { switch (n) { - case 1: work->primary = btn->GetColor(); break; - case 2: work->secondary = btn->GetColor(); break; - case 3: work->outline = btn->GetColor(); break; - case 4: work->shadow = btn->GetColor(); break; + case 1: work->primary = evt.GetPayload(); break; + case 2: work->secondary = evt.GetPayload(); break; + case 3: work->outline = evt.GetPayload(); break; + case 4: work->shadow = evt.GetPayload(); break; default: throw agi::InternalError("attempted setting colour id outside range", 0); } if (SubsPreview) diff --git a/aegisub/src/dialog_style_editor.h b/aegisub/src/dialog_style_editor.h index 991183b0e..b80baf00d 100644 --- a/aegisub/src/dialog_style_editor.h +++ b/aegisub/src/dialog_style_editor.h @@ -74,7 +74,6 @@ class DialogStyleEditor : public wxDialog { wxCheckBox *BoxUnderline; wxCheckBox *BoxStrikeout; ColourButton *colorButton[4]; - wxSpinCtrl *colorAlpha[4]; wxSpinCtrl *margin[3]; wxRadioBox *Alignment; wxTextCtrl *Outline; @@ -103,7 +102,9 @@ class DialogStyleEditor : public wxDialog { /// @param apply Should changes be applied? /// @param close Should the dialog be closed? void Apply(bool apply,bool close); - void OnSetColor(int n, wxCommandEvent& evt); + /// @brief Sets color for one of the four color buttons + /// @param n Colour to set + void OnSetColor(int n, wxThreadEvent& evt); public: DialogStyleEditor(wxWindow *parent, AssStyle *style, agi::Context *c, AssStyleStorage *store = 0, std::string const& new_name = ""); diff --git a/aegisub/src/preferences_base.cpp b/aegisub/src/preferences_base.cpp index bdc4f24ea..4ee0768a8 100644 --- a/aegisub/src/preferences_base.cpp +++ b/aegisub/src/preferences_base.cpp @@ -61,18 +61,7 @@ OPTION_UPDATER(IntUpdater, wxSpinEvent, OptionValueInt, evt.GetInt()); OPTION_UPDATER(IntCBUpdater, wxCommandEvent, OptionValueInt, evt.GetInt()); OPTION_UPDATER(DoubleUpdater, wxSpinEvent, OptionValueDouble, evt.GetInt()); OPTION_UPDATER(BoolUpdater, wxCommandEvent, OptionValueBool, !!evt.GetInt()); -class ColourUpdater { - const char *name; - Preferences *parent; -public: - ColourUpdater(const char *n = "", Preferences *p = nullptr) : name(n), parent(p) { } - void operator()(wxCommandEvent& evt) { - ColourButton *btn = static_cast(evt.GetClientData()); - if (btn) - parent->SetOption(new agi::OptionValueColor(name, btn->GetColor())); - evt.Skip(); - } -}; +OPTION_UPDATER(ColourUpdater, wxThreadEvent, OptionValueColor, evt.GetPayload()); static void browse_button(wxTextCtrl *ctrl) { wxString def = StandardPaths::DecodePath(ctrl->GetValue()); @@ -161,8 +150,8 @@ wxControl *OptionPage::OptionAdd(wxFlexGridSizer *flex, const wxString &name, co } case agi::OptionValue::Type_Color: { - ColourButton *cb = new ColourButton(this, -1, wxSize(40,10), opt->GetColor()); - cb->Bind(wxEVT_COMMAND_BUTTON_CLICKED, ColourUpdater(opt_name, parent)); + ColourButton *cb = new ColourButton(this, wxSize(40,10), opt->GetColor()); + cb->Bind(EVT_COLOR, ColourUpdater(opt_name, parent)); Add(flex, name, cb); return cb; }