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

Take clip_rect into account when storing widgets in interact_with_hovered #4017

Closed
rustbasic opened this issue Feb 10, 2024 · 8 comments · Fixed by #4020
Closed

Take clip_rect into account when storing widgets in interact_with_hovered #4017

rustbasic opened this issue Feb 10, 2024 · 8 comments · Fixed by #4020
Labels
bug Something is broken

Comments

@rustbasic
Copy link
Contributor

rustbasic commented Feb 10, 2024

@emilk

Update Issue : #4013

After update #4013, you will no longer be able to click any button on an image (in fact any button) that is not visible on screen when the image is enlarged.

If you read this post, please mark it with an eye emoji.


Reference :

May be,
Should we save the results of all events first?

egui/crates/egui/src/widgets/image.rs : fn ui(self, ui: &mut Ui)
337 line
let (rect, response) = ui.allocate_exact_size(ui_size, self.sense);
to
let (rect, response) = ui.allocate_exact_size(ui_size, egui::Sense::click_and_drag());

it's a good result.
But, One problem found in this case is that the Image in the ScrollArea cannot be dragged with the mouse after being enlarged.
then,

let (rect, response) = ui.allocate_exact_size(ui_size, egui::Sense::click_and_drag());
to
let (rect, response) = ui.allocate_exact_size(ui_size, egui::Sense::click());
it's a good result.

Or, Should it be this way?

First, save all information with click_and_drag().
Then, information is removed according to the Sense settings.
May be, Remove it from the interact() function
Please refer to the contents so far.

@rustbasic rustbasic added the bug Something is broken label Feb 10, 2024
@rustbasic
Copy link
Contributor Author

Should we use widget_rect & clip_rect ?

@emilk
Copy link
Owner

emilk commented Feb 10, 2024

I don't follow this at all. Is there a new bug?

Can you explain with an image or a video? Maybe some code to reproduce it?

And please write good titles. Put yourself in my choses reading this:

Screenshot 2024-02-10 at 12 34 33

@rustbasic
Copy link
Contributor Author

20240210-1
20240210-2

When you enlarge the displayed image, only part of the image is visible on the screen, but the image is recognized as displaying the entire screen.
So all buttons outside the image cannot be clicked.

In this case, the button doesn't even display hover.

@emilk
Copy link
Owner

emilk commented Feb 10, 2024

oh yeah, I forgot to take the clip_rect into account

@emilk emilk changed the title Update Issue : #4013 : sorry not 4006. Take clip_rect into account when storing widgets in interact_with_hovered Feb 10, 2024
@rustbasic
Copy link
Contributor Author

okay. sorry about that. i'm ugly english.

@emilk
Copy link
Owner

emilk commented Feb 10, 2024

Please test #4020

@rustbasic
Copy link
Contributor Author

rustbasic commented Feb 10, 2024

No. The same bug occurs.
Wait...
It may not have been updated yet, so I'll check again.

@rustbasic
Copy link
Contributor Author

rustbasic commented Feb 10, 2024

@emilk
Yes. It's Good.

There was no PR. I copied it and used it and it works well. If you send us a PR, I will test it further and let you know if there are any problems.

hello_world main.rs : should be excluded from PR.

emilk added a commit that referenced this issue Feb 10, 2024
* Bug introduced in #4013
* Closes #4017

Unfortunately this is a breaking change, since it changes the fields of
`Response`, so can't do a patch-release with this.
emilk added a commit that referenced this issue Feb 10, 2024
* Bug introduced in #4013
* Closes #4017

Unfortunately this is a breaking change, since it changes the fields of
`Response`, so can't do a patch-release with this.
hacknus pushed a commit to hacknus/egui that referenced this issue Oct 30, 2024
* Bug introduced in emilk#4013
* Closes emilk#4017

Unfortunately this is a breaking change, since it changes the fields of
`Response`, so can't do a patch-release with this.
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