Make ViewportInPixels use integers, and clamp to bounds#3604
Merged
Conversation
…ewport that is guaranteed to stay inside the screen bounds
emilk
requested changes
Nov 22, 2023
Owner
emilk
left a comment
There was a problem hiding this comment.
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
approved these changes
Nov 22, 2023
ViewportInPixels use integers, and clamp to bounds
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In rare cases in can happen that the viewport returned by
PaintCallbackInfois 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.