Skip to content

Conversation

hydra
Copy link

@hydra hydra commented May 12, 2025

  • Note: the original API accepted a Vec2b, but didn't use the x/y properties independently, thus this is a non-breaking change and should probably be considered a bug-fix.

Video:

Recording.2025-05-12.211409.mp4

Fix (before and after):

Recording.2025-07-14.133811.mp4
  • I have followed the instructions in the PR template

@hydra hydra marked this pull request as ready for review May 12, 2025 19:24
@hydra
Copy link
Author

hydra commented May 13, 2025

Here's a video of the Resize control in combination with a table, using taffy for layout:

Recording.2025-05-13.091334.mp4

source: https://github.com/MakerPnP/makerpnp/blob/b26be8c4bc73bae1cd06011e6f4a30b793fe0f29/crates/planner_gui_egui/src/project/unit_assignments_tab.rs

This video also highlights a usability issue, where it's hard to resize a table due to the scrollbars. It's hard to find the resize handle; you have to hover around until you see the pointer change. not all of the visible area of the resize handle actually works for resizing.

@michalsustr
Copy link

A nice cool improvement would be also to change the cursor to be only horizontal/vertical/diagonal based on the available x/y/x&y

Copy link

Preview available at https://egui-pr-preview.github.io/pr/7047-resize-improvements-1
Note that it might take a couple seconds for the update to show up after the preview_build workflow has completed.

Comment on lines 255 to 260
if self.resizable.x {
state.desired_size.x = user_requested_size.x;
}
if self.resizable.y {
state.desired_size.y = user_requested_size.y;
}
Copy link
Owner

Choose a reason for hiding this comment

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

AFAICT this code path is only hit by Window, which should already respect resizable. Is this really changing any behavior in practice? Or is the PR just about demonstrating an existing behavior in the demo?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, it's definately changing behavior in practice.
It didn't work for me, so I fixed it and made this PR as a result.
The demo exists to prove the fix works.

Updated original comment with demo of the existing behavor and with the fix.

@hydra
Copy link
Author

hydra commented Jul 14, 2025

Sorry for late response to your question (#7047 (comment))

Rebased on 0.32.0, fix is still required, video added to original comment.

@hydra hydra force-pushed the resize-improvements-1 branch from 3299adf to 56eb753 Compare July 14, 2025 11:42
hydra added 2 commits July 14, 2025 13:48
…g checkboxes to control X/Y axis resizing.

* Note: the original API accepted a `Vec2b`, but didn't use the x/y properties independently.

# Conflicts:
#	crates/egui_demo_lib/src/demo/misc_demo_window.rs
@hydra hydra force-pushed the resize-improvements-1 branch from 56eb753 to 09b8818 Compare July 14, 2025 11:49
@hydra
Copy link
Author

hydra commented Jul 14, 2025

@emilk I just discovered this PR has unintended consequences, I'm not sure of the correct direction to go.

when this PR is enabled, a resize inside a container works as per the demo, however Windows themselves are no-longer resizable, probably this is because the resize for the window is being created with a Resize::resizable member set to [false, false] instead of [true, true] since the old code didn't check the value of the Resize::resizable field and just blindly overwrote ressizable with the user_requested_size.

I feel this PR is the correct fix, but that other fixes are also required.

Guidance appreciated.

@hydra
Copy link
Author

hydra commented Jul 14, 2025

The smoking gun is this, in Window::show_dyn.

let resize = resize.resizable(false); // We resize it manually

Going to investigate a little more...

I found this hack in Window::resize_response()

    if resize_interaction.any_dragged() {
        if let Some(mut state) = resize::State::load(ctx, resize_id) {
            state.requested_size = Some(new_rect.size() - margins);
            state.store(ctx, resize_id);
        }
    }

It really doesn't feel right that the Window is loading the state from Resize and fiddling with it, IMHO that should be the sole responsibility of the code in the resize resize.rs via an API.

So, basically, even though there is a resize that has resizable = [false, false] the hack comes along and updates the internal state of the window's resize. when the hack in Window is combined with the fix in this PR to the resize for windows is thus broken.

AFAICT this code path is only hit by Window

it turns out that is NOT the case, there are two things that update the user_requested_size, a) the window, by the hack above. b) the corner handle, by this chunk of code in Resize::begin.

        let mut user_requested_size = state.requested_size.take();

        let corner_id = self.resizable.any().then(|| id.with("__resize_corner"));

        if let Some(corner_id) = corner_id {
            if let Some(corner_response) = ui.ctx().read_response(corner_id) {
                if let Some(pointer_pos) = corner_response.interact_pointer_pos() {
                    // Respond to the interaction early to avoid frame delay.
                    user_requested_size =
                        Some(pointer_pos - position + 0.5 * corner_response.rect.size()); <-- this line
                }
            }
        }

@hydra
Copy link
Author

hydra commented Jul 14, 2025

I added a commit, which avoid the can-of-worms Resize <-> Window state issue which seems to work. I made a short video about the issue, my thoughts on the Resize <-> Window state, the new approach taken in the latest commit and shows the egui_demo_app and updated resize demo in action along with verification that windows resize correctly still too.

Watch the video

https://www.youtube.com/watch?v=p_QwInM-D4o

hydra added a commit to MakerPnP/makerpnp that referenced this pull request Jul 14, 2025
…ocess externally modified state (in `Window` before applying corner resizing). Only apply resize axis constrains to corner resizing.
@hydra hydra force-pushed the resize-improvements-1 branch from 97b71cb to f0c4130 Compare August 4, 2025 07:44
@hydra hydra requested a review from emilk September 10, 2025 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants