-
Notifications
You must be signed in to change notification settings - Fork 919
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
Implement ResizeObserver
#2859
Implement ResizeObserver
#2859
Conversation
0ea3249
to
288ebdb
Compare
93b34d4
to
ebc0fd7
Compare
I don't understand why that prevents converting between
So, I think it should be perfectly possible to convert between |
In my testing #2074 seems to work perfectly well with "normal" ctrl-+ browser zoom. |
ebc0fd7
to
1011441
Compare
2edcc81
to
70f9a9f
Compare
I rewrote large parts of the OP because of a misconception I had about what exactly CSS pixels represent. Thank you @Liamolucko for pointing it out to me in #2858 (comment).
Even after I rewrote the OP and cleared up any misconceptions I had, it's actually not possible to perfectly convert between So I've reworked the PR three times now, any review is appreciated. In the meantime we will be waiting for a new version of |
Can I ask some questions about using this from an end-user perspective? My usual way of interacting with Winit in browser is I create a window corresponding to a canvas (either full screen or in a layout) and then I draw to its entire extent with WebGL or WebGPU. Naively, if the users uses CTRL-+ or CTRL-MINUS on desktop browser I would expect to continue to be told my width and height in actual screen pixels. But naively, if the user is on mobile, and they pinch to zoom in, I think I'd expect to be treated like an image and for my output to simply be rescaled. If (as in this patch) in that circumstance you instead "do the right thing" and continue to report my updating pixel size continuously, that's a nice surprise— I guess that means I can just keep presenting ever-higher resolution versions of my imagery. But it creates a problem. If the user continues to zoom in on my window, very easily my window could wind up being several times the size of the screen. Mobile screens are already very high resolution. So resource costs to draw a frame could become very high, especially if I am doing multipass rendering (IE allocating an offscreen texture the size of the window— now potentially much larger than the screen), because I'm rendering lots of screen that is in fact offscreen. Besides just tracking WindowEvent::Resized, is there any hope of being informed what the "visible area" of my window is in this scenario? (I'm not sure if this is something CSS/JS even expose.) That way I could draw only in the visible part of the screen. Alternately, can I get the screen size (I think browser tech calls this the "viewport") in winit, so I can cap out there? |
You can use |
Yes, that's what this PR does.
That is how it currently works in Winit already, pinch-to-zoom has no effect on Winit at all.
There isn't a reliable way to detect pinch-to-zoom on Web. You could detect when two pointers are moving away or towards each other and use a bunch of hacks to calculate the zoom factor, but this isn't something really supported or reliable.
Winit has
I'm not sure what you mean with that, that's only useful if the canvas is bigger then the window, which sounds niche to me, but no, we don't have an API that returns that. In any case, sounds like an extended version of |
I had to redo the logic again to account for Safari not sending an initial resize event if the canvas has a size of zero. The PR is ready again. Still waiting for a new |
I am attempting to re-orient my test code, which was previously running on top of PR #2074 , to run on top of this. It appears to work exactly the same as #2074 (including ctrl-+ zooming— I have not tested on mobile as my app doesn't work on mobile). Two points of confusion for me:
|
Yep.
Yep, that was already the case before this PR. |
0eb1d3e
to
65c3439
Compare
With the latest I requested a review from @Liamolucko, if you want to review this please take your time, we are in no hurry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All looks good to me!
Your input was invaluable so far, thank you ❤️.
No worries, I'm just happy to see this finally coming to fruition. Thank you also for all your work on this! ❤️
Co-Authored-By: Liam Murphy <43807659+Liamolucko@users.noreply.github.com>
Co-Authored-By: Liam Murphy <43807659+Liamolucko@users.noreply.github.com>
65c3439
to
e2004eb
Compare
Problems
Accuracy
We have two different forms of accuracy here, which I will name "pixel perfect" and "precise".
"Pixel perfect" information about the size of the canvas is really important, see this blog post for more details, which includes a nice visual demo of the problems that can arise without such information.
But to summarize, the user absolutely requires this information to determine at what resolution to render to the canvas, otherwise they will get visual artifacts. This is important because we calculate physical pixels by converting CSS pixels with the scale factor, if the scale factor is fractional result, we have to round, which may or may not match the actual size in physical pixels on the users screen.
This is only a problem with fractional scale factors or if the canvas is positioned with fractional CSS pixel values.
In any case, the only way to accurately get information about the actual physical pixel size of the canvas is
devicePixelContentBoxSize
."Precise" describes the fact that some APIs report accurate CSS pixel values, while others just don't return fractional values. We want precise values to be able to accurately convert to physical pixels with the scale factor. Again, this is only a problem with fractional scale factors or if the canvas is positioned with fractional CSS pixel values.
Consistency between
Window::set_inner_size()
,Window::inner_size()
andWindowEvent::Resized
Preferably the size set by
set_inner_size()
should be the same as the one returned byinner_size()
and the one received by theWindowEvent::Resized
event. This is difficult because a lot of the APIs are inconsistent with each other or require imprecise conversions.To clarify why conversions are relevant here: there is no API to set the size by physical pixels in the Web, you can only set the size of elements in logical pixel units (plenty of other unit types, but they aren't relevant here, see MDN).
devicePixelContentBoxSize
can never be accurately converted to or from, because it reports pixel perfect physical sizes, any conversion can only round results. But being a pixel off might be an acceptable tradeoff.getBoundingClientRect()
respectstransform
, which would require significant effort on our side to calculate conversions to or from. See the MDN documentation for some examples whattransform
can do. So we will either have to document that usingtransform
might significantly alter the output or just refrain from using this API.Sync
Window::set_inner_size()
andWindow::inner_size()
Unfortunately
ResizeObserver
cannot be queried, we receive it's values only through callbacks. So we will have to decide if we would rather give the user inaccurate information throughWindow::inner_size()
until they receive aWindowEvent::Resized
or just not updateWindow::inner_size()
until we get accurate values, which the user will be notified of byWindowEvent::Resized
.API Overview
API: Name of the API.
queryable: Can be queried at any point in time, in contrast to having to wait for a callback.
transform
: Will adjust the reported size depending on anytransform
properties.precise: Reports CSS pixel values without rounding to the nearest integer.
pixel perfect: Reports the size of the element with exact physical pixels.
transform
devicePixelContentBoxSize
contentRect
1getBoundingClientRect()
getComputedStyle()
scrollWidth/Height
2API Analysis
We definitely want to use
devicePixelContentBoxSize
somehow. If we don't make it the value returned byWindow::inner_size()
andWindowEvent::Resized
, we could at least expose it throughWindowExtWebSys
.If we really want
Window::set_inner_size()
andWindow::inner_size()
/WindowEvent::Resized
to be consistent, we can usecontentRect
.Making sure that
Window::inner_size()
is able to return the correct/same value right after usingWindow::set_inner_size()
is problematic.getBoundingClientRect()
respectstransform
andscrollWidth/Height
is imprecise.Conclusion
I think using
devicePixelContentBoxSize
has the proper tradeoffs for Winit. We get pixel perfect sizes and I find the tradeoffs acceptable.We can treat
Window::set_inner_size()
as a request, so it isn't fulfilled untilWindowEvent::Resized
is received. It being "just a request", I can further extrapolate on saying that it's fine if the received size byWindow::inner_size()
/WindowEvent::Resized
isn't always the same as the one passed intoWindow::set_inner_size()
, which will commonly be the same anyway or at most a pixel off.Implementation
Changes
Window::set_inner_size()
or changesWindowEvent::ScaleFactorChanged::new_inner_size
.Window::inner_size()
now always returns the precise physical pixel size of the canvas, although not in sync withWindow::set_inner_size()
, which now is treated as a request only, see Return the applied size for synchroniousset_inner_size
#2868.ResizeObserver
s, that report the size back when needed. Therefore the resize and scale factor handlers are now merged, as they are closely intertwined.devicePixelContentBoxSize
, which had to be accounted for when scale factor changes, as that could affect the physical size without being reported bycontentRect
, the fallback todevicePixelContentBoxSize
.EventWrapper
type, that allows synchronous event execution without requiring global handlers. Now that resize and scale factor event handlers are merged, they can't be global, but have to be made per canvas.ResizeObserver
if the size of the element is zero, we had to do some internal size tracking to inhibit duplicateResized
events.New scale factor flow
When we notify the resize handler, it will re-observer the canvas with the
ResizeObserver
to get the initial event and get the size from there, if the browser supportsdevicePixelContentBoxSize
. If the browser doesn't, it will just usegetComputedStyle()
to calculate the inner size of the canvas without any callbacks involved.ScaleFactorChanged
to the user.new_inner_size
, we do nothing.new_inner_size
, we send aResized
event.new_inner_size
, we notify the resize handler again and reportResized
from there.Fixes #1661.
Fixes #2864.
Replaces #2074.
Footnotes
It is marked for deprecation in the future, but is the same as
contentBoxSize
. Used here because it's the only option inResizeObserverEntry
that is supported by Safari. ↩There is also
clientWidth/Height
, which doesn't include content hidden by a scrollbar andoffsetWidth/Height
, which is similar toclientWidth/Height
but includes borders. ↩