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

WIP ItemScrollArea #1871

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

WIP ItemScrollArea #1871

wants to merge 3 commits into from

Conversation

xNxExOx
Copy link
Contributor

@xNxExOx xNxExOx commented Jul 30, 2022

Closes Variable Sized Items in Scroll View.

It works but:

  • jumps when scrolling up (left?) to new item, because it does not have size for that item
    • possible solution is to always render item -1 and +1 and store their sizes in the State
    • would it be worth it to render block to an drop buffer, just to get its size, before normal rendering?
    • so would it be worth it? when drawing < 16ms it is barely noticeable (at least to me)
  • How to prevent shrinking with scroll bar?
  • Scrolling speed is not great, I divided it by state.first_shown_item_size[d] to not be too fast, but this can make it too slow. Any better option?
  • A lot of code is just copy pasted from ScrollArea so it would be worth considering, if such feature can be added to ScrollArea somehow to reuse most of the code.
    • Problem is that it is similar not same, so not sure if it would not hurt readability too much 😞
    • I added such option with ScrollState trait to separate the the differences. (not sure if it looks better than
  • Why window placer prevents auto placing windows after this and start stacking them? is there a bug in shrinking (during first frame)?

@xNxExOx xNxExOx force-pushed the item_scroll_area branch from 78d9e89 to 50d7d42 Compare July 31, 2022 05:35
@xNxExOx
Copy link
Contributor Author

xNxExOx commented Aug 9, 2022

@emilk probably no one else is going to take a look at this, so are you interested in either version (copy to separate type, or just added method to the ScrollArea)
And what about the mentioned issues, are they acceptable, or do you have any suggestions?

@xNxExOx
Copy link
Contributor Author

xNxExOx commented Aug 17, 2022

@emilk still waiting on your opinion if you would like it copied to separate widget, or as part of the existing ScrollArea, or not at all. So I know which one I should delete.

@emilk
Copy link
Owner

emilk commented Aug 20, 2022

Hi, sorry for the late reply, but 1400 lines of code is a lot. Before reviewing something like that it would be nice with an overview of what the code even does.

@xNxExOx
Copy link
Contributor Author

xNxExOx commented Aug 20, 2022

I did not ask for review yet @emilk .
Based on the discord questions I will put it also here.
When there is a lot of "blocks" / "items" (name open for discussion) and sizes can change between frames too often. (my use case is debugger for game rendering parts of game state, and I do not have notifications when entity gets new ability, or is killed, so each frame the "block" can look different)
You can imagine "block" as group of information, in my case information about the entity, with list of abilities affecting it.
heterogeneous_rows are not really good for this use case, because they still require knowing the sizes, on the caller side, and I do not see an easy way to obtain new sizes, from it if they change, so I would need to calculate them on every frame again, which isn't much better, than actually rendering it.

This approaches the problem differently, caller only needs to know how many "blocks" there is.
scrollbar will be split to N parts (invisible to user). For example with 10 "blocks" scrolling to exactly 50% will render "block" 5, and next blocks only if there will be enough space to start them. Scrolling to 55% will render "block" 5 offset-ed out by half.
As I mentioned in the top post there are some issues when jumping on the scrollbar, but they are not too relevant now.
My question is on the top level for now:

  • Would you like this as an separate container egui/src/containers/item_scroll_area.rs (name open for discussion)
    • (+) No added complexity to ScrollArea
    • (-) have a lot of code copied from there
  • Would you like this as an extension of existing ScrollArea::show_blocks, needs some parts extracted to trait, and implemented differently egui/src/containers/scroll_area.rs + egui/src/containers/scroll_area/scroll_area_blocks.rs
    • (+) no duplicate code
    • (-) complexity added to ScrollArea
  • Or do you not like this feature at all?

@emilk
Copy link
Owner

emilk commented Aug 20, 2022

Thanks for explaining it a bit more! I cannot really answer how I want the code structured until I feel I understand the method underlying it. I am still hoping there is a simpler way to implement it, for instance similar to how ScrollArea::show_rows just calls show_viewport without much extra code. That would be the ideal imho.

So do I get this right that scrolling 50% shows 5/10 block no matter how big the different blocks are? So even if the first 5 blocks are very small and the last 5 are really tall, the scrollbar will be at 50% when you are viewing the border between block 5 and 6? That's a pretty clever approach!

If that is indeed the approach, then I think it could be implemented by using the underlying ScrollArea. Given an Id for a ScrollArea we can query it for its current offset, and then set it up with a arbitrary height (say 10000) and calculate the items that should be visible from the offset and the height (num_blocks*offset/10000). We can then use show_viewport to paint the blocks that should be visible at the correct height.

…but perhaps I'm missing some subtle things that makes this approach difficult or impossible.

@xNxExOx
Copy link
Contributor Author

xNxExOx commented Aug 20, 2022

Yes you understood it correctly.
I think that would work just fine if all items would be smaller than the view, but I do not see how that would allow scrolling inside an "block".
And just an technical question who would be responsible for not rendering too much? (that is probably the smaller problem)
If you can tell me how to handle this case, I can try to implement it this way, but right now I do not see a way how it should work.

So this example "blocks" 0-4 are single label each, and 5-9 are huge and do not fit to screen (at least 2x bigger than screen).

I my version:

scrolling to to 40% will show whole "block" 4 (single label), and renders whole "block" 5 of which only beginning will be visible.
scrolling to to 45% will render whole "block" 4 (single label), of which only bottom half will be visible, and "block" 5 as before
scrolling to to 55% will render whole "block" 5 and visible area will contain whatever is at 50% of that block and below

Your Idea, or at least my understanding of it: (please fill in)

scrolling to to 40% will show whole "block" 4 (single label), and renders whole "block" 5 of which only beginning will be visible. (same, as mine, because viewport starts at "block" 4, and "block" 5 can also start, but "block" 6 is out of scope
scrolling to to 45% will do what?
scrolling to to 55% will do what?

@emilk
Copy link
Owner

emilk commented Aug 20, 2022

scrolling to to 45% will render whole "block" 4 (single label), of which only bottom half will be visible, and "block" 5 as before

How would you accomplish this? How would you position block 4 so that exactly half of it is visible, given you don't know its height before it has already been painted?

@xNxExOx
Copy link
Contributor Author

xNxExOx commented Aug 20, 2022

I store the height of the first block in state between frames, so I just start drawing at -50% of that height

@jackzhp
Copy link

jackzhp commented Apr 13, 2023

I would recommend it to be another widget.

I have no problem with #1376 which this thread is intended to solve. In my implementation(to present chatting messages), i used less than 200 lines.

My problem is stick_to_end or stick_to_bottom, as stated in #2897 .

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

Successfully merging this pull request may close these issues.

Variable Sized Items in Scroll View
3 participants