Skip to content

fix Mac teardown crashes by correcting QObject ownership hierarchy #526

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
nschimme merged 1 commit into from
Apr 20, 2026
Merged

fix Mac teardown crashes by correcting QObject ownership hierarchy #526

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 17 additions & 19 deletions src/group/groupwidget.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -829,14 +829,7 @@ GroupWidget::GroupWidget(Mmapper2Group *const group, MapData *const md, QWidget
: QWidget(parent)
, m_group(group)
, m_map(md)
, m_model(this)
{
if (m_group) {
m_model.setCharacters(m_group->selectAll());
} else {
m_model.setCharacters({});
}

auto *layout = new QVBoxLayout(this);
layout->setAlignment(Qt::AlignTop);
layout->setContentsMargins(0, 0, 0, 0);
Expand All @@ -849,8 +842,15 @@ GroupWidget::GroupWidget(Mmapper2Group *const group, MapData *const md, QWidget
m_table->horizontalHeader()->setStretchLastSection(true);
m_table->horizontalHeader()->setSectionResizeMode(QHeaderView::ResizeToContents);

m_proxyModel = new GroupProxyModel(this);
m_proxyModel->setSourceModel(&m_model);
m_model = new GroupModel(m_table);
if (m_group) {
m_model->setCharacters(m_group->selectAll());
} else {
m_model->setCharacters({});
}

m_proxyModel = new GroupProxyModel(m_table);
m_proxyModel->setSourceModel(m_model);
m_table->setModel(m_proxyModel);

m_table->setDragEnabled(true);
Expand All @@ -859,7 +859,7 @@ GroupWidget::GroupWidget(Mmapper2Group *const group, MapData *const md, QWidget
m_table->setDefaultDropAction(Qt::MoveAction);
m_table->setDropIndicatorShown(true);

m_table->setItemDelegate(new GroupDelegate(this));
m_table->setItemDelegate(new GroupDelegate(m_table));
layout->addWidget(m_table);

m_pulseTimer = new QTimer(this);
Expand Down Expand Up @@ -912,7 +912,7 @@ GroupWidget::GroupWidget(Mmapper2Group *const group, MapData *const md, QWidget
return;
}

selectedCharacter = m_model.getCharacter(sourceIndex.row());
selectedCharacter = deref(m_model).getCharacter(sourceIndex.row());
if (selectedCharacter) {
// Build Context menu
m_center->setText(
Expand Down Expand Up @@ -947,8 +947,6 @@ GroupWidget::GroupWidget(Mmapper2Group *const group, MapData *const md, QWidget
GroupWidget::~GroupWidget()
{
m_pulseTimer->stop();
delete m_table;
delete m_recolor;
}

QSize GroupWidget::sizeHint() const
Expand All @@ -964,7 +962,7 @@ void GroupWidget::updateColumnVisibility()
{
// Hide unnecessary columns like mana if everyone is a zorc/troll
const auto one_character_had_mana = [this]() -> bool {
for (const auto &character : m_model.getCharacters()) {
for (const auto &character : deref(m_model).getCharacters()) {
if (character && (character->getMana() > 0 || character->getMaxMana() > 0)) {
return true;
}
Expand All @@ -978,7 +976,7 @@ void GroupWidget::updateColumnVisibility()
void GroupWidget::updatePulseTimer()
{
const auto needs_pulse = [this]() -> bool {
for (const auto &character : m_model.getCharacters()) {
for (const auto &character : deref(m_model).getCharacters()) {
if (!character) {
continue;
}
Expand Down Expand Up @@ -1012,29 +1010,29 @@ void GroupWidget::updatePulseTimer()
void GroupWidget::slot_onCharacterAdded(SharedGroupChar character)
{
assert(character);
m_model.insertCharacter(character);
deref(m_model).insertCharacter(character);
updateColumnVisibility();
updatePulseTimer();
}

void GroupWidget::slot_onCharacterRemoved(const GroupId characterId)
{
assert(characterId != INVALID_GROUPID);
m_model.removeCharacterById(characterId);
deref(m_model).removeCharacterById(characterId);
updateColumnVisibility();
updatePulseTimer();
}

void GroupWidget::slot_onCharacterUpdated(SharedGroupChar character)
{
assert(character);
m_model.updateCharacter(character);
deref(m_model).updateCharacter(character);
updatePulseTimer();
}

void GroupWidget::slot_onGroupReset(const GroupVector &newCharacterList)
{
m_model.setCharacters(newCharacterList);
deref(m_model).setCharacters(newCharacterList);
updateColumnVisibility();
updatePulseTimer();
}
6 changes: 3 additions & 3 deletions src/group/groupwidget.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ class NODISCARD_QOBJECT GroupWidget final : public QWidget
Mmapper2Group *m_group = nullptr;
MapData *m_map = nullptr;
GroupProxyModel *m_proxyModel = nullptr;
GroupModel m_model;
GroupModel *m_model = nullptr;
QTimer *m_pulseTimer = nullptr;

void updateColumnVisibility();
Expand All @@ -174,8 +174,8 @@ class NODISCARD_QOBJECT GroupWidget final : public QWidget
void sig_center(glm::vec2);

public slots:
void slot_mapUnloaded() { m_model.setMapLoaded(false); }
void slot_mapLoaded() { m_model.setMapLoaded(true); }
void slot_mapUnloaded() { deref(m_model).setMapLoaded(false); }
void slot_mapLoaded() { deref(m_model).setMapLoaded(true); }

private slots:
void slot_onCharacterAdded(SharedGroupChar character);
Expand Down
4 changes: 2 additions & 2 deletions src/timers/TimerWidget.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ TimerWidget::TimerWidget(CTimers &timers, QWidget *parent)
layout->setContentsMargins(0, 0, 0, 0);
layout->setSpacing(0);

m_model = new TimerModel(m_timers, this);
m_view = new QTableView(this);
m_model = new TimerModel(m_timers, m_view);
m_view->setModel(m_model);
auto *delegate = new TimerDelegate(this);
auto *delegate = new TimerDelegate(m_view);
for (int i = 0; i < TimerModel::ColCount; ++i) {
m_view->setItemDelegateForColumn(i, delegate);
}
Expand Down
Loading