Fix crashes resetting the default layout + remove some hacks

With this patch we've also simplified some hacks handling the
populateComboBox() call, deferring the deletion of widgets/items when
we are inside an event generated by those items.
This commit is contained in:
David Capello 2025-05-07 23:34:05 -03:00
parent dcbf444aaf
commit 763ebba6a2
3 changed files with 43 additions and 25 deletions

View File

@ -105,13 +105,12 @@ public:
, m_selector(selector) , m_selector(selector)
, m_layoutId(layoutId) , m_layoutId(layoutId)
{ {
auto* hbox = new HBox; m_hbox.setTransparent(true);
hbox->setTransparent(true); addChild(&m_hbox);
addChild(hbox);
auto* filler = new BoxFiller(); auto* filler = new BoxFiller();
filler->setTransparent(true); filler->setTransparent(true);
hbox->addChild(filler); m_hbox.addChild(filler);
if (option == USER_DEFINED || if (option == USER_DEFINED ||
((option == DEFAULT || option == MIRRORED_DEFAULT) && !layoutId.empty())) { ((option == DEFAULT || option == MIRRORED_DEFAULT) && !layoutId.empty())) {
@ -128,13 +127,10 @@ public:
ASSERT(!m_layoutId.empty()); ASSERT(!m_layoutId.empty());
// TODO: Custom icons for each one would be nice here. // TODO: Custom icons for each one would be nice here.
m_actionButton = std::unique_ptr<IconButton>( m_actionButton = new IconButton(SkinTheme::instance()->parts.iconClose());
new IconButton(SkinTheme::instance()->parts.iconClose())); const int th = m_actionButton->textHeight();
m_actionButton->setSizeHint( m_actionButton->setSizeHint(gfx::Size(th, th));
gfx::Size(m_actionButton->textHeight(), m_actionButton->textHeight()));
m_actionButton->setTransparent(true); m_actionButton->setTransparent(true);
m_actionButton->InitTheme.connect( m_actionButton->InitTheme.connect(
[this] { m_actionButton->setBgColor(gfx::rgba(0, 0, 0, 0)); }); [this] { m_actionButton->setBgColor(gfx::rgba(0, 0, 0, 0)); });
@ -177,7 +173,7 @@ public:
}); });
} }
children()[0]->addChild(m_actionButton.get()); m_hbox.addChild(m_actionButton);
} }
std::string_view getLayoutId() const { return m_layoutId; } std::string_view getLayoutId() const { return m_layoutId; }
@ -298,15 +294,20 @@ public:
private: private:
LayoutOption m_option; LayoutOption m_option;
LayoutSelector* m_selector; LayoutSelector* m_selector = nullptr;
std::string m_layoutId; std::string m_layoutId;
std::unique_ptr<IconButton> m_actionButton; HBox m_hbox;
IconButton* m_actionButton = nullptr;
obs::scoped_connection m_actionConn; obs::scoped_connection m_actionConn;
}; };
void LayoutSelector::LayoutComboBox::onChange() void LayoutSelector::LayoutComboBox::onChange()
{ {
ComboBox::onChange(); ComboBox::onChange();
if (m_lockChange)
return;
if (auto* item = dynamic_cast<LayoutItem*>(getSelectedItem())) { if (auto* item = dynamic_cast<LayoutItem*>(getSelectedItem())) {
item->selectImmediately(); item->selectImmediately();
m_selected = item; m_selected = item;
@ -316,6 +317,10 @@ void LayoutSelector::LayoutComboBox::onChange()
void LayoutSelector::LayoutComboBox::onCloseListBox() void LayoutSelector::LayoutComboBox::onCloseListBox()
{ {
ComboBox::onCloseListBox(); ComboBox::onCloseListBox();
if (m_lockChange)
return;
if (m_selected) { if (m_selected) {
m_selected->selectAfterClose(); m_selected->selectAfterClose();
m_selected = nullptr; m_selected = nullptr;
@ -363,12 +368,7 @@ void LayoutSelector::addLayout(const LayoutPtr& layout)
{ {
m_layouts.addLayout(layout); m_layouts.addLayout(layout);
// HACK: Because this function is called from inside a LayoutItem, clearing the combobox items populateComboBox();
// will crash.
// TODO: Is there a better way to do this?
auto* msg = new CallbackMessage([this] { populateComboBox(); });
msg->setRecipient(this);
manager()->enqueueMessage(msg);
} }
void LayoutSelector::removeLayout(const LayoutPtr& layout) void LayoutSelector::removeLayout(const LayoutPtr& layout)
@ -376,10 +376,7 @@ void LayoutSelector::removeLayout(const LayoutPtr& layout)
m_layouts.removeLayout(layout); m_layouts.removeLayout(layout);
m_layouts.saveUserLayouts(); m_layouts.saveUserLayouts();
// TODO: See addLayout populateComboBox();
auto* msg = new CallbackMessage([this] { populateComboBox(); });
msg->setRecipient(this);
manager()->enqueueMessage(msg);
} }
void LayoutSelector::removeLayout(const std::string& layoutId) void LayoutSelector::removeLayout(const std::string& layoutId)
@ -523,7 +520,21 @@ void LayoutSelector::setActiveLayoutId(const std::string& layoutId)
void LayoutSelector::populateComboBox() void LayoutSelector::populateComboBox()
{ {
m_comboBox.deleteAllItems(); // Disable combobox onChange() event processing when we are
// re-creating the combobox items. This avoids calling
// LayoutSelector::LayoutItem::selectImmediately() which could
// delete docks that generate this same event, e.g. resizing a dock
// can generate a UserResizedDock which might call this
// populateComboBox() function.
m_comboBox.setLockChange(true);
// Defer deletion of current items because we can be inside one of
// these item callbacks.
auto itemsCopy = m_comboBox.items();
for (auto* item : itemsCopy) {
m_comboBox.removeItem(item);
item->deferDelete();
}
m_comboBox.addItem(new SeparatorInView(Strings::main_window_layout(), HORIZONTAL)); m_comboBox.addItem(new SeparatorInView(Strings::main_window_layout(), HORIZONTAL));
m_comboBox.addItem( m_comboBox.addItem(
@ -558,6 +569,8 @@ void LayoutSelector::populateComboBox()
m_comboBox.setSelectedItemIndex(2); m_comboBox.setSelectedItemIndex(2);
m_comboBox.getEntryWidget()->deselectText(); m_comboBox.getEntryWidget()->deselectText();
m_comboBox.setLockChange(false);
} }
LayoutSelector::LayoutItem* LayoutSelector::getItemByLayoutId(const std::string& id) LayoutSelector::LayoutItem* LayoutSelector::getItemByLayoutId(const std::string& id)

View File

@ -37,10 +37,14 @@ class LayoutSelector : public ui::VBox,
class LayoutItem; class LayoutItem;
class LayoutComboBox : public ui::ComboBox { class LayoutComboBox : public ui::ComboBox {
public:
void setLockChange(bool state) { m_lockChange = state; }
private: private:
void onChange() override; void onChange() override;
void onCloseListBox() override; void onCloseListBox() override;
LayoutItem* m_selected = nullptr; LayoutItem* m_selected = nullptr;
bool m_lockChange = false;
}; };
public: public:

View File

@ -1,5 +1,5 @@
// Aseprite UI Library // Aseprite UI Library
// Copyright (C) 2019-2023 Igara Studio S.A. // Copyright (C) 2019-2025 Igara Studio S.A.
// Copyright (C) 2001-2017 David Capello // Copyright (C) 2001-2017 David Capello
// //
// This file is released under the terms of the MIT license. // This file is released under the terms of the MIT license.
@ -39,6 +39,7 @@ public:
Items::iterator begin() { return m_items.begin(); } Items::iterator begin() { return m_items.begin(); }
Items::iterator end() { return m_items.end(); } Items::iterator end() { return m_items.end(); }
bool empty() const { return m_items.empty(); } bool empty() const { return m_items.empty(); }
const Items& items() { return m_items; }
void setEditable(bool state); void setEditable(bool state);
void setClickOpen(bool state); void setClickOpen(bool state);