From 64dc6d24afba24900ba3899d9f8dfc0488db6d2d Mon Sep 17 00:00:00 2001 From: FurryFur Date: Tue, 19 Sep 2017 11:34:21 +1200 Subject: [PATCH 1/3] Fixed dangling pointer causing crash when widgets were removed by a mouse event. --- include/nanogui/screen.h | 2 +- src/screen.cpp | 16 ++++++++++++++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/include/nanogui/screen.h b/include/nanogui/screen.h index 709789b4da..0638b3e45b 100644 --- a/include/nanogui/screen.h +++ b/include/nanogui/screen.h @@ -195,7 +195,7 @@ class NANOGUI_EXPORT Screen : public Widget { int mMouseState, mModifiers; Vector2i mMousePos; bool mDragActive; - Widget *mDragWidget = nullptr; + ref mDragWidget; double mLastInteraction; bool mProcessEvents; Color mBackground; diff --git a/src/screen.cpp b/src/screen.cpp index 2a2634f07a..93b190336d 100644 --- a/src/screen.cpp +++ b/src/screen.cpp @@ -480,6 +480,12 @@ bool Screen::cursorPosCallbackEvent(double x, double y) { try { p -= Vector2i(1, 2); + // Make sure mDragWidget isn't the only remaining reference + if (mDragActive && mDragWidget->getRefCount() == 1) { + mDragActive = false; + mDragWidget = nullptr; + } + if (!mDragActive) { Widget *widget = findWidget(p); if (widget != nullptr && widget->cursor() != mCursor) { @@ -522,6 +528,12 @@ bool Screen::mouseButtonCallbackEvent(int button, int action, int modifiers) { else mMouseState &= ~(1 << button); + // Make sure mDragWidget isn't the only remaining reference + if (mDragActive && mDragWidget->getRefCount() == 1) { + mDragActive = false; + mDragWidget = nullptr; + } + auto dropWidget = findWidget(mMousePos); if (mDragActive && action == GLFW_RELEASE && dropWidget != mDragWidget) @@ -536,7 +548,7 @@ bool Screen::mouseButtonCallbackEvent(int button, int action, int modifiers) { if (action == GLFW_PRESS && (button == GLFW_MOUSE_BUTTON_1 || button == GLFW_MOUSE_BUTTON_2)) { mDragWidget = findWidget(mMousePos); - if (mDragWidget == this) + if (mDragWidget.get() == this) mDragWidget = nullptr; mDragActive = mDragWidget != nullptr; if (!mDragActive) @@ -649,7 +661,7 @@ void Screen::updateFocus(Widget *widget) { void Screen::disposeWindow(Window *window) { if (std::find(mFocusPath.begin(), mFocusPath.end(), window) != mFocusPath.end()) mFocusPath.clear(); - if (mDragWidget == window) + if (mDragWidget.get() == window) mDragWidget = nullptr; removeChild(window); } From f0a6f00c38dac6c7e2f0d6e4c4f0f7e6a6a81f9b Mon Sep 17 00:00:00 2001 From: FurryFur Date: Tue, 19 Sep 2017 12:32:44 +1200 Subject: [PATCH 2/3] Fixed dangling reference caused by deleted widgets having focus. --- src/widget.cpp | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/widget.cpp b/src/widget.cpp index 90b2461818..ef4d65a93b 100644 --- a/src/widget.cpp +++ b/src/widget.cpp @@ -152,14 +152,21 @@ void Widget::addChild(Widget * widget) { } void Widget::removeChild(const Widget *widget) { - mChildren.erase(std::remove(mChildren.begin(), mChildren.end(), widget), mChildren.end()); - widget->decRef(); + auto it = std::find(mChildren.begin(), mChildren.end(), widget); + if (it == mChildren.end()) { + return; + } + removeChild(it - mChildren.begin()); } void Widget::removeChild(int index) { - Widget *widget = mChildren[index]; - mChildren.erase(mChildren.begin() + index); - widget->decRef(); + Widget *widget = mChildren[index]; + if (widget->focused()) { + requestFocus(); + } + + mChildren.erase(mChildren.begin() + index); + widget->decRef(); } int Widget::childIndex(Widget *widget) const { From 22783e67ec12a8ad89f83b939e9c7f3730f864fc Mon Sep 17 00:00:00 2001 From: FurryFur Date: Wed, 20 Sep 2017 09:58:41 +1200 Subject: [PATCH 3/3] Removed tabs --- src/widget.cpp | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/widget.cpp b/src/widget.cpp index ef4d65a93b..db81808ad4 100644 --- a/src/widget.cpp +++ b/src/widget.cpp @@ -152,21 +152,21 @@ void Widget::addChild(Widget * widget) { } void Widget::removeChild(const Widget *widget) { - auto it = std::find(mChildren.begin(), mChildren.end(), widget); - if (it == mChildren.end()) { - return; - } - removeChild(it - mChildren.begin()); + auto it = std::find(mChildren.begin(), mChildren.end(), widget); + if (it == mChildren.end()) { + return; + } + removeChild(it - mChildren.begin()); } void Widget::removeChild(int index) { - Widget *widget = mChildren[index]; - if (widget->focused()) { - requestFocus(); - } + Widget *widget = mChildren[index]; + if (widget->focused()) { + requestFocus(); + } - mChildren.erase(mChildren.begin() + index); - widget->decRef(); + mChildren.erase(mChildren.begin() + index); + widget->decRef(); } int Widget::childIndex(Widget *widget) const {