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

Feature Request: Relative image transforms for PPI based responsive images. #5774

Closed
RickKukiela opened this issue Mar 9, 2020 · 9 comments
Labels
enhancement improvements to existing features site development 👩‍💻 features related to website/API development
Milestone

Comments

@RickKukiela
Copy link

Description

Lets say I have a place holder for a product or blog post image that is 300x300. That's "CSS Pixels" not "real pixels". So I want to have some css generated that would include the proper image based on devices pixel ratio. Lets say for this example the highest ratio I'm supporting is 3x, so I would want to create my base transform for 3x, so 300x3 X 300x3 = 900x900. Great so I do that. Now in my code I would like to be able to call the transformation like like this: image.getUrl('Transform', '3x') or image.getUrl('Transform', '2x') or image.getUrl('Transform', '1.5x') for each pixel density media query i'm matching for.

I should have to tell my base transform that its set to a max of 3x so when I request a 3x it just sends the 900x900, however when I request a 2x, it should know that it needs to divide each dimension by the max provided (3) to get 300x300, then multiply each dimension by the the requested ratio (2) to get 600x600.

I guess theoretically I can just manually set up transforms for each pixel ratio but that seems much more cumbersome than just requesting a variation based on pixel density and having the system create it for me on the fly.

Steps to reproduce

Additional info

  • Craft version:
  • PHP version:
  • Database driver & version:
  • Plugins & versions:
@brandonkelly brandonkelly added enhancement improvements to existing features site development 👩‍💻 features related to website/API development labels Mar 10, 2020
@brandonkelly brandonkelly added this to the 3.5 milestone Mar 10, 2020
brandonkelly added a commit that referenced this issue Mar 10, 2020
@brandonkelly
Copy link
Member

I’ll do you one better! Just added a new getSrcset() asset method, for Craft 3.5, which takes an array of srcset sizes (100w, 2x, etc.), and returns a srcset attribute value with the transform URLs combined with the given srcset sizes.

{% do asset.setTransform({ width: 300, height: 300 }) %}
{{ tag('img', {
    src: asset.url,
    width: asset.width,
    height: asset.height,
    srcset: asset.getSrcset(['1x', '1.5x', '2x', '3x']),
    alt: asset.title,
}) }}

The getImg() method also now accepts a sizes argument, so you’d get the same output as the above code by doing:

{{ asset.getImg({ width: 300, height: 300 }, ['1x', '1.5x', '2x', '3x']) }}

@rynpsc
Copy link
Contributor

rynpsc commented Mar 10, 2020

@brandonkelly Very nice, going to be a real time saver.

How would eager loading play into this? We currently have a custom module to handle srcset that returns a load of transform urls but the number of db queries ballons once we start having ~ 6 sizes per image. We've never figured out how to eager load as we don't have the actual transform object in the tempate, just the srcset string.

@RickKukiela
Copy link
Author

Nice!

@brandonkelly
Copy link
Member

brandonkelly commented Mar 10, 2020

@rynpsc Good question. Previously you would have had to manually specify each of the resulting transforms. Going with the previous example (300x300 @ 1x, 1.5x, 2x, and 3x), that would look like this:

{% set assets = entry.myAssetsField
    .withTransforms([
        {width: 300, height: 300},
        {width: 450, height: 450},
        {width: 600, height: 600},
        {width: 900, height: 900},
    ])
    .all() %}

I just pushed a new commit (7d97863 ) that brings srcset-style sizes to the withTransforms param though, so that can now be simplified to:

{% set assets = entry.myAssetsField
    .withTransforms([{width: 300, height: 300}, '1.5x', '2x', '3x'])
    .all() %}

Note that you still must start with a normal transform definition; otherwise the param will have no idea what to anchor the width/height on for “1.5x”, etc.

Putting it all together, here’s a way you can keep your template DRY:

{% set baseTransform = {width: 300, height: 300} %}
{% set srcsetSizes = ['1.5x', '2x', '3x'] %}

{% set assets = entry.myAssetsField
    .withTransforms([baseTransform]|merge(srcsetSizes))
    .all() %}

{% for asset in assets %}
    {{ asset.getImg(baseTransform, srcsetSizes) }}
{% endfor %}

@jsunsawyer
Copy link

@brandonkelly This is rad. We usually lower the quality on high res (retina) images for performance reasons. This way you get higher quality images without much of an increase in file size. I wonder if it might make sense to be able to pass in quality settings for each srcset option.

@piotrpog
Copy link

piotrpog commented Aug 3, 2020

@brandonkelly
I tested this today and noticed something.

Let's say we have an image that is 500x500. We request a transform of 200x200, with sizes 1x, 2x, 3x. This means we will end up with three transforms - 200x200, 400x400 and 600x600. The last transform is larger than original image, which means that image was upscaled. Even if upscaleImages setting is set to false.

Maybe there should be some possibility of preventing that? So in that case the last transform would become 500x500 - original size of image.

@brandonkelly
Copy link
Member

@piotrpog just posted a new issue about that: #6530

@jsunsawyer
Copy link

@brandonkelly Finally got around to checking this out, but ran into a small issue with my template (that I think I got resolved).

I am setting up an image template that will utilize lazysizes, so I need to use getSrcset() to populate a data-srcset image attribute.

I was initially having trouble with the srcset not utilizing my base transform's height. It seems that applying my base transform ({% do image.setTransform(baseTransform) %}) to the asset before outputting getSrcset(transformSteps) resolved my issue.

Am I doing this right? If this method is required for outputting the proper srcset, it might be nice if the docs noted that in some way.

@brandonkelly
Copy link
Member

@jsunsawyer Yes that’s the way to do it. We demonstrate it here: https://craftcms.com/docs/3.x/image-transforms.html#generating-srcset-sizes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement improvements to existing features site development 👩‍💻 features related to website/API development
Projects
None yet
Development

No branches or pull requests

5 participants