mobius3 / kiwi

KiWi: Killer Widgets
zlib License
186 stars 19 forks source link

How to properly remove a widget from gui #24

Open martinsik opened 6 years ago

martinsik commented 6 years ago

This is a little followup to #23. I'm wondering how can I remove a widget from its parent (completely destroy the widget). From the comments in the code I suppose I should be using the following to destroy eg. button with label:

KW_DestroyWidget(widget, 1);

However this sometimes throws an error ("Segmentation fault") as can be seen the following GIFs. It seems to be random, sometimes it fails, sometimes it's fine.

By clicking any of the buttons I want to remove them with KW_DestroyWidget.

kapture 2017-11-21 at 9 24 28

... or here

kapture 2017-11-21 at 10 22 16

... or here

kapture 2017-11-21 at 10 22 55

The error backtrace is always the same:

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   libKiWi.dylib                   0x000000010b77d5be CalculateMouseOver + 30 (KW_eventwatcher.c:15)
1   libKiWi.dylib                   0x000000010b77db0d MouseReleased + 253 (KW_eventwatcher.c:104)
2   libKiWi.dylib                   0x000000010b77deff KW_ProcessEvents + 367 (KW_eventwatcher.c:151)
3   test-widget-removal             0x000000010b653d2a main + 922 (test-widget-removal.c:66)
4   libdyld.dylib                   0x00007fff9f302235 start + 1

Test source code:

#include "SDL.h"
#include "KW_gui.h"
#include "KW_button.h"
#include "KW_scrollbox.h"
#include "KW_renderdriver_sdl2.h"

void MouseDown(KW_Widget * widget, int button) {
    KW_DestroyWidget(widget, 1);
}

int main(int argc, char ** argv) {
    SDL_Window * window;
    SDL_Renderer * renderer;
    KW_RenderDriver * driver;
    KW_Surface * set;
    KW_GUI * gui;
    KW_Font * font;
    KW_Rect geometry = {0, 0, 480, 320};
    KW_Widget * frame, * button;
    int i;
    SDL_Event ev;
    size_t len = 0;

    /* initialize SDL */
    SDL_Init(SDL_INIT_EVERYTHING);
    SDL_CreateWindowAndRenderer(geometry.w, geometry.h, SDL_WINDOW_RESIZABLE, &window, &renderer);
    SDL_SetRenderDrawColor(renderer, 100, 200, 100, 1);
    driver = KW_CreateSDL2RenderDriver(renderer, window);
    set = KW_LoadSurface(driver, "tileset.png");

    /* initialize gui */
    gui = KW_Init(driver, set);
    font = KW_LoadFont(driver, "SourceSansPro-Semibold.ttf", 12);
    KW_SetFont(gui, font);

    geometry.x = (unsigned)(geometry.w * 0.0625f);
    geometry.y = (unsigned)(geometry.h * .0625f);
    geometry.w *= .875f;
    geometry.h *= .875;
    frame = KW_CreateScrollbox(gui, NULL, &geometry);

    geometry.x = 10; geometry.y = 0; geometry.h = 40; geometry.w = 230;

    for (i = 0; i < 10; i++) {
        button = KW_CreateButtonAndLabel(gui, frame, "Text label", &geometry);
        KW_AddWidgetMouseDownHandler(button, MouseDown);

        geometry.y += geometry.h;
    }

    /* create another parent frame */
    while (!SDL_QuitRequested()) {
        while (SDL_PollEvent(&ev)) {
            if (ev.type == SDL_WINDOWEVENT && ev.window.event == SDL_WINDOWEVENT_SIZE_CHANGED) {
                geometry.w = (unsigned)ev.window.data1;
                geometry.h = (unsigned)ev.window.data2;
                geometry.x = (unsigned)(geometry.w * 0.0625);
                geometry.y = (unsigned)(geometry.h * .0625);
                geometry.w *= .875f;
                geometry.h *= .875;
                KW_SetWidgetGeometry(frame, &geometry);
            }
        }
        SDL_RenderClear(renderer);
        KW_ProcessEvents(gui);
        KW_Paint(gui);
        SDL_RenderPresent(renderer);
        SDL_Delay(1);
    }
    KW_ReleaseFont(driver, font);
    KW_Quit(gui);
    KW_ReleaseSurface(driver, set);
    SDL_Quit();

    return 0;
}
mobius3 commented 6 years ago

I suppose removing a widget inside its event callback function breaks mouse over iteration by changing its parrent children widget array. Could you test the following, to confirm: store widget in a temporary variable and call KW_DestroyWidget in your main loop instead of calling it directly in the click callback.

martinsik commented 6 years ago

I tried this and it seems to be fine. I haven't seen any crash after few tries:

// ... the rest of the code
    int counter = 0;
    KW_Widget *tmp_widgets[10];

    for (i = 0; i < 10; i++) {
        button = KW_CreateButtonAndLabel(gui, frame, "Text label", &geometry);
        KW_AddWidgetMouseDownHandler(button, MouseDown);

        geometry.y += geometry.h;

        tmp_widgets[i] = button;
    }

    /* create another parent frame */
    while (!SDL_QuitRequested()) {
        while (SDL_PollEvent(&ev)) {
            if (ev.type == SDL_WINDOWEVENT && ev.window.event == SDL_WINDOWEVENT_SIZE_CHANGED) {
                geometry.w = (unsigned)ev.window.data1;
                geometry.h = (unsigned)ev.window.data2;
                geometry.x = (unsigned)(geometry.w * 0.0625);
                geometry.y = (unsigned)(geometry.h * .0625);
                geometry.w *= .875f;
                geometry.h *= .875;
                KW_SetWidgetGeometry(frame, &geometry);
            }
        }
        SDL_RenderClear(renderer);

        if (++counter == 300) {
            for (i = 0; i < 10; i++) {
                KW_DestroyWidget(tmp_widgets[i], 1);
            }
        }

        KW_ProcessEvents(gui);
        KW_Paint(gui);
        SDL_RenderPresent(renderer);
        SDL_Delay(1);
    }
// ... the rest of the code

The widgets disappear and the scrollarea is properly resized.

kapture 2017-11-21 at 13 35 14

mobius3 commented 6 years ago

Great. It kind of confirms what I was thinking. Widget destruction must be postponed until event processing finishes. I'll find some time to work on this. Thanks for your report!