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

@@ -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);
+				wxControl *cw = new ColourButton(parent, wxSize(50*width,10*height), colour, ColorValidator(&text));
-				cw->SetValidator(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<ColourButton*> (evt.GetEventObject())) return;
 			evt.SetId(wxID_OK);
 		}
 		LOG_D("automation/lua/dialog") << "button_pushed set to: " << button_pushed;

aegisub/src/colour_button.cpp

@@ -1,88 +1,55 @@
-// Copyright (c) 2006, Rodrigo Braz Monteiro
+// Copyright (c) 2013, Thomas Goyne <plorkyeran@aegisub.org>
-// All rights reserved.
 //
-// Redistribution and use in source and binary forms, with or without
+// Permission to use, copy, modify, and distribute this software for any
-// modification, are permitted provided that the following conditions are met:
+// 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,
+// THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
-//     this list of conditions and the following disclaimer.
+// WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
-//   * Redistributions in binary form must reproduce the above copyright notice,
+// MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
-//     this list of conditions and the following disclaimer in the documentation
+// ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
-//     and/or other materials provided with the distribution.
+// WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
-//   * Neither the name of the Aegisub Group nor the names of its contributors
+// ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
-//     may be used to endorse or promote products derived from this software
+// OR IN CONNECTION WITH THE USE OR PERFORMANCE OF 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.
 //
 // 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 <wx/dcmemory.h>
-
 #include "colour_button.h"
 
 #include "compat.h"
 #include "dialog_colorpicker.h"
 
-ColourButton::ColourButton(wxWindow* parent, wxWindowID id, const wxSize& size, agi::Color col)
+#include <wx/dcmemory.h>
-: wxBitmapButton(parent, id, wxBitmap(size), wxDefaultPosition, wxSize(size.GetWidth() + 6, size.GetHeight() + 6))
+
-, bmp(GetBitmapLabel())
+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();
+	UpdateBitmap();
-	SetBitmapLabel(bmp);
+	Bind(wxEVT_COMMAND_BUTTON_CLICKED, [=](wxCommandEvent&) {
-	Bind(wxEVT_COMMAND_BUTTON_CLICKED, &ColourButton::OnClick, this);
+		GetColorFromUser<ColourButton, &ColourButton::SetColour>(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
+void ColourButton::SetColour(agi::Color new_color) {
-/// @param col New color
+	colour = new_color;
-void ColourButton::SetColour(agi::Color col) {
+	UpdateBitmap();
-	colour = col;
 
-	// Draw colour
+	wxThreadEvent evt(EVT_COLOR, GetId());
-	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);
 	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<ColourButton, &ColourButton::SetColour>(GetParent(), colour, this);
-	}
-}

aegisub/src/colour_button.h

@@ -1,51 +1,45 @@
-// Copyright (c) 2007, Rodrigo Braz Monteiro
+// Copyright (c) 2013, Thomas Goyne <plorkyeran@aegisub.org>
-// All rights reserved.
 //
-// Redistribution and use in source and binary forms, with or without
+// Permission to use, copy, modify, and distribute this software for any
-// modification, are permitted provided that the following conditions are met:
+// 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,
+// THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
-//     this list of conditions and the following disclaimer.
+// WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
-//   * Redistributions in binary form must reproduce the above copyright notice,
+// MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
-//     this list of conditions and the following disclaimer in the documentation
+// ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
-//     and/or other materials provided with the distribution.
+// WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
-//   * Neither the name of the Aegisub Group nor the names of its contributors
+// ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
-//     may be used to endorse or promote products derived from this software
+// OR IN CONNECTION WITH THE USE OR PERFORMANCE OF 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.
 //
 // Aegisub Project http://www.aegisub.org/
 
-/// @file colour_button.h
+#include <wx/button.h>
-/// @see colour_button.cpp
-/// @ingroup custom_control
-///
-
-#include <wx/bmpbuttn.h>
 
 #include <libaegisub/color.h>
 
-class ColourButton: public wxBitmapButton {
+/// Emitted by ColourButton when the user picks a new color, with the chosen
-	wxBitmap bmp; /// The button's bitmap label
+/// color set to the event payload
-	agi::Color colour; /// The current colour
+wxDECLARE_EVENT(EVT_COLOR, wxThreadEvent);
 
-	void Paint();
+/// A button which displays a currently-selected color and lets the user pick
-	void OnClick(wxCommandEvent &event);
+/// 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; }
 };

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_btn->Bind(EVT_COLOR, [=](wxThreadEvent& e) { color = color_btn->GetColor(); });
-		color = color_btn->GetColor();
-		e.Skip();
-	});
 	Bind(wxEVT_COMMAND_SPINCTRL_UPDATED, [=](wxCommandEvent&) {
 		TransferDataFromWindow();
 		UpdateLengthDisplay();

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[0] = new ColourButton(this, wxSize(55, 16), style->primary);
-	colorButton[1] = new ColourButton(this, -1, wxSize(55, 16), style->secondary);
+	colorButton[1] = new ColourButton(this, wxSize(55, 16), style->secondary);
-	colorButton[2] = new ColourButton(this, -1, wxSize(55, 16), style->outline);
+	colorButton[2] = new ColourButton(this, wxSize(55, 16), style->outline);
-	colorButton[3] = new ColourButton(this, -1, wxSize(55, 16), style->shadow);
+	colorButton[3] = new ColourButton(this, 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);
 	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
+void DialogStyleEditor::OnSetColor(int n, wxThreadEvent& evt) {
-/// @param n Colour to set
-void DialogStyleEditor::OnSetColor(int n, wxCommandEvent& evt) {
-	ColourButton *btn = static_cast<ColourButton*>(evt.GetClientData());
-	if (!btn) {
-		evt.Skip();
-		return;
-	}
-
 	switch (n) {
-		case 1: work->primary = btn->GetColor(); break;
+		case 1: work->primary = evt.GetPayload<agi::Color>(); break;
-		case 2: work->secondary = btn->GetColor(); break;
+		case 2: work->secondary = evt.GetPayload<agi::Color>(); break;
-		case 3: work->outline = btn->GetColor(); break;
+		case 3: work->outline = evt.GetPayload<agi::Color>(); break;
-		case 4: work->shadow = btn->GetColor(); break;
+		case 4: work->shadow = evt.GetPayload<agi::Color>(); break;
 		default: throw agi::InternalError("attempted setting colour id outside range", 0);
 	}
 	if (SubsPreview)

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 = "");

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 {
+OPTION_UPDATER(ColourUpdater, wxThreadEvent, OptionValueColor, evt.GetPayload<agi::Color>());
-	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<ColourButton*>(evt.GetClientData());
-		if (btn)
-			parent->SetOption(new agi::OptionValueColor(name, btn->GetColor()));
-		evt.Skip();
-	}
-};
 
 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());
+			ColourButton *cb = new ColourButton(this, wxSize(40,10), opt->GetColor());
-			cb->Bind(wxEVT_COMMAND_BUTTON_CLICKED, ColourUpdater(opt_name, parent));
+			cb->Bind(EVT_COLOR, ColourUpdater(opt_name, parent));
 			Add(flex, name, cb);
 			return cb;
 		}