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

Window contents can sometimes be affected by mouse interaction outside the window #4349

Closed
s-nie opened this issue Apr 11, 2024 · 7 comments · Fixed by #4611
Closed

Window contents can sometimes be affected by mouse interaction outside the window #4349

s-nie opened this issue Apr 11, 2024 · 7 comments · Fixed by #4611
Labels
bug Something is broken

Comments

@s-nie
Copy link
Contributor

s-nie commented Apr 11, 2024

egui version 0.27.2, also present on master

Describe the bug

Interactions outside a window can sometimes interact with elements inside the window if they allocate enough space.

To Reproduce

use eframe::{
    egui::{self, vec2},
    run_native, App,
};

struct TestApp;

impl App for TestApp {
    fn update(&mut self, ctx: &egui::Context, _frame: &mut eframe::Frame) {
        ctx.set_debug_on_hover(true);
        egui::Window::new("test")
            .scroll(true)
            .resizable(true)
            .title_bar(false)
            .show(ctx, |ui| {
                let _ = ui.allocate_space(vec2(1000.0, 100.0));
                ui.label("hello");
            });
    }
}

fn main() {
    run_native(
        "Test",
        Default::default(),
        Box::new(|_cc| Ok(Box::new(TestApp))),
    )
    .unwrap();
}

Expected behavior

The drag affects whatever is inside the window.

Screenshots

demo.mp4
@s-nie s-nie added the bug Something is broken label Apr 11, 2024
@rustbasic
Copy link
Contributor

rustbasic commented Apr 18, 2024

@emilk

It would be good for emilk to fix this.

This is a problem that has existed for a long time.

The area of egui::Window is recognized as large, so the buttons below that area cannot be clicked.

It will be easier to check the problem using the example below.

struct TestApp;

impl eframe::App for TestApp {
    fn update(&mut self, ctx: &egui::Context, _frame: &mut eframe::Frame) {
        egui::Window::new("test")
            .scroll(true)
            .resizable(true)
            .show(ctx, |ui| {
                ui.label("looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong");
                ui.collapsing(
                    "looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong",
                    |_ui| {},
                );
            });

        egui::CentralPanel::default().show(ctx, |ui| {
            ctx.options_mut(|options| {
                options.tessellation_options.debug_paint_clip_rects = true;
            });
            ctx.settings_ui(ui);
        });
    }
}

fn main() {
    let _result = eframe::run_native(
        "Test",
        Default::default(),
        Box::new(|_cc| Box::new(TestApp)),
    );
}

@s-nie
Copy link
Contributor Author

s-nie commented Jun 3, 2024

I have updated the code to reproduce the issue, turns out that all that is needed is something that allocates space within a window.

@s-nie
Copy link
Contributor Author

s-nie commented Jun 3, 2024

@emilk It seems that the interact_rect of widgets does not get clipped with the parent window. Resizing that window updates the widget's interact_rect, although it still ends up being a bit too large, as seen in the video.

@emilk
Copy link
Owner

emilk commented Jun 3, 2024

Yup, somewhere the clip_rect set by ScrollArea is not properly intersected with the widget rect to produce the interact_rect.

@emilk emilk added this to the Next Patch Release milestone Jun 3, 2024
@s-nie
Copy link
Contributor Author

s-nie commented Jun 5, 2024

I looked a bit more into it and couldn't find any reference to a clip_rect being set by ScrollArea. However it looks like the scroll area detects interactions before adding the content, i.e. before it knows in which area it should actually detect inputs:

// Drag contents to scroll (for touch screens mostly).
// We must do this BEFORE adding content to the `ScrollArea`,
// or we will steal input from the widgets we contain.
let content_response = ui.interact(inner_rect, id.with("area"), Sense::drag());

It does this based on the rectangle returned by available_frame_before_wrap here, which has the size defined by the last window resize operation, instead of the actual window size.

One fix that I could think of is to store the last content size and use it for the drag input detection on the next frame. Let me know if that would be an okay solution or if you have a different suggestion.

@emilk
Copy link
Owner

emilk commented Jun 5, 2024

I think you're right in your conclusion @s-nie. Storing the content size or inner_rect from the previous frame in scroll_area::State would work, for sure.

Another way to do it is to leverage Context::read_response. You can replace the call to

// (NOTE: `inner_rect` here is a bad estimate, which is what is causing the bug)
let content_response = ui.interact(inner_rect, id.with("area"), Sense::drag());

With

let drag_area_id = id.with("area");
let content_response = ui.ctx().read_response(drag_area_id);

And then at the end of Prepared::end add

// (NOTE: `inner_rect` here is the _actual_, correct final rect of the scroll area)
// Result ignored, but read at the start of next frame with `read_response`.
let _ = ui.interact(inner_rect, drag_area_id, Sense::drag());

Maybe this could be easier, maybe not.

@s-nie
Copy link
Contributor Author

s-nie commented Jun 5, 2024

Another way to do it is to leverage Context::read_response.

That worked great! I'll open a PR.

@emilk emilk closed this as completed in 1f008fb Jun 6, 2024
emilk added a commit that referenced this issue Jun 19, 2024
This fixes a bug which sometimes would make it possible to interact with
widgets that were outside the parent clip_rect.

Interaction with a widget is done with the `interact_rect`, which is the
intersection of the widget rect and the parent clip rect. If these
rectangles are disjoint (the widget is outside the parent clip rect),
this results in a _negative rectangle_ (a rectangle with a negative
width and/or height). The distance tests for negative rectangles were
broken, causing the bug.

* This is part of solving #4475
* It is also likely this would have solved
#4349 (which now has another fix for
it)


### Breaking changes
`Rect::distance_to_pos`, `distance_sq_to_pos`, `signed_distance_to_pos`
now all return `f32::INFINITY` if the rectangle is negative.
valadaptive pushed a commit to valadaptive/egui that referenced this issue Jun 20, 2024
This fixes a bug which sometimes would make it possible to interact with
widgets that were outside the parent clip_rect.

Interaction with a widget is done with the `interact_rect`, which is the
intersection of the widget rect and the parent clip rect. If these
rectangles are disjoint (the widget is outside the parent clip rect),
this results in a _negative rectangle_ (a rectangle with a negative
width and/or height). The distance tests for negative rectangles were
broken, causing the bug.

* This is part of solving emilk#4475
* It is also likely this would have solved
emilk#4349 (which now has another fix for
it)


### Breaking changes
`Rect::distance_to_pos`, `distance_sq_to_pos`, `signed_distance_to_pos`
now all return `f32::INFINITY` if the rectangle is negative.
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
3 participants