Skip to content

Conversation

@ickshonpe
Copy link
Contributor

@ickshonpe ickshonpe commented Nov 22, 2025

Objective

Fix two problems

  1. Scrollbars are placed in the padding region of the node, not the content region. The content insets returned by ComputedNode::content_inset should account for the sizes of the scrollbars.
  2. In update_clipping_system the clipping rect calculations are incorrect, scrollbar_size should only be added to the right and bottom when visual_area is set to ContentBox. For BorderBox and PaddingBox, the scrollbars are inside the clipping rect.

Solution

  1. Add the scrollbar size to the right and bottom insets returned by ComputedNode::content_inset.
  2. Since content_inset now includes scrollbar size, remove the subtraction from clip_rect.max in update_clipping_system.

Testing

Look at:

cargo run --example scrollbars --features="experimental_bevy_ui_widgets"

@ickshonpe ickshonpe added C-Bug An unexpected or incorrect behavior A-UI Graphical user interfaces, styles, layouts, and widgets D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Nov 22, 2025
* The content insets returned by `ComputedNode::content_inset` should include the scroll width.
* In `update_clipping_system` the clipping rect calculations are incorrect, `scrollbar_size` should only be added to the `right` and `bottom` if when `visual_area` is set to `ContentBox`. For `BorderBox` and `PaddingBox`, the scrollbars are inside the clipping rect.
@ickshonpe ickshonpe force-pushed the ui-scroll-clipping-fix branch from 8a05dfc to c9acba2 Compare November 22, 2025 11:40
@ickshonpe ickshonpe changed the title UI clipping fix with scrolling UI scrollbars clipping fix Nov 22, 2025
Copy link
Contributor

@kfc35 kfc35 left a comment

Choose a reason for hiding this comment

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

I’m assuming this change is also relevant ui_node.rs resolve_clip_rect as well and that everything checks out there. I say this just because I see a usage of content_inset in that function.

My only small suggestion might be to note in the function comment that content_inset also includes scrollbars but probably not required?

@ickshonpe
Copy link
Contributor Author

I’m assuming this change is also relevant ui_node.rs resolve_clip_rect as well and that everything checks out there. I say this just because I see a usage of content_inset in that function.

Yes, the code using resolve_clip_rect in focus was incorrect before and should be fixed now.

My only small suggestion might be to note in the function comment that content_inset also includes scrollbars but probably not required?

Maybe the docs should be improved somewhere, but I'd rather leave it to another PR.

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 D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants