w3c / csswg-drafts

CSS Working Group Editor Drafts
https://drafts.csswg.org/
Other
4.5k stars 668 forks source link

[cssom-view] No (easy) way to access the viewport size without losing precision. #5260

Open emilio opened 4 years ago

emilio commented 4 years ago

Related to https://github.com/w3c/csswg-drafts/issues/5259.

I recently caused a compat issue in Firefox for making the layout viewport size more often a subpixel size. Here are the gory details, which amount basically to people making layout decisions based on window.innerWidth, window.innerHeight and so on. That seems a totally reasonable thing to do.

But turns out you can't do it in a sound way (the original page has issues in all browsers, just a different zoom levels / resolutions / etc). Well, I guess technically you can call getBoundingClientRect() on a position: fixed; top: 0; left: 0; right: 0; bottom: 0 element, but that seems rather hackish.

So, I think we should either:

Is there any context I'm missing? If not and people agree that this is better than the status quo, I'd be happy to experiment with this in Firefox and see what the compat impact might be.

frivoal commented 4 years ago

This is also related to https://github.com/w3c/csswg-drafts/issues/4713, where we concluded the same thing: either make the existing API return doubles, or look into making new ones that do.

mrdoob commented 4 years ago

Another workaround seems to be this:

const dpr = window.devicePixelRatio;
const width = Math.floor( window.innerWidth * dpr ) / dpr;
const height = Math.floor( window.innerHeight * dpr ) / dpr;

But I vote for turning these into doubles too.

emilio commented 4 years ago

Related: https://lists.w3.org/Archives/Public/www-style/2015Feb/0195.html

emilio commented 4 years ago

(And 4e888a204c)

chrishtr commented 4 years ago

@bokand

css-meeting-bot commented 4 years ago

The CSS Working Group just discussed [cssom-view] No way to access the viewport size without losing precision., and agreed to the following:

The full IRC log of that discussion <dael> Topic: [cssom-view] No way to access the viewport size without losing precision.
<dael> github: https://github.com/w3c/csswg-drafts/issues/5260
<dael> emilio: window.innerheight and width are lengths. Bad because you want viewport width, esp for fractional widths. No work arounds.
<dael> emilio: I know there's history here and that some of OM prep remained as long intentionally but I don't think this was one.
<dael> emilio: I think we should be able to change
<dael> smfr: Sounds fraught with compat risk. Will people parse int on inner height?
<dael> emilio: This came from slight change to FF UI that added a half pixel to UI which reduced viewport. A bunch of sites broke. I think this fixes more, esp for uses that use zoom-like things to change viewport size.
<dael> emilio: You can always parse it. Right now people set .width to value of innerWidth and that leaves seams when on fractional dpi context
<Rossen_> q?
<dael> emilio: Agree there is risk
<dael> emilio: If people think this is not a bad idea I can try to change gecko and see if other engines will follow. If you're convinced it can't happen no point to try
<dael> smfr: Not convinced but when we tried with other metrics it was a problem. Other option is new APIs that return doubles. Happy for you to try and if it works I can try on WK. Needs extended in the wild tests
<dael> Rossen_: We tried something similar in I9 days. We exposed a bunch of OM and I believe this is one we changed because a lot of problems. I remember fighting with cnn.com which was fighting to try and readjust and avoid scrollbars.
<dael> Rossen_: At that time we could confuse such a large property so I'm not sure what would be there for small end of the web. I would not underestimate compat risk
<dael> emilio: But inner width and height don't depend on scrollbars. Not sure it can be the case. But sure.
<Rossen_> s/I9/IE9/
<dael> Rossen_: That's the feedback for now. If we proceed another point in issue from florian where we took a no change for media width and height. The issue is linked
<dael> Rossen_: We can follow that
<dael> emilio: That was very much the opposite
<dael> emilio: afaict the proposal in the other was rounding for MQ, right?
<dael> florian: afaict what this means in MQ stays what they are b/c make sense that way on OM stays because compat. End result is inconsitent. If we want consistant we have to break one
<dael> Rossen_: Do we favor the risk or the similar. It was the argument of sticking with compat
<dael> emilio: Fair. I think I'm still going to give it a shot. Nightly or beta to see if there's reckage. If there is I'll prop a new API. If that sounds good I don't need resolution
<dael> Rossen_: resolve no change, emilio experiments?
<dael> emilio: Agree no change for now. Spec shouldn't change w/o proof change is compat
<dael> florian: we're not saying it's the end of the story. I don't htink we should close, we should resolve you're investigating
<dael> Rossen_: Closing and resolving are different. If there's more information with strong suggestion in opposite we revisit
<dael> Rossen_: Obj to resolve no change?
<dael> RESOLVED: No change
bokand commented 4 years ago

FWIW, the issue that led to #4713 (https://crbug.com/1043456) was caused by Blink trying to make MQ truncate viewport dimensions like innerWidth is, since the latter is (often?) used in media queries. The change to make the MQ compare against a subpixel width made it to stable and the above bug was the only one I saw but we did revert it in the next milestone. Looking back at it, it's not even clear to me the behavior is unexpected or broke any production sites.

I'm curious to know if we could make viewport dimensions subpixel. I think it would simplify cases like the above.

emilio commented 4 years ago

Yes, the resolution above is "No change, pending Emilio trying to do it in Gecko (on Nightly / Beta) and reporting back if there's breakage or not".

mrdoob commented 4 years ago

I thought I would give more background to this issue...

Historically, many canvas/webgl projects configured the canvas element size like this:

canvas.width = window.innerWidth;
canvas.height = window.innerHeight;

When window.devicePixelRatio was introduced, this pattern was used to render at the physical resolution:

canvas.width = window.innerWidth * window.devicePixelRatio;
canvas.height = window.innerHeight * window.devicePixelRatio;
canvas.style.width = window.innerWidth + 'px';
canvas.style.height = window.innerHeight + 'px';

This worked fine until devices started to use non-integer DPRs. In devices with DPRs such as 1.25, 2.75, ... we can end up with canvas elements that are bigger than the viewport (forcing the scrollbar to show up if the developer didn't set overflow: hidden ).

I've made a demo to illustrate this: https://devicepixelratio.glitch.me/

The demo shows the (virtual) resolution provided by innerWidth and innerHeight and the computed physical resolution by taking dpr into account. I've also added an edge to edge canvas using that information and it draws a 1 pixel blue border inside:

Screen Shot 2020-11-12 at 11 06 31 AM

Things break when DPRs aren't integers.

For example, Google's PixelBook Go has a DPR of 1.25 and depending on the window size the canvas element is bigger than the viewport so the outline gets clipped:

Screenshot 2020-11-12 at 11 14 51

Google's Pixel 4a has a DPR of 2.75 and we see the same issue. In this case we know that the device's physical resolution is 1080px width. If we divide 1080 by 2.75 we end up with 392.72px which would be the value of window.innerWidth if it was not an integer.

Screenshot_20201112-111237

It's worth noting that this doesn't seem to be an issue on Mac/iOS devices because, as far as I can see, all the devices have integer DPRs (1.0, 2.0, 3.0).

emilio commented 4 years ago

https://bugzilla.mozilla.org/show_bug.cgi?id=1676843 has a patch to give this a shot.

mrdoob commented 4 years ago

Another interesting thing I just found out is that MacOS doesn't seem to support resizing windows to subpixel sizes. Does anyone know if this is the case too on Windows and/or Linux?

emilio commented 4 years ago

I'm moderately sure that all windowing systems work on device pixels, so you shouldn't be able to resize under 1 device pixel if that's what you mean.

mrdoob commented 4 years ago

Using the terms we're using here, MacOS seems to be using "css pixels" when resizing windows. If I resize a window, I can see the cursor moving, but the window only resizes when I reach the next "css pixel".

chrishtr commented 4 years ago

I think the use case @mrdoob explained above (sizing a canvas to be fullscreen) can also be solved via the existing ResizeObserver feature to observe pixel-snapped rects.

emilio commented 4 years ago

Ok, the patch above landed and next day I got two regressions in major web properties (WhatsApp and YouTube), so I don't think this is going to fly compat-wise unfortunately.

Unless there's a strong objection against it I think a new API is the way to go if we want to expose this. I'd go for innerWidthDouble and innerHeightDouble, wdyt?

heycam commented 4 years ago

I'd go for innerWidthDouble and innerHeightDouble, wdyt?

I don't have anything better to suggest right now, but one concern with these names is that "double" isn't a JS concept so it might feel strange to be referring to some value as a double in script.

mrdoob commented 4 years ago

What about properties that gives the physical pixels instead?

const cssWidth = window.innerPhysicalWidth / window.devicePixelRatio;
emilio commented 4 years ago

We generally don't expose actual device pixels in APIs, I think (but you could compute the device pixel width by multiplying by devicePixelRatio...)

emilio commented 5 months ago

https://github.com/w3c/csswg-drafts/issues/9237 just resolved to create a new object for layout related bits. It seems that's a good opportunity to make something like window.layoutViewport.{width,height} doubles, thus maybe fixing this? cc @bramus

bramus commented 5 months ago

It seems that's a good opportunity to make something like window.layoutViewport.{width,height} doubles, thus maybe fixing this?

YES!

css-meeting-bot commented 4 months ago

The CSS Working Group just discussed [cssom-view] No way to access the viewport size without losing precision., and agreed to the following:

The full IRC log of that discussion <TabAtkins> emilio: we've discussed in the past, making innerWidth and innerHeight not round is not compatible, it breaks things
<TabAtkins> emilio: but we recently decided to add an object to the Window that represents the layout viewport (mainly for segments stuff)
<TabAtkins> emilio: But it sounds like a good place to expose the full double-precision viewport dimensions
<TabAtkins> emilio: So they'll do the same as innerHeight/Width, but without the weird rounding
<flackr> +1
<TabAtkins> emilio: Proposal is to add ... unsure if we decided it to be window.viewport or window.layoutViewport, but whatever, add .width and .height
<TabAtkins> astearns: Idle thought that maybe this should have a slightly different name to indicate it shouldn't be rounded, but nm, that sounds awful
<TabAtkins> emilio: Before we had this new object, best proposal i could come up with was .innerWidthDouble or .innerWidthUnrounded
<TabAtkins> emilio: but those are bad
<TabAtkins> emilio: MDN and the spec could have a note about the difference
<TabAtkins> flackr: Another bad alternative would be to have a gBCR() api on one of these objects, those also return doubles
<TabAtkins> emilio: Right, but top and left would be 0 always
<TabAtkins> flackr: You *could* imagine them being the scroll position...
<TabAtkins> astearns: that sounds worse
<TabAtkins> emilio: There's other issues to expose the other layout viewport things (small/big/etc)
<TabAtkins> emilio: But I think .width and .height should do the right thing.
<TabAtkins> emilio: And consider other names for the small/large viewport sizes
<TabAtkins> astearns: Anyone remember if it's .viewport or .layoutViewport?
<TabAtkins> emilio: I think we punted on the name in the preivous call
<TabAtkins> astearns: So proposal is to add .width and .height as doubles to the layout viewport interface
<TabAtkins> RESOLVED: add .width and .height as doubles to the layout viewport interface
<emilio> TabAtkins: when we have these box sizes, I'm always confused about whether they have scrollbars or not, do we need variants to account for that?
<TabAtkins> emilio: I think right now the way to do that is document.scrollingElement.scrollWidth, or clientWidth
<TabAtkins> emilio: Which I agree isn't great
<TabAtkins> emilio: I'm also not sure if those round or not off the top of my head
<TabAtkins> emilio: but gBCR() gets around it
<TabAtkins> emilio: we shoudl have a separate issue
<TabAtkins> TabAtkins: agreed
bramus commented 4 months ago

(Apologies for the short drive-by comment. I am currently OOO.)

Another thing authors also want to know beforehand are the width/height of the small and large viewports.

I think we have a separate issue for that but if not, then maybe we could add large and small sub-objects that also have width and height properties?

@emilio @tabatkins

emilio commented 4 months ago

@bramus yeah that was mentioned on the minutes, that's https://github.com/w3c/csswg-drafts/issues/8709