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

image media: add parameter to enable interlaced mode (to get progressive jpegs) #984

Closed
simonhaenisch opened this issue Aug 16, 2016 · 11 comments

Comments

@simonhaenisch
Copy link

I like to use progressive jpegs on my sites, but when using Grav's methods like ?cropResize they become baseline ("normal") jpegs.

From the notes in http://php.net/manual/en/function.imagejpeg.php:

If you want to output Progressive JPEGs, you need to set interlacing on with imageinterlace().

So my suggestion is to add a new url parameter ?interlaced with possible values true/false (or on/off), which (according to http://php.net/manual/en/function.imageinterlace.php) should do something like:

imageInterlace($img, $_GET["interlaced"] ? true : false);

As it is not suggested to use interlacing with pngs or gifs (see this stackoverflow answer), it may be better to make a parameter like ?progressive=true instead, which only applies if the file type is jpg. On the other side s.o. might want to use interlaced pngs for some reason. It could then be stated in the docs that is only suggested for jpegs.

If you like it and want me to create a PR for that (add it to the existing image actions), I can do it but it might take me a few weeks until I have the time.


TL;DR: Something like

![my image](image.jpg?cropResize=640&interlaced=true)

or

![my image](image.jpg?cropResize=640&progressive=true)

would be nice!

@rhukster
Copy link
Member

Progressive is actually enabled by default already.. Look in system/config/media.yaml:

types:
  defaults:
    type: file
    thumb: media/thumb.png
    mime: application/octet-stream
    image:
      filters:
        default:
          - enableProgressive

@simonhaenisch
Copy link
Author

That doesn't work for me (I checked my media.yaml). I have a progressive jpeg on my site, I downloaded and tested it (with http://techslides.com/demos/progressive-test.html). Then I set ?cropResize=640 and downloaded/tested again, it wasn't progressive anymore.

Explicitly enabling progressive with an url parameter works though, i.e. ?cropResize=640&enableProgressive=true or just ?cropResize=640&enableProgressive.

Can someone reproduce it, i.e. is it a bug? Otherwise I'll have to have another look at my setup.

@simonhaenisch simonhaenisch changed the title [enhancement] image media: add parameter to enable interlaced mode (to get progressive jpegs) image media: add parameter to enable interlaced mode (to get progressive jpegs) Aug 17, 2016
@rhukster
Copy link
Member

Does sound like a bug. As that should work.

@simonhaenisch
Copy link
Author

If I got it right this is the function applying the default filters:
system/src/Grav/Common/Page/Medium/ImageMedium.php#L543

Last commit message of system/config/media.yaml is:

Moved media list items into an types: key.

Maybe the filter function wasn't adapted to that?

Or does default filters mean: only apply to images that don't specify any filters? Anyway it doesn't work in that case either (I think).

@rhukster rhukster added the bug label Aug 18, 2016
@simonhaenisch
Copy link
Author

I did some more testing (on Nginx and Apache) and if I do

?cropResize=320&enableProgressive

it works, but if I instead do

?enableProgressive&cropResize=320

it doesn't. The order of the rules matters. So maybe the reason why the default enableProgressive rule doesn't work is because it is applied before cropResize.

@rhukster
Copy link
Member

rhukster commented Aug 25, 2016

it's weird, when I debug, the enableProgressive is applied to all images because it's in my base configuration as a default filter.

This is is on a bare-bones setup.

Do you have the configuration overridden with /user/config/media.yaml??? You might have a problem with the file if so, please provide a copy of your /user/config/media.yaml

@simonhaenisch
Copy link
Author

simonhaenisch commented Aug 25, 2016

I just cloned the grav repo to my server, did a bin/grav install, uploaded a big jpg to user/pages/01.home/ and inserted it into the default.md page (![sea](sea.jpg)), see simonhaenisch.com/grav/. Then I downloaded the image and uploaded it to the progressive test, but it says "not progressive" :/ There is no user/config/media.yaml file. The one in system/config is like the one you posted earlier.

I noticed that I head media.yaml and streams.yaml in the user config folder on the page I tested it on initially, but I didn't create them. Also they were empty, so I deleted them. I noticed that they get re-created (empty) when I log into admin (intended?).

So I logged into admin, cleared all cache, deleted the two empty yaml files, tried again... still not progressive.

At first when I created the issue, I tested it on another server (vps, Nginx instead of Apache) and it behaves the same there.

How do you check if the image is progressive?

@simonhaenisch
Copy link
Author

simonhaenisch commented Aug 25, 2016

I just checked my fresh install again and noticed that if I don't put any parameters, the image url on the parsed page is /grav/user/pages/01.home/sea.jpg, which makes Apache serve it directly instead of rewriting the url to Grav (I think). I also tried /grav/home/sea.jpg, but it's also not progressive.

Then I also included ![sea](sea.jpg?quality=75) as a second image and that one is served from the cache dir (/grav/images/3/d/6/3/1/3d6315bb0c8c9f6de9edb68a8e755ca5371facf7-sea.jpeg) and actually is progressive! Quite strange to me.

@simonhaenisch
Copy link
Author

Well after I just had a look again and noticed that you didn't specify an alt text, I tried that as a third image on my page (![](sea.jpg)) and that one gets cached and is progressive, too!

Quite a mean bug...

@rhukster
Copy link
Member

rhukster commented Sep 2, 2016

Ok turns out using enableProgressive as a default filter is a terrible example. Basically enableProgessive only works if it's the last item in the list of actions:

![](progress.jpg?quality=50&cropZoom=300,300&enableProgressive)

This works.. however:

![](progress.jpg?enableProgressive&quality=50&cropZoom=300,300)

Does not work.

I'm going to move the filters to the 'save' rather than the creation of the image so it's automatically applied at the end.

@ichik
Copy link

ichik commented Dec 9, 2018

I'm having a similar issue while outputting images with Twig in templates.
{{ page.media['cover.jpg'].url }} outputs non-progressive JPEG despite media.yaml being default system one. Setting output to {{ page.media['cover.jpg'].enableProgressive.url }} solves the issue.

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

No branches or pull requests

3 participants