Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

What is the correct/easiest way to get the max y scroll value? #770

Open
rswinkle opened this issue Feb 6, 2025 · 5 comments
Open

What is the correct/easiest way to get the max y scroll value? #770

rswinkle opened this issue Feb 6, 2025 · 5 comments

Comments

@rswinkle
Copy link
Contributor

rswinkle commented Feb 6, 2025

My question could apply to any window/group etc. though I'm specifically dealing with list views.

I have code that lets the user navigate through the list with the keyboard and I need to update the scroll position manually based on that as appropriate. I've mostly gotten by with code like this:

        // list stuff here removed for brevity
        list_height = ctx->current->layout->clip.h; // ->bounds.h?
        nk_list_view_end(&rview);
    }

if (do_adjust && rview.total_height > list_height) {
    int scroll_limit = rview.total_height - list_height; // little off
    nk_uint y = (selection/(float)(list.size-1) * scroll_limit) + 0.999f;
    nk_group_set_scroll(ctx, "List", 0, y);
    do_adjust = SDL_FALSE;
}

With a few variants. Sometimes it seems to work perfectly, but often it's off a little at the end. Also sometimes list views let you scroll past the last element by a whole row or two and I don't know why.

Digging through the code has been less than helpful and I'd really rather not waste more time trying to figure it out if someone already knows. I've seen some old issues that are adjacent to this issue. Some about a flashing screen when setting to some arbitrary high number to get to the end. I have seen that flashing before when unnecessarily adjusting the scroll position but that's not really the issue.

Thanks in advance for any help/advice.

@klei1984
Copy link

Based on v4.12.3, the nk_list_view struct can be a local temporary object that is an output parameter of nk_list_view_begin. All its members are overwritten by the API call, it does not need to be persistent or initialized.

Also sometimes list views let you scroll past the last element by a whole row or two and I don't know why.

The minimum value of nk_list_view's begin member is 0. The maximum value of the end member is row_count meaning that a for loop that uses begin and end must test against index < end and should not attempt to instantiate more nk_selectable_label widgets inside the group than `row_count˙.

The vertical scrollbar limit is determined based on the panel height of the nk_list_view's window group which is determined by nk_list_view_begin API arguments and styling, (row_height + window spacing style attribute) * row_count, independent of the actual bounds that the window group itself has based on list view widget layout configurations. This means that if you do not accurately specify the space (row height) versus the list item count (row count) considering styling, then you will be able to scroll into empty space on the panel where no nk_selectable_label widgets are "placed".

Sometimes it seems to work perfectly, but often it's off a little at the end.

In the above described case, where the panel vertical space is not determined accurately as expected by the list view widget, the given formula could be caused for misalignments:

    // this is using panel height which is not necessarily equals item count * item height
    int scroll_limit = rview.total_height - list_height;
    // the next section uses item count and selected item which will not scale properly in case there is "gap" at the end of the
    // which is not considered here
    nk_uint y = (selection/(float)(list.size-1) * scroll_limit) + 0.999f;
    // considering that horizontal scrollbar cannot be disabled in case there is vertical one currently, hard coding 0 for x offset
    // may be bad practice. The current x offset could be fetched via the get scroll API and could be fed into the setter.
    nk_group_set_scroll(ctx, "List", 0, y);

I did not look into the mouse scroll event handling part of the code and could not determine why y offset max becomes what it does. If you can figure that out I am also interested in it.

As a design alternative, could it happen that injecting NK_KEY_SCROLL_* Nuklear input events would be simpler on keyboard navigation events instead of manipulating the scrollbar offsets by hand?

@rswinkle
Copy link
Contributor Author

The vertical scrollbar limit is determined based on the panel height of the nk_list_view's window group which is determined by nk_list_view_begin API arguments and styling, (row_height + window spacing style attribute) * row_count, independent of the actual bounds that the window group itself has based on list view widget layout configurations.

I'm pretty sure I'm already taking that into account.

I based my code on this from nk_list_view_begin():

    item_spacing = style->window.spacing;
    row_height += NK_MAX(0, (int)item_spacing.y);
...
    view->total_height = row_height * NK_MAX(row_count,1);
    view->begin = (int)NK_MAX(((float)view->scroll_value / (float)row_height), 0.0f);
    view->count = (int)NK_MAX(nk_iceilf((layout->clip.h)/(float)row_height),0);
    view->count = NK_MIN(view->count, row_count - view->begin);
    view->end = view->begin + view->count;

This means that you can't just put in your font size (or even more conveniently 0 as you can for most other common functions), but you also can't include spacing since they do it for you, clearly. You can see in my original code snippet that I use the nk_list_view total height which is exactly the calculation you're talking about. But logically the max scroll value should not be that unless the list panel were 1 pixel thick. The scrollbar should have have a travel equal to the total height of all the elements minus the height visible in the pane (ie the visible clipping rectangle) because you only have to scroll till the bottom of the list view hits the end of the list, not the top. Clearly I'm close but missing some aspect.

I did not look into the mouse scroll event handling part of the code and could not determine why y offset max becomes what it does. If you can figure that out I am also interested in it.

This is the same problem as above. Whether it's manual or internal based on scroll events, I am not grokking the calculation or how to get it in my own code.

As a design alternative, could it happen that injecting NK_KEY_SCROLL_* Nuklear input events would be simpler on keyboard navigation events instead of manipulating the scrollbar offsets by hand?

This is worth looking into as it might sidestep the issue to some extent but then you have to know how big a delta to inject which might just mean a different but equally close-but-not-quite calculation. Regardless, I'd still rather know and how to do it right should be documented and understood.

This discussion would be much easier with real world code to look at. I have it already, as part of a larger project, but give me a few days and I'll have a smaller project and/or hopefully a pull request we can experiment with and discuss specifically.

@rswinkle
Copy link
Contributor Author

Ok I've managed to cleanup/organize the code a bit more, enough to separate it out into a library for demo/discussion purposes at least:

file_browser

Long term it would be cool to get this into the repo as one of the common demos commented out in all. The current file_browser demo is nice (and I took inspiration from it for the breadcrumb buttons) but a bit unwieldy and I think my list based approach modeled on standard file selection dialogs is more practical.

You can see the scrolling code we were discussing before here.

There are actually 3 reasonable ways I can think of to handle scrolling via keyboard:

  1. Adjust the scroll position for every key press to a unique offset for the current selection so the relative position of the highlighted selection varies from the top for the first item to the bottom for the last item, moving down as you arrowed through. That's what I attempt to do in this code, but I don't think it's actually the best.
  2. Adjust the scroll only when the selection is the first or last fully visible element in the list, so as you move down you would stay at the bottom relatively as it's only barely adjusting so the current is visible (You might adjust it so it's always the second to last one as well for better visibility). I think this behavior is more common in list based UI elements including file browsers and it requires fewer adjustments than method 1.
  3. Adjust an entire page up or down when you move off the visible items. This could be done by injecting page up/down into nuklear. I was unaware those existed till you mentioned that method, thinking we could only inject mouse scrollwheel deltas. Unfortunately it doesn't seem to work perfectly/consistently. Usually it leaves what was the last element before at the top (and vice versa when paging up) but not as you approach the end of the list (even when there's still room to do so).

In either case 1 or 2, we need a reliable convenient way of getting the max scroll value and calculating an offset based on that, or an API function that does exactly what we need directly somehow. Something that throws a wrench in the works of all of these to some extent is when the list panel shows partial elements. If you have space to show 10.5 list elements, you don't want to not scroll because item 11 is technically already shown and I think it also might be a source of my inconsistent results as well.

You can see I have a hacky hybrid 1+2 approach in my larger project here and 3 other places in that file. I'm not happy with it, I would definitely prefer 2 on its own but there's little point till I understand how to do the calculation correctly.

Any help/advice from anyone is appreciated, not just about this specific issue but about the nuklear file_browser code in general and ways to improve it and make it more plug n' play. Of course SDL3 just made it obsolete since they added standard Dialogs not to mention some convenient file system functions but SDL3 is still a long way from make it into the majority of distros, let alone as the default, and all the other Nuklear backends could still benefit from an easy to use file browser/selection dialog.

@klei1984
Copy link

I went through nuklear_filebrowser.c and made the following observations:

  • at line 550, nk_layout_row(ctx, NK_STATIC, scr_h, 2, group_szs);, the height specified by scr_h is not the remaining vertical space in the window. The folder path loop can dynamically grow into new rows which means the remaining window vertical space will always change for the side-panel and list groups. Using scr_h - ctx->current->layout->at_y or comparable as height is more accurate, but this is reliance on implementation details, circumventing the public API which is just as bad or maybe worse. Any ways, scr_h as height is wrong.

  • after line 627, nk_layout_row(ctx, NK_DYNAMIC, 0, 5, header_ratios);, ctx.current.layout.row structure stores height of 33 and min_height of 29 pixels. 3 buttons and two spacers are added into the 5 columns of the given single row. OK.

  • at line 703, nk_layout_row_dynamic(ctx, scr_h-(path_rows+2)*search_height, 1);, height is calculated as scr_h-(path_rows+2)*search_height. Variable search_height is determined from a widget that belongs to window 1 row 1 assuming row 2 and optional further rows keep row height, font size, font padding and vertical spacing the same. The row height after the layout rule becomes 717 pixels in case 2 rows are present without optional ones. The group's at_y is 107 so 800 - 107 != 717 so calculated height is wrong.

  • at line 756, nk_list_view_begin(ctx, &lview, "File List", NK_WINDOW_BORDER, FONT_SIZE+16, f->size), the list view gets a row height of FONT+16 = 32 even though nominal auto row height is set on line 759 to 33, min_height to 29 and vertical spacing (by default) is 4 pixels inside the list view. This suggests to me that later on rview.total_height and actual nk_selectable_label widget space will not correlate to each other.

After several hours of trial and error I could not figure out any ways via using public API to set nk_group_scrolled_offset_begin widget row height to reminder of parent window/group height in a way that vertical scrollbar correctly aligns and renders child panel as long as scrolled group height is smaller than panel space inside it.

@rswinkle
Copy link
Contributor Author

at line 550, nk_layout_row(ctx, NK_STATIC, scr_h, 2, group_szs);, the height specified by...

This is what I get for developing it in parallel, sometimes I fix things in sdl_img and don't mirror them back or vice versa. I had this fixed in sdl_img but not here. It changes things a little but things still don't look like I expect them to.

after line 627, nk_layout_row(ctx, NK_DYNAMIC, 0, 5, header_ratios);, ...

This is just an observation right? Nothing wrong here, though I'll get back to your numbers later

at line 703, nk_layout_row_dynamic(ctx, scr_h-(path_rows+2)*search_height, 1);...

I don't know why I'm solving it that way in the first place (which I also have in sdl_img) instead of the way I changed line 550. It may still be incorrect but if at all possible we should be abl eto get the correct height directly without having to do annoying calculations like this. I've changed it to get bound.y like I did for the other one.

at line 756, nk_list_view_begin(ctx, &lview, "File List", NK_WINDOW_BORDER, FONT_SIZE+16, f->size), the list view gets a row height of FONT+16 = 32 even though nominal auto row height is set on line 759 to 33, min_height to 29 and vertical spacing (by default) is 4 pixels inside the list view. This suggests to me that later on rview.total_height and actual nk_selectable_label widget space will not correlate to each other.

This is where we get back to those numbers and you lose me. I dug into this before for determining min window dimensions for a single row and came up with +16 from nk_layout_reset_min_row_height() which has this

    layout = win->layout;
    layout->row.min_height = ctx->style.font->height;
    layout->row.min_height += ctx->style.text.padding.y*2;
    layout->row.min_height += ctx->style.window.min_row_height_padding*2;

text.padding.y is 0 and min_row_height_padding is 8 so I do FONT_SIZE+16 for nk_list_view_begin().
remember it adds window spacing internally so row_height should come out to 36 so I'm not sure where you're getting 33 or 29. Again clearly I'm missing something but once again there should be an easier way of doing this. We shouldn't have to know the implementation details of the library to use nk_list_view. If we can let it automatically set height for others, why not this?

After several hours of trial and error I could not figure out any ways via using public API to set nk_group_scrolled_offset_begin widget row height to reminder of parent window/group height in a way that vertical scrollbar correctly aligns and renders child panel as long as scrolled group height is smaller than panel space inside it.

I'm not sure I'm parsing this correctly but it sounds like you might have beat your head against one of the same thing I did a while ago till I just said "good enough, doesn't seem to make much visual difference".

To sum up we have at least a few distinct but related issues:

  • How to properly size/arrange sub groups in windows so they look the way we think they should
  • What height to pass to nk_list_view_begin under various circumstances
  • How to manually do scrolling correctly, ie how to get max y scroll and calculate the correct offset for any of the 3 behaviors I mentioned in my last comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants