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

TimberImage Resize combined with Retina creates upscaled image, incorrect URL #404

Closed
mennolui opened this issue Dec 9, 2014 · 13 comments
Closed
Labels
needs-investigation We've got all the info, now someone needs to look into this to figure out what's going on

Comments

@mennolui
Copy link

mennolui commented Dec 9, 2014

My uploaded source is a 3500px wide image. I want to display this image width:100% at a viewport width of 1200px, on retina (2x) displays.

I could just resize to a width of 2400px...

<img src="{{ post.thumbnail.src | resize(2400) }}" />

..which works fine and the image looks great, but i wanted to use the retina filter as shown in the Timber docs (and proposed in issue #371):

<img src="{{ post.thumbnail.src | resize(1200) | retina(2) }}" />

This however generates an unsharp image. Looks like it was actually resized to 1200px, and then upscaled to 2400px.

Also, when using the retina filter, the URL returned is incorrect: '/wp-content/uploads/xxxx.jpg'. Without retina filter the correct full URL is returned, 'http://localhost/website-dir/wp-content/uploads/xxxx.jpg'.

Menno
(Using Timber 0.20.8, WP 4.0.1)

@aristath
Copy link
Contributor

aristath commented Jan 9, 2015

I can confirm this.
It looks like retina calculations are off... in order to get something acceptable in the above example I had to so something like this:

<img src="{{ post.thumbnail.src | retina(2) | resize(2400) | retina(2) }}" />

applying the retina filter twice on an image double the size.

@kylehotchkiss
Copy link

I've seen a few casual references to retina - will this be able to detect the DPI of the users screen or is that a stretch?

@simonmilz
Copy link

I can confirm this to. I use this also like this:

{{ product.thumbnail | retina(2) | resize(700, 600, 'center') | retina(2) }}

@Twansparant
Copy link

Same here, this should be fixed.

@connorjburton connorjburton added the needs-investigation We've got all the info, now someone needs to look into this to figure out what's going on label Feb 24, 2016
@kmbt
Copy link

kmbt commented Feb 13, 2017

That is because in current implementation if Retina is chained after resize it is given the url of an already resized image, not the original one. Retina->run() currently does not try to resolve the original image path. Also, the resized image is generated as an intermediate step even if it not needed.

One quick workaround would be to add a Twig filter like this one:

    function retinize( $src, $pixel_ratio, $w, $h = 0, $crop = 'default', $force = false) {
        $ih = new Timber\ImageHelper();
        if ( !is_numeric($w) && is_string($w) ) {
            if ( $sizes = $ih::find_wp_dimensions($w) ) {
                $w = $sizes['w'];
                $h = $sizes['h'];
            } else {
                return $src;
            }
        }

        $new_w = $w * $pixel_ratio;
        $new_h = $h * $pixel_ratio;
        $suffix = "#@{$pixel_ratio}x";

        return $ih->resize($src, $new_w, $new_h, $crop, $force).$suffix;
    }

and use it instead of resize | retina combination. One downside is that filenames are not annotated with information on retina pixel ratio. However, the URLs are (with a #-sign fragment).

@pjv
Copy link

pjv commented Apr 4, 2017

I'm evaluating using Timber as a starting point for a new theme. Am I understanding this issue to mean that generating optimized pixel-density images within Timber's framework is borked and has been since 2014?

@kmbt
Copy link

kmbt commented Apr 4, 2017

This issue is about how (badly) retina filter works in Timber (Twig) templates. However, don't let this minor nuisance discourage you from using Timber in your new theme!. You can easily work around the problem with a custom filter or perhaps a macro. What's more, if you are planning to use the HTML5 picture element or the srcset attribute of HTML5 img element, you will most likely turn to macros anyway. So, some extra work would be unavoidable even if the retina filter worked properly.

In general, I think that Timber is a solid, production-ready framework and my overall experience is very positive. I recommend that you try it out as it might change your way of thinking about Wordpress theme development. It changed mine. With Timber alongsite your favourite meta-field plugin you can do wonders. Good luck!

@zackphilipps
Copy link
Contributor

@connorjburton any update here? These images really do look awful.

@markhowellsmead
Copy link

This issue is still open. image.src('original')|resize(800,600)|retina(2) scales the image down to 800x600 then blows the generated image up to 1600x1200. We currently have to use image.src('original')|resize(800*2,600*2) instead.

@zackphilipps
Copy link
Contributor

@markhowellsmead thanks, that's actually a better workaround than the retinize thingy above. Don't know why I didn't think of that. I'll submit a PR for the docs, since they specifically call out being able to chain resize and retina together: https://timber.github.io/docs/guides/cookbook-images/#generating-retina-sizes

@markhowellsmead
Copy link

@zackphilipps Strictly speaking, you could just use 1600,1200. But 800*2,600*2 is more readable. :)

@zackphilipps
Copy link
Contributor

@markhowellsmead Yup, agree, since you could potentially have 3x and 4x images as well.

@jarednova
Copy link
Member

I'm closing all 2+ year old tickets. If someone wants to pick this up, let's make it a fresh issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-investigation We've got all the info, now someone needs to look into this to figure out what's going on
Projects
None yet
Development

No branches or pull requests