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

ScrollArea contents briefly escape it when it becomes larger #4742

Closed
e00E opened this issue Jun 30, 2024 · 1 comment · Fixed by #5006
Closed

ScrollArea contents briefly escape it when it becomes larger #4742

e00E opened this issue Jun 30, 2024 · 1 comment · Fixed by #5006
Labels
bug Something is broken

Comments

@e00E
Copy link
Contributor

e00E commented Jun 30, 2024

I have content inside a ScrollArea. When the content's size changes and goes from fitting into the area to no longer fitting, then for a split second the content escapes the ScrollArea. After the split second the content is correctly limited to the area and can be scrolled.

I would expect the content to not appear out of bounds for the split second. I realize there are some layout limitations with egui's design. I feel this is a bug because the available size of the area is known at all times. I do not see why in principle there has to be a moment (one frame?) of bad layout.

Below is a minimal reproduction. The program displays a label inside a fixed size scroll area. When it starts, the label fits and there is no need to scroll. When you click the button, the amount of text changes so that it no longer fits into the scroll area and scrolling is needed.

use eframe::Frame;
use egui::{Context, ScrollArea};

fn main() -> eframe::Result<()> {
    let mut lines: usize = 1;

    let run = move |c: &Context, _: &mut Frame| {
        let text = "aaaa\n".repeat(lines);

        egui::CentralPanel::default().show(c, |ui| {
            ui.horizontal(|ui| {
                ui.allocate_ui((50., 50.).into(), |ui| {
                    ScrollArea::both()
                        .auto_shrink(false)
                        .show(ui, |ui| ui.label(text));
                });

                if ui.button("button").clicked() {
                    if lines == 10 {
                        lines = 1;
                    } else {
                        lines = 10;
                    }
                };
            });
        });
    };

    let options = eframe::NativeOptions::default();
    eframe::run_simple_native("", options, run)
}

Here are screenshots displaying the bug.

Before you press the button. There is no bug.
1
After you press the button, for a split second you can see the bug. The text exceeds the area. If I had more content below, the text would be rendered on top of it.
2
After the split second. There is no bug.
3

@e00E e00E added the bug Something is broken label Jun 30, 2024
@emilk
Copy link
Owner

emilk commented Jun 30, 2024

This should ineed be fixable. The clip rect is probably not set correctly when the ScrollArea believes (based on sizes from previous frame) that it won't need to clip.

emilk pushed a commit that referenced this issue Aug 27, 2024
When a `ScrollArea` is added to a `Ui` or its contents change
dynamically, the contents will briefly escape the container. This occurs
because `ScrollArea` internally maintains `content_is_too_large` flags,
from which it determines when to clip. The `content_is_too_large` flags
are calculated after painting, so they always lag one frame behind. This
can lead to flickering.

To fix this, I have changed the `ScrollArea` so that it always clips
scrollable content. I believe that this should fix things without
negatively impacting other behavior. To see this, consider how
`ScrollArea` calculates the `content_is_too_large` flag:

```rust
// This calculates a new inner rect, after painting, from the initial clip rect
let inner_rect = {
  // At this point this is the available size for the inner rect.
  let mut inner_size = inner_rect.size();
  
  for d in 0..2 {
      inner_size[d] = match (scroll_enabled[d], auto_shrink[d]) {
          (true, true) => inner_size[d].min(content_size[d]), // shrink scroll area if content is small
          (true, false) => inner_size[d], // let scroll area be larger than content; fill with blank space
          (false, true) => content_size[d], // Follow the content (expand/contract to fit it).
          (false, false) => inner_size[d].max(content_size[d]), // Expand to fit content
      };
  }
  
  Rect::from_min_size(inner_rect.min, inner_size)
};

let outer_rect = Rect::from_min_size(inner_rect.min, inner_rect.size() + current_bar_use);

let content_is_too_large = Vec2b::new(
  scroll_enabled[0] && inner_rect.width() < content_size.x,
  scroll_enabled[1] && inner_rect.height() < content_size.y,
);
```
If `scroll_enabled[d] == true`, then the actual `inner_rect` (which is
calculated after painting contents) will always be smaller than the
original `inner_rect`. Hence, it is safe to unconditionally clip the
contents to `inner_rect` whenever `scroll_enabled[d] == true`.

<!--
Please read the "Making a PR" section of
[`CONTRIBUTING.md`](https://github.com/emilk/egui/blob/master/CONTRIBUTING.md)
before opening a Pull Request!

* Keep your PR:s small and focused.
* The PR title is what ends up in the changelog, so make it descriptive!
* If applicable, add a screenshot or gif.
* If it is a non-trivial addition, consider adding a demo for it to
`egui_demo_lib`, or a new example.
* Do NOT open PR:s from your `master` branch, as that makes it hard for
maintainers to test and add commits to your PR.
* Remember to run `cargo fmt` and `cargo clippy`.
* Open the PR as a draft until you have self-reviewed it and run
`./scripts/check.sh`.
* When you have addressed a PR comment, mark it as resolved.

Please be patient! I will review your PR, but my time is limited!
-->

* Closes <#4742>
* [x] I have followed the instructions in the PR template
486c pushed a commit to 486c/egui that referenced this issue Oct 9, 2024
…k#5006)

When a `ScrollArea` is added to a `Ui` or its contents change
dynamically, the contents will briefly escape the container. This occurs
because `ScrollArea` internally maintains `content_is_too_large` flags,
from which it determines when to clip. The `content_is_too_large` flags
are calculated after painting, so they always lag one frame behind. This
can lead to flickering.

To fix this, I have changed the `ScrollArea` so that it always clips
scrollable content. I believe that this should fix things without
negatively impacting other behavior. To see this, consider how
`ScrollArea` calculates the `content_is_too_large` flag:

```rust
// This calculates a new inner rect, after painting, from the initial clip rect
let inner_rect = {
  // At this point this is the available size for the inner rect.
  let mut inner_size = inner_rect.size();
  
  for d in 0..2 {
      inner_size[d] = match (scroll_enabled[d], auto_shrink[d]) {
          (true, true) => inner_size[d].min(content_size[d]), // shrink scroll area if content is small
          (true, false) => inner_size[d], // let scroll area be larger than content; fill with blank space
          (false, true) => content_size[d], // Follow the content (expand/contract to fit it).
          (false, false) => inner_size[d].max(content_size[d]), // Expand to fit content
      };
  }
  
  Rect::from_min_size(inner_rect.min, inner_size)
};

let outer_rect = Rect::from_min_size(inner_rect.min, inner_rect.size() + current_bar_use);

let content_is_too_large = Vec2b::new(
  scroll_enabled[0] && inner_rect.width() < content_size.x,
  scroll_enabled[1] && inner_rect.height() < content_size.y,
);
```
If `scroll_enabled[d] == true`, then the actual `inner_rect` (which is
calculated after painting contents) will always be smaller than the
original `inner_rect`. Hence, it is safe to unconditionally clip the
contents to `inner_rect` whenever `scroll_enabled[d] == true`.

<!--
Please read the "Making a PR" section of
[`CONTRIBUTING.md`](https://github.com/emilk/egui/blob/master/CONTRIBUTING.md)
before opening a Pull Request!

* Keep your PR:s small and focused.
* The PR title is what ends up in the changelog, so make it descriptive!
* If applicable, add a screenshot or gif.
* If it is a non-trivial addition, consider adding a demo for it to
`egui_demo_lib`, or a new example.
* Do NOT open PR:s from your `master` branch, as that makes it hard for
maintainers to test and add commits to your PR.
* Remember to run `cargo fmt` and `cargo clippy`.
* Open the PR as a draft until you have self-reviewed it and run
`./scripts/check.sh`.
* When you have addressed a PR comment, mark it as resolved.

Please be patient! I will review your PR, but my time is limited!
-->

* Closes <emilk#4742>
* [x] I have followed the instructions in the PR template
hacknus pushed a commit to hacknus/egui that referenced this issue Oct 30, 2024
…k#5006)

When a `ScrollArea` is added to a `Ui` or its contents change
dynamically, the contents will briefly escape the container. This occurs
because `ScrollArea` internally maintains `content_is_too_large` flags,
from which it determines when to clip. The `content_is_too_large` flags
are calculated after painting, so they always lag one frame behind. This
can lead to flickering.

To fix this, I have changed the `ScrollArea` so that it always clips
scrollable content. I believe that this should fix things without
negatively impacting other behavior. To see this, consider how
`ScrollArea` calculates the `content_is_too_large` flag:

```rust
// This calculates a new inner rect, after painting, from the initial clip rect
let inner_rect = {
  // At this point this is the available size for the inner rect.
  let mut inner_size = inner_rect.size();
  
  for d in 0..2 {
      inner_size[d] = match (scroll_enabled[d], auto_shrink[d]) {
          (true, true) => inner_size[d].min(content_size[d]), // shrink scroll area if content is small
          (true, false) => inner_size[d], // let scroll area be larger than content; fill with blank space
          (false, true) => content_size[d], // Follow the content (expand/contract to fit it).
          (false, false) => inner_size[d].max(content_size[d]), // Expand to fit content
      };
  }
  
  Rect::from_min_size(inner_rect.min, inner_size)
};

let outer_rect = Rect::from_min_size(inner_rect.min, inner_rect.size() + current_bar_use);

let content_is_too_large = Vec2b::new(
  scroll_enabled[0] && inner_rect.width() < content_size.x,
  scroll_enabled[1] && inner_rect.height() < content_size.y,
);
```
If `scroll_enabled[d] == true`, then the actual `inner_rect` (which is
calculated after painting contents) will always be smaller than the
original `inner_rect`. Hence, it is safe to unconditionally clip the
contents to `inner_rect` whenever `scroll_enabled[d] == true`.

<!--
Please read the "Making a PR" section of
[`CONTRIBUTING.md`](https://github.com/emilk/egui/blob/master/CONTRIBUTING.md)
before opening a Pull Request!

* Keep your PR:s small and focused.
* The PR title is what ends up in the changelog, so make it descriptive!
* If applicable, add a screenshot or gif.
* If it is a non-trivial addition, consider adding a demo for it to
`egui_demo_lib`, or a new example.
* Do NOT open PR:s from your `master` branch, as that makes it hard for
maintainers to test and add commits to your PR.
* Remember to run `cargo fmt` and `cargo clippy`.
* Open the PR as a draft until you have self-reviewed it and run
`./scripts/check.sh`.
* When you have addressed a PR comment, mark it as resolved.

Please be patient! I will review your PR, but my time is limited!
-->

* Closes <emilk#4742>
* [x] I have followed the instructions in the PR template
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is broken
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants