Skip to content

Conversation

@JDDev0
Copy link

@JDDev0 JDDev0 commented Dec 31, 2025

Objective

  • The scroll position is not bound in the on_scroll_handler in the ui/scroll.rs example. (This is only a problem in combination with scrollbars. If scrollbars are present they could slightly overshoot.)

Solution

  • This PR clamps the scroll position between 0.0 and the max_position value.

Testing

  • I tested those changes by running the Scroll example on Ubuntu (Wayland). Scrolling still works. I tested those changes in my own bevy project in combination with scrollbars and this change fixed the problem.

@github-actions
Copy link
Contributor

Welcome, new contributor!

Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨

@JDDev0 JDDev0 changed the title Clamp scroll position within bounds Clamp scroll position within bounds in the ui/scroll.rs example Dec 31, 2025
@alice-i-cecile alice-i-cecile added C-Bug An unexpected or incorrect behavior A-UI Graphical user interfaces, styles, layouts, and widgets S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Dec 31, 2025
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sensible fix, but a comment on the math would probably be useful for readers.

Copy link
Contributor

@ickshonpe ickshonpe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very sleepy but isn't the scroll position clamped during layout anyway, why is this necessary?

There is something wrong though somewhere because I just tried adding scrollbars to the example and they undershoot now with or without this change. I'll take another look tomorrow.

@JDDev0
Copy link
Author

JDDev0 commented Dec 31, 2025

This is the maximum position you can get the scrollbar thumb with the mouse:
image

When resizing the window the scroll bar might overshoot this position (This is an issue with the scrollbar itself and not with the example):
image

@ickshonpe
Copy link
Contributor

ickshonpe commented Jan 1, 2026

When resizing the window the scroll bar might overshoot this position (This is an issue with the scrollbar itself and not with the example): image

Oh yeah that looks bad, is that bevy_ui_widgets scrollbar?

It looks like underlying the problem is that update_scrollbar_thumb uses the value of the ScrollPosition component to calculate the position of the thumb, instead of the clamped scroll position from computed node.

@JDDev0
Copy link
Author

JDDev0 commented Jan 1, 2026

From my testing it only overshoots in two cases:

  1. If the scroll position is set to something lower than 0.0 or higher than the maximum
  2. If the window is resized and the max scroll position of the scrollable area gets smaller

edit: When creating this PR, I did not test resizing the window. I only tested the first case for which this PR fixed the problem.

@JDDev0
Copy link
Author

JDDev0 commented Jan 1, 2026

Yes, this is the bevy_ui_widgets scrollbar.

@ickshonpe
Copy link
Contributor

ickshonpe commented Jan 1, 2026

Maybe it's a change detection bug or something then, where the position isn't updated correctly when the size changes.

Part of the purpose of the examples is to help identify bugs, so I'm not sure a work around like this is the right approach. It'd be better I think if you could find a fix for whatever the underlying problem is in the scrollbar implementation itself.

@alice-i-cecile alice-i-cecile added X-Contentious There are nontrivial implications that should be thought through S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jan 1, 2026
@JDDev0
Copy link
Author

JDDev0 commented Jan 1, 2026

I can't reproduce the problem outside of my bevy project [running on bevy v0.17.3] (I'm using a deeply nested UI node hierarchy where everything uses vmin, vw, vh for sizing, border size, padding, and margin and the font size is also automatically adapted for the current window size).

I'm trying to create a MRE with bevy v0.17.3. Afterwards I updated it to the latest commit of bevy and check if the problem still persists.

@JDDev0
Copy link
Author

JDDev0 commented Jan 1, 2026

I closed this PR, because the problem is no longer present in 0.18.0-rc.2.
I think that the problem was fixed in #21378.

@JDDev0 JDDev0 closed this Jan 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-UI Graphical user interfaces, styles, layouts, and widgets C-Bug An unexpected or incorrect behavior S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged X-Contentious There are nontrivial implications that should be thought through

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants