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 · 8 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 · 8 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)),
    );
}

@rustbasic
Copy link
Contributor

rustbasic commented Apr 20, 2024

@emilk

I will solve this and submit a Pull Request, soon.

@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.

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