Skip to content
This repository has been archived by the owner on Dec 2, 2019. It is now read-only.

Layouting using groups #906

Open
Hejsil opened this issue Sep 18, 2019 · 14 comments
Open

Layouting using groups #906

Hejsil opened this issue Sep 18, 2019 · 14 comments

Comments

@Hejsil
Copy link
Contributor

Hejsil commented Sep 18, 2019 •
edited

So, I want to layout my widgets like this:

+------++------+
|      ||      |
|      |+------+
|      |+------+
|      ||      |
+------++------+

I see in overview.c that there is a Complex section that lays out widgets in a similar way using nk_layout_space_xxx. But nk_layout_space_xxx requires that I do the entire structure and style myself.

So my thought was to just use groups to achieve this.

nk_layout_row_dynamic(ctx, 50, 2);
if (nk_group_begin(ctx, "1", NK_WINDOW_BORDER | NK_WINDOW_NO_SCROLLBAR)) {
    nk_group_end(ctx);
}

if (nk_group_begin(ctx, "2", NK_WINDOW_BORDER | NK_WINDOW_NO_SCROLLBAR)) {
    nk_layout_row_dynamic(ctx, 22, 1);
    if (nk_group_begin(ctx, "3", NK_WINDOW_BORDER | NK_WINDOW_NO_SCROLLBAR)) {
        nk_group_end(ctx);
    }

    if (nk_group_begin(ctx, "4", NK_WINDOW_BORDER | NK_WINDOW_NO_SCROLLBAR)) {
        nk_group_end(ctx);
    }
    nk_group_end(ctx);
}

I get this.
image

This seems to be what I want, I just need to remove group 2's border and inner padding and I would get what I want.

Here is my first attempt

nk_layout_row_dynamic(ctx, 50, 2);
if (nk_group_begin(ctx, "1", NK_WINDOW_BORDER | NK_WINDOW_NO_SCROLLBAR)) {
    nk_group_end(ctx);
}

// Remove padding
struct nk_style_window old = ctx->style.window;
ctx->style.window.group_padding = nk_vec2(0, 0);
int is_showing = nk_group_begin(ctx, "2", NK_WINDOW_BORDER | NK_WINDOW_NO_SCROLLBAR);
ctx->style.window = old;

if (is_showing) {
    nk_layout_row_dynamic(ctx, 22, 1);
    if (nk_group_begin(ctx, "3", NK_WINDOW_BORDER | NK_WINDOW_NO_SCROLLBAR)) {
        nk_group_end(ctx);
    }

    if (nk_group_begin(ctx, "4", NK_WINDOW_BORDER | NK_WINDOW_NO_SCROLLBAR)) {
        nk_group_end(ctx);
    }
    nk_group_end(ctx);
}

image

Hmm. The top padding is gone, but the spacing between 1 and 2 is now gone to.

I'll try removing the padding from within the group instead.

nk_layout_row_dynamic(ctx, 50, 2);
if (nk_group_begin(ctx, "1", NK_WINDOW_BORDER | NK_WINDOW_NO_SCROLLBAR)) {
    nk_group_end(ctx);
}

if (nk_group_begin(ctx, "2", NK_WINDOW_BORDER | NK_WINDOW_NO_SCROLLBAR)) {
    struct nk_style_window old = ctx->style.window;
    ctx->style.window.group_padding = nk_vec2(0, 0);
    nk_layout_row_dynamic(ctx, 22, 1);
    if (nk_group_begin(ctx, "3", NK_WINDOW_BORDER | NK_WINDOW_NO_SCROLLBAR)) {
        nk_group_end(ctx);
    }

    if (nk_group_begin(ctx, "4", NK_WINDOW_BORDER | NK_WINDOW_NO_SCROLLBAR)) {
        nk_group_end(ctx);
    }
    ctx->style.window = old;
    nk_group_end(ctx);
}

image

This removes some of the left and right padding, but still not what I'm looking for.

Is there a way to get the desired outcome without over complicating my UI with nk_layout_space_xxx?
@dumblob
Copy link
Contributor

dumblob commented Sep 19, 2019

Is there a way to get the desired outcome without over complicating my UI with nk_layout_space_xxx?

I can't think of any such option - that's why it's called complex and why you have to do everything manually 😢. Feel free though to look e.g. at a separate layouting library from the original author of Nuklear to maybe get some inspiration on how to do it yourself in a manageable way (the linked library is about 2 years newer design than Nuklear).

@Hejsil
Copy link
Contributor Author

Hejsil commented Sep 19, 2019

Hmm. So there is really no way to remove the inner padding of group 2 to get the desired effect then. I guess I'll play around with nk_layout_space_xxx then.

@dumblob
Copy link
Contributor

dumblob commented Sep 19, 2019

So there is really no way to remove the inner padding of group 2 to get the desired effect then.

I don't know off the top of my head, I would need to dive into the code, it's been already few years since the layouting was added 😉. Feel free to take a look, it's not that complicated, but I'm still too constrained to look into it myself 😢.

@Hejsil
Copy link
Contributor Author

Hejsil commented Sep 19, 2019 •
edited

Playing with it a little more, I actually managed to get the desired result:

nk_layout_row_dynamic(ctx, 50, 2);
if (nk_group_begin(ctx, "1", NK_WINDOW_BORDER | NK_WINDOW_NO_SCROLLBAR)) {
    nk_group_end(ctx);
}

struct nk_style_window old = ctx->style.window;
ctx->style.window.group_padding = nk_vec2(0, 0);
if (nk_group_begin(ctx, "2", NK_WINDOW_NO_SCROLLBAR)) {
    nk_layout_row_dynamic(ctx, 23, 1);
    if (nk_group_begin(ctx, "3", NK_WINDOW_BORDER | NK_WINDOW_NO_SCROLLBAR)) {
        nk_group_end(ctx);
    }

    if (nk_group_begin(ctx, "4", NK_WINDOW_BORDER | NK_WINDOW_NO_SCROLLBAR)) {
        nk_group_end(ctx);
    }
    nk_group_end(ctx);
}
ctx->style.window = old;

image

Ofc, this also removes group 3 and 4's padding.

nk_layout_row_dynamic(ctx, 100, 2);
if (nk_group_begin(ctx, "1", NK_WINDOW_BORDER | NK_WINDOW_NO_SCROLLBAR)) {
    nk_layout_row_dynamic(ctx, 0, 1);
    nk_label(ctx, "test", NK_TEXT_LEFT);
    nk_group_end(ctx);
}

struct nk_style_window old = ctx->style.window;
ctx->style.window.group_padding = nk_vec2(0, 0);
if (nk_group_begin(ctx, "2", NK_WINDOW_NO_SCROLLBAR)) {
    nk_layout_row_dynamic(ctx, 48, 1);
    if (nk_group_begin(ctx, "3", NK_WINDOW_BORDER | NK_WINDOW_NO_SCROLLBAR)) {
        nk_layout_row_dynamic(ctx, 0, 1);
        nk_label(ctx, "test", NK_TEXT_LEFT);
        nk_group_end(ctx);
    }

    if (nk_group_begin(ctx, "4", NK_WINDOW_BORDER | NK_WINDOW_NO_SCROLLBAR)) {
        nk_layout_row_dynamic(ctx, 0, 1);
        nk_label(ctx, "test", NK_TEXT_LEFT);
        nk_group_end(ctx);
    }
    nk_group_end(ctx);
}
ctx->style.window = old;

image

@Hejsil
Copy link
Contributor Author

Hejsil commented Sep 19, 2019

I'll try playing a little more. I can probably restore the padding again.

@Hejsil
Copy link
Contributor Author

Hejsil commented Sep 19, 2019 •
edited

Hmm. Restoring the padding inside 3 and 4 doesn't work as I thought it would.

nk_layout_row_dynamic(ctx, 100, 2);
if (nk_group_begin(ctx, "1", NK_WINDOW_BORDER | NK_WINDOW_NO_SCROLLBAR)) {
    nk_layout_row_dynamic(ctx, 0, 1);
    nk_label(ctx, "test", NK_TEXT_LEFT);
    nk_group_end(ctx);
}

struct nk_style_window old = ctx->style.window;
ctx->style.window.group_padding = nk_vec2(0, 0);
if (nk_group_begin(ctx, "2", NK_WINDOW_NO_SCROLLBAR)) {
    ctx->style.window = old;
    nk_layout_row_dynamic(ctx, 48, 1);
    ctx->style.window.group_padding = nk_vec2(0, 0);
    if (nk_group_begin(ctx, "3", NK_WINDOW_BORDER | NK_WINDOW_NO_SCROLLBAR)) {
        ctx->style.window = old;
        nk_layout_row_dynamic(ctx, 0, 1);
        nk_label(ctx, "test", NK_TEXT_LEFT);
        ctx->style.window.group_padding = nk_vec2(0, 0);
        nk_group_end(ctx);
    }

    if (nk_group_begin(ctx, "4", NK_WINDOW_BORDER | NK_WINDOW_NO_SCROLLBAR)) {
        ctx->style.window = old;
        nk_layout_row_dynamic(ctx, 0, 1);
        nk_label(ctx, "test", NK_TEXT_LEFT);
        ctx->style.window.group_padding = nk_vec2(0, 0);
        nk_group_end(ctx);
    }
    nk_group_end(ctx);
}
ctx->style.window = old;

image

It seems I don't understand padding.

nk_layout_space_xxx is beginning to seem like the simpler option :)

@Hejsil
Copy link
Contributor Author

Hejsil commented Sep 19, 2019

Ooh. A simpler, near perfect solution

nk_layout_row_dynamic(ctx, 50, 2);
if (nk_group_begin(ctx, "1", NK_WINDOW_BORDER | NK_WINDOW_NO_SCROLLBAR)) {
    nk_group_end(ctx);
}

struct nk_style_window old = ctx->style.window;
ctx->style.window.group_padding = nk_vec2(0, 0);
int is_showing = nk_group_begin(ctx, "2", NK_WINDOW_BORDER | NK_WINDOW_NO_SCROLLBAR);
ctx->style.window = old;

if (is_showing) {
    nk_layout_row_dynamic(ctx, 22, 1);
    
    if (nk_group_begin(ctx, "3", NK_WINDOW_BORDER | NK_WINDOW_NO_SCROLLBAR)) {
        nk_group_end(ctx);
    }

    if (nk_group_begin(ctx, "4", NK_WINDOW_BORDER | NK_WINDOW_NO_SCROLLBAR)) {
        nk_group_end(ctx);
    }
    ctx->style.window.group_padding = nk_vec2(0, 0);
    nk_group_end(ctx);
    ctx->style.window = old;
}

image

Now I only need to figure out how to get rid of those small gaps to the left and right of 3 and 4.

@Hejsil
Copy link
Contributor Author

Hejsil commented Sep 20, 2019

Looking even more into this I've found some behavior that I find unexpected. With the default padding (4,4) we get this

image

I would expect that the top padding and side padding would be the same, but there is clearly more side padding than top padding. If I set padding to (10,10) the problems reveals itself.
image

It seems that side padding is double the top padding. This seems like a bug. I've been looking into this, and it seems to be because nk_layout_widget_space and the function it calls (nk_layout_row_calculate_usable_space) uses the x padding when getting widget space. This seems wrong as ctx->current->layout should already have it's bounds padded.

This patch removes this behavior:

diff --git a/nuklear.h b/nuklear.h
index 9b64e43..12a5f21 100644
--- a/nuklear.h
+++ b/nuklear.h
@@ -17622,15 +17622,12 @@ nk_layout_row_calculate_usable_space(const struct nk_style *style, enum nk_panel
     float panel_space;
 
     struct nk_vec2 spacing;
-    struct nk_vec2 padding;
 
     spacing = style->window.spacing;
-    padding = nk_panel_get_padding(style, type);
 
     /* calculate the usable panel space */
-    panel_padding = 2 * padding.x;
     panel_spacing = (float)NK_MAX(columns - 1, 0) * spacing.x;
-    panel_space  = total_space - panel_padding - panel_spacing;
+    panel_space  = total_space - panel_spacing;
     return panel_space;
 }
 NK_LIB void
@@ -18174,7 +18171,6 @@ nk_layout_widget_space(struct nk_rect *bounds, const struct nk_context *ctx,
     NK_ASSERT(bounds);
 
     spacing = style->window.spacing;
-    padding = nk_panel_get_padding(style, layout->type);
     panel_space = nk_layout_row_calculate_usable_space(&ctx->style, layout->type,
                                             layout->bounds.w, layout->row.columns);
 
@@ -18279,7 +18275,7 @@ nk_layout_widget_space(struct nk_rect *bounds, const struct nk_context *ctx,
     bounds->w = item_width;
     bounds->h = layout->row.height - spacing.y;
     bounds->y = layout->at_y - (float)*layout->offset_y;
-    bounds->x = layout->at_x + item_offset + item_spacing + padding.x;
+    bounds->x = layout->at_x + item_offset + item_spacing;
     if (((bounds->x + bounds->w) > layout->max_x) && modify)
         layout->max_x = bounds->x + bounds->w;
     bounds->x -= (float)*layout->offset_x;

Now padding looks more correct:
image

And I get the layout that I want
image

Now my only question is, if I'm correct in assuming this is a bug, and if this is the right fix. If it is, then I'll open a PR :)

Note: There seems to be another problem with window padding. I noticed it after I did my patch, but this is also a problem on master.

With patch:
image

Master:
image

@dumblob
Copy link
Contributor

dumblob commented Sep 20, 2019

@eax0r IIRC you said you already have some infrastructure for testing and I think this would be a good opportunity to test this subtle behavior @Hejsil describes. @eax0r feel free to take a look 😉.

@ghost
Copy link

ghost commented Sep 20, 2019

I will have a look tonight 😉.

@ghost
Copy link

ghost commented Sep 20, 2019

Hey @Hejsil can you send me a snippet of your last template that reproduce the issue.

Is this only occuring wtih specific nk_layout_xxx methods?
Also make sure that all of your style settings are set to default or even 0.

@Hejsil
Copy link
Contributor Author

Hejsil commented Sep 20, 2019 •
edited

To test these things, i use the x11_xtf demo.

printf("-------\n");
printf("%f %f\n", ctx->style.window.group_padding.x, ctx->style.window.group_padding.y);
printf("%f %f\n", ctx->style.window.padding.x, ctx->style.window.padding.y);
/* GUI */
if (nk_begin(ctx, "Demo", nk_rect(50, 50, 200, 200),
    NK_WINDOW_BORDER |
    NK_WINDOW_NO_SCROLLBAR))
{
    nk_layout_row_dynamic(ctx, 50, 1);
    if (nk_group_begin(ctx, "1", NK_WINDOW_BORDER | NK_WINDOW_NO_SCROLLBAR)) {
        nk_layout_row_dynamic(ctx, 0, 1);
        if (nk_group_begin(ctx, "1", NK_WINDOW_BORDER | NK_WINDOW_NO_SCROLLBAR)) {
            nk_group_end(ctx);
        }
        nk_group_end(ctx);
    }
}
nk_end(ctx);

I haven't edited the default styling here. The default is (4,4) for both window.group_padding and window.padding-

-------
4,000000 4,000000
4,000000 4,000000

What is draw (annotated in red with the actual pixel sizes of the padding.
image

Here we can see that side padding is bigger than top padding, both within the window, but also the group. Only the window have the inconsistent left/right padding problem. Also, notice that the window's top padding is 5, and not 4.

Here is testing with nk_layout_row_static instead:

printf("-------\n");
printf("%f %f\n", ctx->style.window.group_padding.x, ctx->style.window.group_padding.y);
printf("%f %f\n", ctx->style.window.padding.x, ctx->style.window.padding.y);
/* GUI */
if (nk_begin(ctx, "Demo", nk_rect(50, 50, 200, 200),
    NK_WINDOW_BORDER |
    NK_WINDOW_NO_SCROLLBAR))
{
    nk_layout_row_static(ctx, 50, 50, 1);
    if (nk_group_begin(ctx, "1", NK_WINDOW_BORDER | NK_WINDOW_NO_SCROLLBAR)) {
        nk_layout_row_static(ctx, 0, 30, 1);
        if (nk_group_begin(ctx, "1", NK_WINDOW_BORDER | NK_WINDOW_NO_SCROLLBAR)) {
            nk_group_end(ctx);
        }
        nk_group_end(ctx);
    }
}
nk_end(ctx);

image

Again, the same doubling of side padding. Can't really test the right padding, as this is controlled by the nk_layout_row_static.

@ghost
Copy link

ghost commented Sep 21, 2019 •
edited by ghost

Note: There seems to be another problem with window padding. I noticed it after I did my patch, but this is also a problem on master.

@Hejsil If you have any information, let me know.

It would be cool to fix window problems/internal drawing first, then investigate on elements implementation (such as layouts).

@Hejsil
Copy link
Contributor Author

Hejsil commented Sep 22, 2019

@eax0r If the problems pointed out in my last comment are fixed, then I don't need any new layouts to be implemented, as I can use what is currently in nuklear to do the layouting the way I want.

I haven't looked into the window padding problem yet, so I don't know what would cause that.

@Hejsil Hejsil mentioned this issue Nov 9, 2019
Open
@dumblob dumblob mentioned this issue Dec 3, 2019
Open
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@dumblob @Hejsil