Ensure same intersection bounds are always used #218
Conversation
puffin_egui/src/lib.rs
Outdated
| @@ -689,7 +689,7 @@ impl ProfilerUi { | |||
| }; | |||
|
|
|||
| let frame_rect = Rect::from_min_max( | |||
| Pos2::new(x, rect.top()), | |||
| Pos2::new(x - 0.5 * frame_spacing, rect.top() + 0.5 * frame_spacing), | |||
| Pos2::new(x + frame_width, rect.bottom()), | |||
| ); | |||
There was a problem hiding this comment.
Could maybe move the expand2 up to here, like:
| ); | |
| ) | |
| .expand2(vec2(0.5 * frame_spacing, 0.0)) |
but inlining the expansion is probably fine too
There was a problem hiding this comment.
You mean instead of doing the subtraction and addition on line 692?
There was a problem hiding this comment.
right - my impression was that this inlined the equivalent of doing the .expand2 which was removed where checking if the mouse_pos is contained in the frame.
There was a problem hiding this comment.
Yeah I can change that, I also noticed the addition is wrong as it adds to the Y component of the min position. So changing to .expand2 seems like a good solution 👍
There was a problem hiding this comment.
but on second glance maybe not - this now just expands the min position not the max position, so expand2 wouldn't be equivalent.
There was a problem hiding this comment.
The function not expanding max is what's wrong that I mentioned in my comment above, I'm fixing that as well.
Edit: It should expand both min and max. And when rendering the frame rect should be shrunken to not create a solid chunk of rects. (Something I also noticed now with correct expansion)
There was a problem hiding this comment.
yeah, I realized afterwards that the bars were getting filled out visually if I tried using expand2 up-front and yeah the follow up to reduce the visual rect makes sense
Hoodad
changed the title
Checklist
Description of Changes
This ensures the bounds for a
frame_rectis always the same when checking for intersection in puffin_egui.Also removed unnecessary call to
response.hovered().Related Issues
Fixes #217