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

[Clarification] px in sizes in relation to pixel density #7580

Closed
strarsis opened this issue Feb 3, 2022 · 10 comments
Closed

[Clarification] px in sizes in relation to pixel density #7580

strarsis opened this issue Feb 3, 2022 · 10 comments

Comments

@strarsis
Copy link

strarsis commented Feb 3, 2022

In some cases the img sizes attribute value may be specified in pixels (px). For example, under a specific breakpoint/media query the image element may have a static width.

During manual browser testing I noticed that the absolute width in pixels used in sizes is not interpreted as CSS pixels (1x density), but rather as absolute pixels.
The browser will pick an image source that is always closest or equal to that given width of pixels, regardless of pixel density.
That means that e.g a 100px sizes will result in a 100px image source for a 1x dpi screen, 2x dpi screen, 3x dpi screen and so on, and not in a 200px image source for a 2x dpi screen and a 300px image source for a 3x dpi screen.

Is this behavior intended? So for a static sizes in px extra media queries would need to be used, one for each possible pixel density with its own pre-multiplied pixel value, beyond the standard 1x?

@tabatkins
Copy link
Contributor

Can you provide some example source code to illustrate exactly what you're having a problem with?

@zcorpan
Copy link
Member

zcorpan commented Feb 4, 2022

I'm not able to reproduce what you described -- with this demo, with devicePixelRatio being 2, Chrome, Safari and Firefox load the 200w candidate:

http://software.hixie.ch/utilities/js/live-dom-viewer/saved/10026

<!DOCTYPE html>
<img srcset="image?100 100w, image?200 200w, image?300 300w" sizes="100px" onload="w(currentSrc)">
<script>w(devicePixelRatio)</script>

@zcorpan
Copy link
Member

zcorpan commented Feb 4, 2022

Note that browsers are free to decide how they choose a candidate from srcset, and this can differ a bit between browsers. For example, they can have a maximum density so that they don't load overly large images, or have some heuristic to decide what to load when the calculated density is in between two candidates, or a "save data" user setting is enabled, etc.

Here's the current logic in chromium: https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/html/parser/html_srcset_parser.cc;l=423

@strarsis
Copy link
Author

strarsis commented Feb 4, 2022

@zcorpan:

Could it be an issue/missing implementation in the mobile emulation of Chrome Dev tools?

See this minimal example:
https://codepen.io/strarsis/pen/XWzjRWM
https://codepen.io/strarsis/full/XWzjRWM (full view)

  1. Open the minimal example page or another example page that has a srcset with image sources and a static sizes (in px), like the minimal example.
  2. Open the Chrome Dev tools.
  3. Enable the mobile emulator (mobile/tablet 2nd icon in upper left corner of the Dev Tools panel/window).
  4. In mobile emulator options bar at the top switch Dimensions to Responsive, then DPR (Device pixel ratio) can be manually toggled.
  5. Toggle the DPR between different steps between 1x and 3x. Note that the image source doesn't change, regardless of DPR. Reloading the page with a different DPR doesn't update the image source either.

@zcorpan
Copy link
Member

zcorpan commented Feb 4, 2022

Yeah, it's possible the emulated DPR in devtools isn't honored by srcset selection.

I found this issue, not sure if it's the same: https://bugs.chromium.org/p/chromium/issues/detail?id=1131815 cc @mathiasbynens (edit: hmm it seems to be a feature request)

@zcorpan
Copy link
Member

zcorpan commented Feb 4, 2022

I believe this isn't an issue with the spec, so closing.

@strarsis
Copy link
Author

strarsis commented Feb 4, 2022

@zcorpan: So it works correctly in native browsers, just not in Dev tools? Then this is a dev tools/mobile emulator bug in Chrome.
Thanks for clarifying!

@zcorpan
Copy link
Member

zcorpan commented Feb 4, 2022

Yeah, at least I couldn't get the image to change to the smaller one (with Disable cache on) with a custom emulated device with DPR of 1. (I don't see an option to edit DPR for Responsive.) You can report it here: https://crbug.com/new

It responds to page zoom: View -> zoom out, then reload with devtools open (with Disable cache on), I get the 400x400 image. Zooming in it loads a bigger image.

@strarsis
Copy link
Author

strarsis commented Feb 4, 2022

@zcorpan: Yes, I can confirm that it works on native Chrome but not on Chrome Dev Tools Mobile emulator.

As you recommended I will report this issue as a new bug in the mobile emulator of Chrome Dev Tools.

@strarsis
Copy link
Author

strarsis commented Feb 4, 2022

@zcorpan: I created a new issue for Chrome/Chromium: https://bugs.chromium.org/p/chromium/issues/detail?id=1294293

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants