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

Make ViewportInPixels use integers, and clamp to bounds #3604

Merged
merged 3 commits into from
Nov 22, 2023

Conversation

Wumpf
Copy link
Collaborator

@Wumpf Wumpf commented Nov 21, 2023

In rare cases in can happen that the viewport returned by PaintCallbackInfo is outside the bounds of the screen.
for at least wgpu/webgpu in particular this is invalid usage, other backends might be affected as well.
Since this happened due to a float rounding error (in one repro case I had I got (width==1126.5625) + (offset=715.4376) = 1842.0001 for a resolution of 1842) I decided to do away with fractional values on the viewport alltogether. They technically make sense since a viewport is only specifying the NDC to pixel coordinate conversion, but practically this may lead to surprising sub-sampling issues.

…ewport that is guaranteed to stay inside the screen bounds
@Wumpf Wumpf added bug Something is broken epaint labels Nov 21, 2023
Copy link
Owner

@emilk emilk left a comment

Choose a reason for hiding this comment

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

Keeping in bounds makes sense, and rounding to integers makes sense too.
I’m not sure it should be epaints job, but since this will likely be a footgun for many backend, maybe it should.

There is a small detail of how to round.
We want two adjacent Rect (sharing an edge) to have non-overlapping viewports with no gap or overdraw. For that I think we need to round the edges (right/bottom) rather than the sizes (width/height).

Also, if epaint is always rounding, then ViewportInPixels should use i32 instead of f32

@emilk emilk merged commit f9f5db9 into master Nov 22, 2023
35 checks passed
@emilk emilk deleted the fix-fractional-wgpu-viewport branch November 22, 2023 20:01
@emilk emilk added egui_glow Relates to running egui_glow on native egui-wgpu labels Nov 23, 2023
@emilk emilk changed the title PaintCallbackInfo::viewport_in_pixels now guarantees to stay in bounds Make ViewportInPixels use integers, and clamp to bounds Nov 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is broken egui_glow Relates to running egui_glow on native egui-wgpu epaint
Projects
None yet
Development

Successfully merging this pull request may close these issues.

wgpu 0.18 warning spam when resizing window
2 participants