cloudinary / cloudinary_npm

Cloudinary NPM for node.js integration
629 stars 323 forks source link

Device Pixel Ratio (DPR) support for Picture tag #469

Open ryanmeador opened 3 years ago

ryanmeador commented 3 years ago

Feature request for Cloudinary_NPM SDK

Explain your use case

I'd like to be able to automatically generate source variants based on DPR inside a picture tag. Currently, we can use cloudinary.picture() to generate a picture tag, but there doesn't seem to be a way to factor DPR into either the srcset URLs nor the media query.

Describe the problem you’re trying to solve

High-resolution screens, such as those on many Apple products, require higher-resolution images to display at full quality than the size in CSS pixels would suggest. There are (at least) two mechanisms in HTML and CSS to solve this: the srcset attribute of source (or img) tags can specify multiple URLs (each tagged with the pixel density), or you can use the min-resolution CSS media query. I'd like to be able to generate picture tags that look like these:

srcset example

<picture>
  <source
    media="(max-width: 600px)"
    srcset="https://res.cloudinary.com/mycloud/image/upload/q_auto/f_auto/w_500/v1/example-image,
            https://res.cloudinary.com/mycloud/image/upload/q_auto/f_auto/dpr_2,w_500/v1/example-image 2x">
  <source
    media="(max-width: 1500px)"
    srcset="https://res.cloudinary.com/mycloud/image/upload/q_auto/f_auto/w_400/v1/example-image,
            https://res.cloudinary.com/mycloud/image/upload/q_auto/f_auto/dpr_2,w_400/v1/example-image 2x">
  <source
    srcset="https://res.cloudinary.com/mycloud/image/upload/q_auto/f_auto/w_500/v1/example-image,
            https://res.cloudinary.com/mycloud/image/upload/q_auto/f_auto/dpr_2,w_500/v1/example-image 2x">
  <img
    src="https://res.cloudinary.com/mycloud/image/upload/q_auto/f_auto/v1/example-image">
</picture>

media query example

<picture>
  <source
    media="(max-width: 600px) and (min-resolution: 2dppx)"
    srcset="https://res.cloudinary.com/mycloud/image/upload/q_auto/f_auto/dpr_2,w_500/v1/example-image">
  <source
    media="(max-width: 600px)"
    srcset="https://res.cloudinary.com/mycloud/image/upload/q_auto/f_auto/w_500/v1/example-image">
  <source
    media="(max-width: 1500px) and (min-resolution: 2dppx)"
    srcset="https://res.cloudinary.com/mycloud/image/upload/q_auto/f_auto/dpr_2,w_400/v1/example-image">
  <source
    media="(max-width: 1500px)"
    srcset="https://res.cloudinary.com/mycloud/image/upload/q_auto/f_auto/w_400/v1/example-image">
  <source
    media="(min-resolution: 2dppx)"
    srcset="https://res.cloudinary.com/mycloud/image/upload/q_auto/f_auto/dpr_2,w_500/v1/example-image">
  <source
    srcset="https://res.cloudinary.com/mycloud/image/upload/q_auto/f_auto/w_500/v1/example-image">
  <img
    src="https://res.cloudinary.com/mycloud/image/upload/q_auto/f_auto/v1/example-image">
</picture>

As far as I can tell, neither of these are possible to generate using the options to the picture function, invoked like this on my Node backend:

cloudinary.picture('example-image', {
  fetch_format: 'auto',
  transformation: { quality: 'auto' },
  sources: [
    { max_width: 600, transformation: { width: 500 } },
    { max_width: 1500, transformation: { width: 400 } },
    { transformation: { width: 500 } }
  ]});

Do you have a proposed solution?

I propose we could generate the first one with a new attribute like this:

cloudinary.picture('example-image', {
  fetch_format: 'auto',
  transformation: { quality: 'auto' },
  dpr: [1, 2],
  sources: [
    { max_width: 600, transformation: { width: 500 } },
    { max_width: 1500, transformation: { width: 400 } },
    { transformation: { width: 500 } }
  ]});

Or generate the second one with something like this:

cloudinary.picture('example-image', {
  fetch_format: 'auto',
  transformation: { quality: 'auto' },
  sources: [
    { max_width: 600, min_resolution: "2dppx", transformation: { width: 500, dpr: 2 } },
    { max_width: 600, transformation: { width: 500 } },
    { max_width: 1500, min_resolution: "2dppx", transformation: { width: 400, dpr: 2 } },
    { max_width: 1500, transformation: { width: 400 } },
    { transformation: { width: 500 } }
  ]});

The second proposal more closely matches the existing interface of the picture function, is more flexible, and is probably easier to implement, but it requires repeating the DPR twice in each source specification (once as min_resolution and once as dpr in the transformation). In my mind, the entire point of this library is to save us from tedious and error-prone boilerplate, so I don't like having to repeat that. Thus, I think the first proposal is probably the better choice.

Is there any plan to implement a solution like this? If not, would you accept a PR that implements one of these? Thanks!

igy commented 3 years ago

Hi Ryan,

I'm not aware of any current plans to change how the picture() method works and I believe we currently only support the min_width and max_width selectors , but I'd be happy to forward this to the SDK team for consideration as a feature request.

That said, I may be missing something about your requirements and have a follow-up question if you don't mind?

In your example, are you intending to load a completely different image (or different transformation options) on screens with higher device pixel ratios, or just to use larger dimensions with larger DPR screens either by explicitly changing the width and height, adding the dpr transformation to multiply the existing width and height provided?

I ask because if the same image is used each time and only the dimensions will change, my understanding is that browsers will automatically take the device's pixel ratio into consideration when looking at a simple srcset, and will implicitly multiply the 'normal' CSS dimensions by the DPR when choosing which image to use - am I not correct about that, or is there another reason to be more explicit with the media query?

Or to ask another way, is there a practical difference between :

<source
    media="(max-width: 600px) and (min-resolution: 2dppx)"
    srcset="https://res.cloudinary.com/mycloud/image/upload/q_auto/f_auto/dpr_2,w_500/v1/example-image">
  <source
    media="(max-width: 600px)"
    srcset="https://res.cloudinary.com/mycloud/image/upload/q_auto/f_auto/w_500/v1/example-image">
....

and

<source
    media="(max-width: 600px) 
    srcset="https://res.cloudinary.com/mycloud/image/upload/q_auto/f_auto/w_500/v1/example-image">
  <source
    media="(max-width: 1200px)"
    srcset="https://res.cloudinary.com/mycloud/image/upload/q_auto/f_auto/dpr_2,w_500/v1/example-image">
  <source

?

ryanmeador commented 3 years ago

Hi igy,

As far as I know, the CSS px unit is device independent (MDN agrees). I don't think a browser can apply the logic you're suggesting without breaking a lot of stuff; the pixel has to have the same definition at the layout step as it does when evaluating the picture tag so that you can use it for art direction. Just to be sure, I ran a test:

<html>
  <head>
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
  </head>
  <body>
    <picture>
      <source
        media="(max-width: 600px) and (min-resolution: 2dppx)"
        srcset="https://res.cloudinary.com/hfiohoooe/image/upload/q_auto/f_auto/dpr_2,w_500/v1/models/amel-super-maramu">
      <source
        media="(max-width: 600px)"
        srcset="https://res.cloudinary.com/hfiohoooe/image/upload/q_auto/f_auto/w_500/v1/models/amel-super-maramu">
      <source
        srcset="https://res.cloudinary.com/hfiohoooe/image/upload/q_auto/f_auto/w_100/v1/models/amel-super-maramu">
      <img
        src="https://res.cloudinary.com/hfiohoooe/image/upload/q_auto/f_auto/v1/models/amel-super-maramu" style="width: 500px">
    </picture>
    <picture>
      <source
        media="(max-width: 600px)"
        srcset="https://res.cloudinary.com/hfiohoooe/image/upload/q_auto/f_auto/w_500/v1/models/amel-super-maramu">
      <source
        media="(max-width: 1200px)"
        srcset="https://res.cloudinary.com/hfiohoooe/image/upload/q_auto/f_auto/dpr_2,w_500/v1/models/amel-super-maramu">
      <source
        srcset="https://res.cloudinary.com/hfiohoooe/image/upload/q_auto/f_auto/w_100/v1/models/amel-super-maramu">
      <img
        src="https://res.cloudinary.com/hfiohoooe/image/upload/q_auto/f_auto/v1/models/amel-super-maramu" style="width: 500px">
    </picture>
  </body>
</html>

On my MacBook running Chrome 87:

If the browser was accounting for pixel density, I would expect the second picture tag to load the 1000 pixel image when the viewport is less than 600px wide (because it would be looking at the 1200px media query).

If you agree with my assessment, could you please pass on my original feature request to the SDK team? Also, would you be willing to accept a PR if I implemented it myself?

Side rabbit hole: I realized there is another feature in the Cloudinary SDK that may attempt to solve this problem: the image() method can generate srcset and sizes attributes. Is this documented in any more detail than the comments in the code? I can't quite figure out how to get it to do what I want... it looks like the sizes it generates are always the same size as the images, but they should be specified by me because they relate to the viewport size. Where the picture() method allows me to specify the media query but won't let me account for DPR, the image() method accounts for DPR but won't let me specify the media query!

I feel like I must be doing something fundamentally wrong, since I can't be the first one of your users who has tried to support high density screens. Do you have any idea how others approach this problem?

igy commented 3 years ago

Hi Ryan,

Thanks for the additional information - After reading the example you provided, I think I was thinking of a simple srcset without media queries involved - my experience there was that browsers take the CSS/logical pixel width and then add a multiplier for the DPR when choosing which of the offered images to show.

I performed a quick test in Chrome also on a Macbook Pro, and created an image tag with widths 250, 500, 750, 1000, 1250, 1500 - (i.e <img srcset = .....,jpg 250w, ......jpg 500w, and so on) When the Chrome window was sized so that the image tag is 480 pixels wide, it chose the 500 width version on my secondary screen which has DPR 1, and when i moved the window to the laptop screen (DPR 2) and refreshed, it used the 1000 pixels wide version- i thought that would apply to the use of media queries also but it seems I was incorrect about that, so thanks for clarifying.

In the context of the cloudinary.picture() method of the SDK, you're correct that it's not possible to specify media queries in the way that you need, currently only min_width and max_width can be set - I've created a feature request internally about adding more support for media queries for you.

To answer your other question, most implementations that I've seen use either a relatively simple image tag with an srcset, use a client-side framework to build the image tags (sometimes a third-party SDK, sometimes one of our client-side SDKs), or use our responsive() javascript method to replace w_auto and dpr_auto placeholders based on the display-time width and DPR. In the case of w_auto in particular, you can configure breakpoints either manually or automatically to optimise how many copies of the images are created at different sizes

Regards, Stephen

ryanmeador commented 3 years ago

Yes, an img with srcset is probably the simplest solution to my problem. Without using sizes, it has the drawback that the browser must download and parse the CSS before it can make the request for the image (so it can figure out which size to use). The media queries are intended to avoid that delay. I don't know if this is actually a perceptible delay on my site; I'm just trying to make it as fast as possible.

Client-side solutions have the same problem (trading download of the CSS for download of the script). I try to avoid client-side javascript unless absolutely necessary; it's amazing how far you can get with just modern HTML and CSS.

Thank you for opening the feature request. I may take a stab at implementing it myself, and if I do I'll open a PR.

igy commented 3 years ago

Hi Ryan,

We don't have a CONTRIBUTING file in our repository currently, but generally speaking we are open to reviewing and accepting PRs from customers, if you'd like to share a possible implementation for this.

If you do send something, I recommend that to aid the SDK team in reviewing it, it includes tests and that it's fully backwards-compatible (i.e that existing users of the picture() method will be able to apply your change without changing any of their own code or causing a breaking change in what that code produces). Even if the SDK team aren't able to accept it for some reason, having a PR here would allow you to apply it as a patch easily, and could let other customers do the same if they have the same requirements as you

Regards, Stephen