frang75 / nappgui_src

SDK for building cross-platform desktop apps in ANSI-C
https://www.nappgui.com
MIT License
506 stars 50 forks source link

Unexpected Asserts on SplitView #33

Closed tobiasford closed 1 year ago

tobiasford commented 1 year ago

This library looks / seems extremely promising... It's very clean and it appears to be very well thought out.

Right now, I'm experimenting with the API to see how well it will work with some rendering related tools. So far, it's looking promising but I've ran into a couple of issues that are triggering unexpected asserts. My test code is below:

MainWindow::MainWindow(Application * application)
{
    auto describePanel_Color = [this](std::string const & title, Pixel::packedRGBA const & color)
    {
        Label * titleLabel = label_create();
        label_text(titleLabel, title.c_str());

        Label * redLabel = label_create();
            label_text(redLabel, "Red:");
        Slider * redSlider = slider_create();
            slider_value(redSlider, .5f);
        //  slider_OnMoved(redSlider, listener(app, i_OnAngle, App));

        Label * greenLabel = label_create();
            label_text(greenLabel, "Green:");
        Slider * greenSlider = slider_create();
            slider_value(greenSlider, .5f);
        //  slider_OnMoved(greenSlider, listener(app, i_OnScale, App));

        Label * blueLabel = label_create();
            label_text(blueLabel, "Blue:");
        Slider * blueSlider = slider_create();
            slider_value(blueSlider, .5f);
        //  slider_OnMoved(blueSlider, listener(app, i_OnScale, App));

        Layout * layout = layout_create(5, 4); // <--- why does ncols need to be >= 5. I'd expect 2 to be valid
            layout_label(layout,  titleLabel,  0, 0);
            layout_label(layout,  redLabel,    0, 1);
            layout_slider(layout, redSlider,   1, 1);
            layout_label(layout,  greenLabel,  0, 2);
            layout_slider(layout, greenSlider, 1, 2);
            layout_label(layout,  blueLabel,   0, 3);
            layout_slider(layout, blueSlider,  1, 3);

            layout_halign(layout, 0, 1, ekRIGHT);
            layout_halign(layout, 0, 2, ekRIGHT);
            layout_halign(layout, 0, 3, ekRIGHT);

            layout_hmargin(layout, 0, 10);
            layout_hmargin(layout, 1, 10);

        Panel * panel = panel_create();
            panel_layout(panel, layout);

        return panel;
    };

    auto describePanel_Main = [&]()
    {
        Label * label = label_create();
            label_text(label, "Hello!, I'm a label");

        Button * button = button_push();
            button_text(button, "Click Me!");
            button_OnClick(button, IListen(this, MainWindow, onButton));

        m_textView = textview_create();

        m_view = view_create();
            view_size(m_view, s2df(1024, 768));
            view_OnDraw(m_view, IListen(this, MainWindow, onDraw));

        Panel * panelColor0 = describePanel_Color("color0", m_color0);
        Panel * panelColor1 = describePanel_Color("color1", m_color1);

        // attempting to chain together multiple SplitView(s) to
        // create 3 resizable columns                       <------
        //  ||XXXXXX||XXXXXX||XXXXXX||
        //  ||XXXXXX||XXXXXX||XXXXXX||
        //  ||XXXXXX||XXXXXX||XXXXXX||
        //  ||XXXXXX||XXXXXX||XXXXXX||

        SplitView * split1 = splitview_vertical();
            splitview_view( split1,  m_view);
            splitview_panel(split1, panelColor1);
            splitview_size(split1, s2df(800, 480));
            splitview_pos( split1, 0.25f);

        SplitView * split0 = splitview_vertical();
            splitview_panel(split0, panelColor0);
            splitview_split(split0, split1);
            splitview_size(split0, s2df(400, 480));
            splitview_pos( split0, 0.5f);

        Layout * layout = layout_create(1, 1);
            layout_splitview(layout, split0, 0, 0);

        Panel * panel = panel_create();
            panel_layout(panel, layout);

        return panel;
    };

    m_window = window_create(ekWINDOW_STDRES);
        window_panel(m_window, describePanel_Main());
        window_title(m_window, "Hello, C++!");
        window_origin(m_window, v2df(500, 200));
        window_OnClose(m_window, IListen(application, Application, onClose));
        window_show(m_window);
}

the first issue is in describePanel_Color where I'm expecting to be able to use layout_create(2, 4) but instead anything with columns less than 5 causes an assert.

the second issue is when attempting to chain multiple horizontal SplitView(s) together to create 3 resizable columns (data | view/texture | data). Can SplitViews even be combined in this way?

Thanks!

lbhack commented 1 year ago

hi @tobiasford The issue you are running into is that the layout_create function expects the number of columns to be greater than or equal to 5. This may be a requirement of the library, as it seems to be using the additional columns for layout management. Additionally, you are trying to chain together multiple SplitViews to create 3 resizable columns.

frang75 commented 1 year ago

Hi @tobiasford ! Thank you very much for use NAppGUI and the feedback.

The problems is in: layout_hmargin(layout, 1, 10); https://nappgui.com/en/gui/layout.html#f39

Establish an inter-column margin within the layout. It is the separation between two consecutive columns. Index of the column. The index 0 refers to the separation between column 0 and column 1. ncols-2 is the maximum accepted value.

If you want a margin for the layout border, you must use

layout_margin4() https://nappgui.com/en/gui/layout.html#f53

image

static auto describePanel_Color(std::string const & title)
{
    Label * titleLabel = label_create();
    label_text(titleLabel, title.c_str());

    Label * redLabel = label_create();
        label_text(redLabel, "Red:");
    Slider * redSlider = slider_create();
        slider_value(redSlider, .5f);
    //  slider_OnMoved(redSlider, listener(app, i_OnAngle, App));

    Label * greenLabel = label_create();
        label_text(greenLabel, "Green:");
    Slider * greenSlider = slider_create();
        slider_value(greenSlider, .5f);
    //  slider_OnMoved(greenSlider, listener(app, i_OnScale, App));

    Label * blueLabel = label_create();
        label_text(blueLabel, "Blue:");
    Slider * blueSlider = slider_create();
        slider_value(blueSlider, .5f);
    //  slider_OnMoved(blueSlider, listener(app, i_OnScale, App));

    Layout * layout = layout_create(2, 4); // <--- why does ncols need to be >= 5. I'd expect 2 to be valid
        layout_label(layout,  titleLabel,  0, 0);
        layout_label(layout,  redLabel,    0, 1);
        layout_slider(layout, redSlider,   1, 1);
        layout_label(layout,  greenLabel,  0, 2);
        layout_slider(layout, greenSlider, 1, 2);
        layout_label(layout,  blueLabel,   0, 3);
        layout_slider(layout, blueSlider,  1, 3);

        layout_halign(layout, 0, 1, ekRIGHT);
        layout_halign(layout, 0, 2, ekRIGHT);
        layout_halign(layout, 0, 3, ekRIGHT);

        layout_hmargin(layout, 0, 10);
        //layout_hmargin(layout, 1, 10);

    Panel * panel = panel_create();
        panel_layout(panel, layout);

    return panel;
}
frang75 commented 1 year ago

On the other hand, you can use the latest C/C++ standards by indicating it in the desktopApp() command of the CMakeLists.txt

desktopApp("TobiasFord" "tobias" "osapp" NRC_NONE, "C++17")

https://nappgui.com/en/guide/newprj.html#h4

tobiasford commented 1 year ago

Thanks for the responses and for pointing out the issues regarding layout_hmargin. I had been working off of the samples as an example and I totally misunderstood that API (I also had not read its documentation). As consequence, I totally glossed over it and had just assumed that it was actually adding a padding border to that cell...

Thank you for pointing out my issue.

The other question that I have is whether or not it's possible chain together split views of the same type. The spits.c example shows chaining a horizontal to a vertical... Is it possible to chain together multiple verticals? if not, then what would be the recommended approach for creating N numbers of resizable columns?

ascii art example: ||XXXXXX||XXXXXX||XXXXXX||XXXXXX|| ||XXXXXX||XXXXXX||XXXXXX||XXXXXX|| ||XXXXXX||XXXXXX||XXXXXX||XXXXXX|| ||XXXXXX||XXXXXX||XXXXXX||XXXXXX||

frang75 commented 1 year ago

If in splits.c you change the line:

SplitView *split2 = splitview_horizontal();

by

SplitView *split2 = splitview_vertical();

It's just what you need.

Splitview_Vert

Also in this commit https://github.com/frang75/nappgui_src/commit/af7731dcdd782058bbf56681268ce2e5990b70e7 I've fixed some assert in splitview limits.

tobiasford commented 1 year ago

I've re-downloaded to pick up the commit.

unfortunately, I'm still hitting unexpected asserts with the following code example:

struct Pixel_float32RGBA
{
    float r = 0.f;
    float g = 0.f;
    float b = 0.f;
    float a = 1.f;
};

auto describePanel_Color = [this](std::string const & title, Pixel_float32RGBA const & color)
{
    Label * titleLabel = label_create();
    label_text(titleLabel, title.c_str());

    Label * redLabel = label_create();
        label_text(redLabel, "Red:");
    Slider * redSlider = slider_create();
        slider_value(redSlider, color.r);

    Label * greenLabel = label_create();
        label_text(greenLabel, "Green:");
    Slider * greenSlider = slider_create();
        slider_value(greenSlider, color.g);

    Label * blueLabel = label_create();
        label_text(blueLabel, "Blue:");
    Slider * blueSlider = slider_create();
        slider_value(blueSlider, color.b);

    Layout * layout = layout_create(2, 4);
        layout_label(layout,  titleLabel,  0, 0);
        layout_label(layout,  redLabel,    0, 1);
        layout_slider(layout, redSlider,   1, 1);
        layout_label(layout,  greenLabel,  0, 2);
        layout_slider(layout, greenSlider, 1, 2);
        layout_label(layout,  blueLabel,   0, 3);
        layout_slider(layout, blueSlider,  1, 3);

        layout_halign(layout, 0, 1, ekRIGHT);
        layout_halign(layout, 0, 2, ekRIGHT);
        layout_halign(layout, 0, 3, ekRIGHT);

    Panel * panel = panel_create();
        panel_layout(panel, layout);

    return panel;
};

auto describePanel_Main = [&]()
{
    Panel * panelColor0 = describePanel_Color("color0", Pixel_float32RGBA());
    Panel * panelColor1 = describePanel_Color("color1", Pixel_float32RGBA());
    Panel * panelColor2 = describePanel_Color("color2", Pixel_float32RGBA());

    Panel *panel1 = panel_create();
    Layout *layout = layout_create(1, 1);
    SplitView *split1 = splitview_vertical();
    SplitView *split2 = splitview_vertical();
    splitview_pos(split1, .25f);
    splitview_size(split1, s2df(800, 480));
    splitview_size(split2, s2df(640, 480));
    splitview_panel(split2, panelColor2);
    splitview_panel(split2, panelColor1);
    splitview_panel(split1, panelColor0);
    splitview_split(split1, split2);
    layout_splitview(layout, split1, 0, 0);
    panel_layout(panel1, layout);
    return panel1;
};

m_window = window_create(ekWINDOW_STDRES);
    window_panel(m_window, describePanel_Main());
    window_title(m_window, "Hello, C++!");
    window_origin(m_window, v2df(500, 200));
    window_OnClose(m_window, IListen(application, Application, onClose));
    window_show(m_window);

describePanel_Main is a modified version of splits.c with splitview_vertical being used for both splits and both TextView *text and View *view being replaced with Panels.

Thank you again for the responses.

frang75 commented 1 year ago

Hi again!

Yes, there is a bug in SplitView with nested panels. I've fixed it in this commit https://github.com/frang75/nappgui_src/commit/dfaf85f801890e0dcd62f31836d87e3fc2267ff6, please update.

I also include some tips for your code here:

/* NAppGUI Hello World */

#include "nappgui.h"
#include <string>

typedef struct _app_t App;

struct _app_t
{
    Window *window;
};

struct Pixel_float32RGBA
{
    float r = 0.f;
    float g = 0.f;
    float b = 0.f;
    float a = 1.f;
};

/*---------------------------------------------------------------------------*/

static auto describePanel_Color(std::string const & title, Pixel_float32RGBA const & color)
{
    Label * titleLabel = label_create();
    label_text(titleLabel, title.c_str());

    Label * redLabel = label_create();
        label_text(redLabel, "Red:");
    Slider * redSlider = slider_create();
        slider_value(redSlider, color.r);

    Label * greenLabel = label_create();
        label_text(greenLabel, "Green:");
    Slider * greenSlider = slider_create();
        slider_value(greenSlider, color.g);

    Label * blueLabel = label_create();
        label_text(blueLabel, "Blue:");
    Slider * blueSlider = slider_create();
        slider_value(blueSlider, color.b);

    Layout * layout = layout_create(2, 4);
        layout_label(layout,  titleLabel,  0, 0);
        layout_label(layout,  redLabel,    0, 1);
        layout_slider(layout, redSlider,   1, 1);
        layout_label(layout,  greenLabel,  0, 2);
        layout_slider(layout, greenSlider, 1, 2);
        layout_label(layout,  blueLabel,   0, 3);
        layout_slider(layout, blueSlider,  1, 3);

        layout_halign(layout, 0, 1, ekRIGHT);
        layout_halign(layout, 0, 2, ekRIGHT);
        layout_halign(layout, 0, 3, ekRIGHT);

        // This will be leave an horizontal fixed margin between labels and sliders
        layout_hmargin(layout, 0, 5);

        // This will be leave a vertical margin between sliders
        layout_vmargin(layout, 0, 5);
        layout_vmargin(layout, 1, 5);
        layout_vmargin(layout, 2, 5);

        // This will be expand horizontally ONLY the sliders column
        // and leave fixed the column of labels
        layout_hexpand(layout, 1);

        // Border margin arround the layout
        layout_margin4(layout, 10, 5, 0, 5);

    // This will align all components to the TOP
    // On other case, because Window is higher than layout, the layout will be expand vertically
    Layout *layout1 = layout_create(1, 1);
    layout_layout(layout1, layout, 0, 0);
    layout_valign(layout1, 0, 0, ekTOP);
    // 

    Panel * panel = panel_create();
        panel_layout(panel, layout1);

    return panel;
};

/*---------------------------------------------------------------------------*/

static auto describePanel_Main()
{
    Panel * panelColor0 = describePanel_Color("color0", Pixel_float32RGBA());
    Panel * panelColor1 = describePanel_Color("color1", Pixel_float32RGBA());
    Panel * panelColor2 = describePanel_Color("color2", Pixel_float32RGBA());

    Panel *panel1 = panel_create();
    Layout *layout = layout_create(1, 1);
    SplitView *split1 = splitview_vertical();
    SplitView *split2 = splitview_vertical();

    // Three panels begin with the same width 
    splitview_pos(split1, .33f);
    splitview_pos(split2, .5f);
    //

    splitview_size(split1, s2df(800, 480));
    splitview_size(split2, s2df(640, 480));

    // Panels swap
    splitview_panel(split2, panelColor1);
    splitview_panel(split2, panelColor2);
    //

    splitview_panel(split1, panelColor0);
    splitview_split(split1, split2);
    layout_splitview(layout, split1, 0, 0);
    panel_layout(panel1, layout);
    return panel1;
};

/*---------------------------------------------------------------------------*/

static void i_OnClose(App *app, Event *e)
{
    osapp_finish();
    unref(app);
    unref(e);
}

/*---------------------------------------------------------------------------*/

static App *i_create(void)
{
    App *app = heap_new0(App);
    Panel *panel = describePanel_Main();
    app->window = window_create(ekWINDOW_STD);
    window_panel(app->window, panel);
    window_title(app->window, "Hello, World!");
    window_origin(app->window, v2df(500, 200));
    window_OnClose(app->window, listener(app, i_OnClose, App));
    window_show(app->window);
    return app;
}

/*---------------------------------------------------------------------------*/

static void i_destroy(App **app)
{
    window_destroy(&(*app)->window);
    heap_delete(app, App);
}

/*---------------------------------------------------------------------------*/

#include "osmain.h"
osmain(i_create, i_destroy, "", App)

Split_win Split_Linux Split_macos

tobiasford commented 1 year ago

Thank you very much for the quick fixes and the additional suggestions. With those changes everything seems to be working as expected!

I'm currently in the process of updating and un-rotting my old OpenGL-based rendering engine and toolset to Metal / Vulkan / D3D and so far I really like this API and I think that it might be able to work for what I'm wanting to do without too much additional work.

For now, I need to focus on the renderer up and running and then getting that blitted into a view (which might be good enough).

I'll let you know how that goes when I get there (it's a weekend project, so it will be while :)

Thanks again.

frang75 commented 1 year ago

Thats sound good!

Keep me up to date on your work with the render engine. I'm working on OpenGL support for NAppGUI (extensible to D3D, Vulkan and Metal). This will allow you to connect an OpenGL context to a View directly, without any additional logic on your part. My idea is to publish it at the end of February or beginning of March.

nappgui_ogl

tobiasford commented 1 year ago

nice. looks promising! I like the graph editor, too.