Skip to content

Commit

Permalink
[UI] Fix ClearInput not called in ImGuiDrawer after deferred dialog r…
Browse files Browse the repository at this point in the history
…emoval

Also cleanup the code involved in dialog registration, and update the explanation of why dialog removal is delayed until the end of drawing (the original was written back when window listener and UI drawer callback registration during the execution of the callbacks was deferred, but that was wrong as that might result in execution of callbacks belonging to now-deleted objects).
  • Loading branch information
Triang3l committed Oct 31, 2022
1 parent a37b57c commit 778333b
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 23 deletions.
55 changes: 32 additions & 23 deletions src/xenia/ui/imgui_drawer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,11 @@ void ImGuiDrawer::AddDialog(ImGuiDialog* dialog) {
dialogs_.cend()) {
return;
}
if (dialog_loop_next_index_ == SIZE_MAX && dialogs_.empty()) {
// First dialog added. dialog_loop_next_index_ == SIZE_MAX is also checked
// because in a situation of removing the only dialog, then adding a dialog,
// from within a dialog's Draw function, the removal would not cause the
// listener and the drawer to be removed (it's deferred in this case).
if (dialogs_.empty() && !IsDrawingDialogs()) {
// First dialog added. !IsDrawingDialogs() is also checked because in a
// situation of removing the only dialog, then adding a dialog, from within
// a dialog's Draw function, re-registering the ImGuiDrawer may result in
// ImGui being drawn multiple times in the current frame.
window_->AddInputListener(this, z_order_);
if (presenter_) {
presenter_->AddUIDrawerFromUIThread(this, z_order_);
Expand All @@ -81,25 +81,15 @@ void ImGuiDrawer::RemoveDialog(ImGuiDialog* dialog) {
if (it == dialogs_.cend()) {
return;
}
if (dialog_loop_next_index_ != SIZE_MAX) {
if (IsDrawingDialogs()) {
// Actualize the next dialog index after the erasure from the vector.
size_t existing_index = size_t(std::distance(dialogs_.cbegin(), it));
if (dialog_loop_next_index_ > existing_index) {
--dialog_loop_next_index_;
}
}
dialogs_.erase(it);
if (dialog_loop_next_index_ == SIZE_MAX && dialogs_.empty()) {
if (presenter_) {
presenter_->RemoveUIDrawerFromUIThread(this);
}
window_->RemoveInputListener(this);
// Clear all input since no input will be received anymore, and when the
// drawer becomes active again, it'd have an outdated input state otherwise
// which will be persistent until new events actualize individual input
// properties.
ClearInput();
}
DetachIfLastDialogRemoved();
}

void ImGuiDrawer::Initialize() {
Expand Down Expand Up @@ -301,7 +291,7 @@ void ImGuiDrawer::Draw(UIDrawContext& ui_draw_context) {

ImGui::NewFrame();

assert_true(dialog_loop_next_index_ == SIZE_MAX);
assert_true(!IsDrawingDialogs());
dialog_loop_next_index_ = 0;
while (dialog_loop_next_index_ < dialogs_.size()) {
dialogs_[dialog_loop_next_index_++]->Draw();
Expand All @@ -319,11 +309,11 @@ void ImGuiDrawer::Draw(UIDrawContext& ui_draw_context) {
io.MousePos = ImVec2(-FLT_MAX, -FLT_MAX);
}

if (dialogs_.empty()) {
// All dialogs have removed themselves during the draw, detach.
presenter_->RemoveUIDrawerFromUIThread(this);
window_->RemoveInputListener(this);
} else {
// Detaching is deferred if the last dialog is removed during drawing, perform
// it now if needed.
DetachIfLastDialogRemoved();

if (!dialogs_.empty()) {
// Repaint (and handle input) continuously if still active.
presenter_->RequestUIPaintFromUIThread();
}
Expand Down Expand Up @@ -557,5 +547,24 @@ void ImGuiDrawer::SwitchToPhysicalMouseAndUpdateMousePosition(
UpdateMousePosition(float(e.x()), float(e.y()));
}

void ImGuiDrawer::DetachIfLastDialogRemoved() {
// IsDrawingDialogs() is also checked because in a situation of removing the
// only dialog, then adding a dialog, from within a dialog's Draw function,
// re-registering the ImGuiDrawer may result in ImGui being drawn multiple
// times in the current frame.
if (!dialogs_.empty() || IsDrawingDialogs()) {
return;
}
if (presenter_) {
presenter_->RemoveUIDrawerFromUIThread(this);
}
window_->RemoveInputListener(this);
// Clear all input since no input will be received anymore, and when the
// drawer becomes active again, it'd have an outdated input state otherwise
// which will be persistent until new events actualize individual input
// properties.
ClearInput();
}

} // namespace ui
} // namespace xe
3 changes: 3 additions & 0 deletions src/xenia/ui/imgui_drawer.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,9 @@ class ImGuiDrawer : public WindowInputListener, public UIDrawer {
void UpdateMousePosition(float x, float y);
void SwitchToPhysicalMouseAndUpdateMousePosition(const MouseEvent& e);

bool IsDrawingDialogs() const { return dialog_loop_next_index_ != SIZE_MAX; }
void DetachIfLastDialogRemoved();

Window* window_;
size_t z_order_;

Expand Down

0 comments on commit 778333b

Please sign in to comment.