wjakob / nanogui

Minimalistic GUI library for OpenGL
Other
4.66k stars 608 forks source link

Bug report: remove widget in callback #331

Open jzrake opened 6 years ago

jzrake commented 6 years ago

Attempting to remove a button from its parent on click results in a seg fault:

class TestApp : public nanogui::Screen {
public:
    TestApp() : nanogui::Screen(Vector2i(400, 400), "") {
        auto b = new Button(this, "Click to remove");
        b->setCallback([this] () { removeChild(0); });
        performLayout();
    }
};
class TestApp(nanogui.Screen):
    def __init__(self):
        super(TestApp, self).__init__((400, 400), "")
        nanogui.Button(self, "Click to remove me").setCallback(lambda: self.removeChild(0))
        self.performLayout()

This appears to be a bug, because according to button.cpp, the button should be retained until after any callbacks are finished:

    /* button.cpp, line 52
       Temporarily increase the reference count of the button in case the
       button causes the parent window to be destructed */
    ref<Button> self = this;
svenevs commented 6 years ago

You should capture b and remove it explicitly:

b->setCallback([this, b]() { removeChild(b); });

For python you'll need to be more explicit. Untested, but something like this should work

b = nanogui.Button(self, "Click to remove me")
b.setCallback(lambda: self.removeChild(b))

The implementation differences are here:

https://github.com/wjakob/nanogui/blob/885e4fccc69bbfdd4c527009eef8ed33641d9765/src/widget.cpp#L154-L163

Basically, you should prefer the explicit Widget * version. IIRC the callback is executed twice (put some print statements in to confirm), once for push and once for release? If so, then the second one is where the segfault comes from, because it was already removed in the first execution.

svenevs commented 6 years ago

UhOh. I just added that and tested, and if you do the explicit capture of b then you'll get a segfault later. First click removes it, but then if you click in the same spot you'll get a segfault...