Add support for responsive images

[gerdint]

Add the srcset and sizes attribute to img tags whose parent element class is 'figure'. Each matched image is made avaiable in three version (roughly intended to match a mobile, 1x desktop and 2x/hi-dpi desktop clients). Images are cached in a configurable directory.

Depend on ImageMagick® for size identification and scaling. Making use of the Racket htdp2/image module as a fall-back is probably possible but not implemented.

[gerdint]

There may be things that can be improved upon, but sending this pull request to solicit some feedback.

[gerdint]

I will look into the failed tests.

As for the motivation, I maintain a food blog (it's in Swedish) hence my desire for a fair amount of snazzy images (including high-dpi versions). This feature may of less interest in the case of PLT-oriented blog. In any case, making a small image available as well has the potential for increased page load speed (especially on mobile devices).

The main inspiration is the responsive image support added to Wordpress 4.4 (see https://make.wordpress.org/core/2015/11/10/responsive-images-in-wordpress-4-4/ for some background).

[gerdint]

I believe the remaining culprit of the failed tests is that the test server is missing ImageMagick.

[greghendershott]

This is a really interesting idea and from looking at your blog I can appreciate the motivation!

I have mixed feelings about needing ImageMagick. I'd prefer if this could be done with pure Racket so that it would "just work". (A big motivation for Frog was that other things I tried seemed fussy to install and configure.)

Admittedly I suppose there's a precedent here using Pygments for syntax highlighting. I think that's justified due to the huge body of lexers; there is no way that could (or should) be re-invented in Racket. Whereas I'd like to believe that image resizing is something we could/should be able to do in Racket. But maybe I'm under-estimating what's required and what ImageMagick contributes.

I believe the remaining culprit of the failed tests is that the test server is missing ImageMagick.

Could you try installing it (e.g. apt-get) in the before_install section of .travis.yvml?

---

I have some micro code feedback and questions, but I don't want to provide it and have you do work until/unless I feel like the PR is something I'd want to accept at all.

[gerdint]

As I wrote in the initial comment above I believe the 2htdp/image module does provide the needed functionality (file format support may be weaker than ImageMagick, but then again gif, png and jpeg would probably suffice, though WebP support would be nice). I suspect that ImageMagick is faster (but have not verified, even though the more verbose mode times the resizing operation so I meant to investigate this). I agree going with an all-racket approach would be nice, even though the 2htdp module doesn't feel like part of standard Racket.

I guess a sensible next step would be to have a go at a pure Racket option to get a feel for differences in performance. If it's decent, it may be a good option for someone looking to try out the functionality. And if they would like better performance and file format support they could install ImageMagick. Since images are cached and thus only built once, the speed of the resize operation may not be so important (but they are deleted upon clean).

[gerdint]

Looks like sudo would be required to install ImageMagick. Would it be okay to flip the switch?

[greghendershott]

I agree it would be acceptable if the build times are somewhat slower for a pure-Racket image-resizing solution. Would you be willing to experiment with that? If so that would be wonderful. Because I don't really have time now. More importantly I think you'd be a better judge of the results: Is the processing time acceptable? Is the resized image quality?

[gerdint]

Resizing imaces using racket/draw is actually much faster (like 6x on my machine) compared to the default ImageMagick algorithm (which is apparenty optimized for high-quality). I can't really see much difference on the images I look at, but ImageMagick has an amazing amount of knobs when it comes to resizing. The downside is that Racket only supports PNG and JPEG (and bmp, and xbm and something else), but for this usecase I suspect it's all JPEG so probably ok.

I'm inclined to make Racket the default (although that means racket/draw, which seems to depend on racket/gui, is that a problem?). If ImageMagick is available, it will be used (giving support for more image formats and possibly higher-quality images. Sounds like a plan?

[gerdint]

Does it make sense to try to not require racket/draw it ImageMagick is available?

[greghendershott]

I'm inclined to make Racket the default (although that means racket/draw, which seems to depend on racket/gui, is that a problem?).

Oh! That's "heavier" than I realized. Hmm.

Does it make sense to try to not require racket/draw it ImageMagick is available?

It might make sense, especially if we were to decide that info.rkt is not going to say racket/draw (and therefore transitively racket/gui) is a dependency for installation. But actually ...

If ImageMagick is available, it will be used (giving support for more image formats and possibly higher-quality images. Sounds like a plan?

I'm worried about having too many configuration permutations: ImageMagic or racket/gui or neither.

The spirit of my original question was hoping that instead of ImageMagick we could keep this pure Racket.

---

So. I'm not sure the best thing to do.

Perhaps we should back up. Maybe we should stick with your original PR: ImageMagick is optional, for people who care to do image-resizing. Exactly like how Pygments is optional, for people who care to do syntax highlighting.

What do you think?

---

If you agree:

Currently .travis.yml does not install Pygments. As a result:

• Travis exercises the scenario where Pygments is not installed -- does it degrade as expected.
• Only when a developer runs tests locally with Pygments installed, is that exercised.

You could argue that's bad, and we should add Travis matrix Pygments-installed tests. But until/unless we do that, perhaps ImageMagick should follow that example?

That would mean adjusting your tests so they succeed both with and without ImageMagic installed. (If that turns out to be too awkward, you could "cheat" and check for running on Travis with something like (= (getenv "CI") "true"), I guess?)

[gerdint]

Given that Frog is a static site generator and I think usually invoked on the blog author's workstation I suspect there will probably be a full installation of Racket available (and not something like minimal Racket). So I do not really see having racket/draw as a dependency a problem, but as you mention targeting both Racket and ImageMagick implies more configuration permutations. If you rather not depend on racket/draw I propose we go back to ImageMagick-only, like you suggest. That way people who would like to enable this always get high-quality resizing and good file format support.

Regarding the tests, I see that we would like to handle things the same way as Pygments, but I do think that it makes more sense to have Travis run the full test suite. (i.e. with Pygments installed as well). Wouldn't a simple "apt-get install python-pygments" in .travis.yml get this? (provided that sudo is enabled).

Either way, I will make the tests pass when ImageMagick is not installed.

[greghendershott]

Given that Frog is a static site generator and I think usually invoked on the blog author's workstation I suspect there will probably be a full installation of Racket available (and not something like minimal Racket).

On other projects like racket-mode I've learned that some people get Racket using Homebrew, which by default installs Minimal Racket.

So I do not really see having racket/draw as a dependency a problem, but as you mention targeting both Racket and ImageMagick implies more configuration permutations.

Also this.

If you rather not depend on racket/draw I propose we go back to ImageMagick-only, like you suggest. That way people who would like to enable this always get high-quality resizing and good file format support.

Yes, please.

Either way, I will make the tests pass when ImageMagick is not installed.

That would be great. Thanks!

Regarding the tests, I see that we would like to handle things the same way as Pygments, but I do think that it makes more sense to have Travis run the full test suite. (i.e. with Pygments installed as well). Wouldn't a simple "apt-get install python-pygments" in .travis.yml get this? (provided that sudo is enabled).

I'm not necessarily opposed to expanding the Travis test matrix to include having Pygments and/or ImageMagic installed. But I'd like to suggest we first get a clean finish on this PR, without that. Then loop back to address that as a separate item, someday.

---

I really appreciate you taking the time to contribute this. I'm sorry I've been slow to respond, and even to determine with you what should be done. I wish I had more time available to work on this lately.

[gerdint]

So tests pass now, because they don't run if ImageMagick isn't found (in path).

There is at least one thing I would like to change (indicated by the TODO in responsive-images.rkt), and I haven't considered/tested Scribble files (I'm assuming they don't wrap img tags in a figure div which is what the codes looks for). Any thoughts on Scribble? I'm thinking that doing this for all img tags would be overkill, since the big win is for basically for photos.

[gerdint]

I fixed the TODO (there can now be an arbitrary number of resized images and the default size is configured through a separate variable.

For testing purposes it was convenient to make the above two options parameters. While I don't intend to document and put them in .frogrc at this point, this means that in total I have added no less than five new parameters. Let me know if you find this onerous I will look into a solution.

Lastly, I thought it would be fun to spawn the resize processes asynchronously to enable parallel resizing (should speed things up on multicore machines). But it would probably be a good idea to merge the current (synchronous) version first.

[greghendershott]

I haven't considered/tested Scribble files (I'm assuming they don't wrap img tags in a figure div which is what the codes looks for). Any thoughts on Scribble? I'm thinking that doing this for all img tags would be overkill, since the big win is for basically for photos.

I think it's OK to say this won't work for Scribble format sources. Already in .frogrc you talk about Markdown. So hopefully it will be clear this isn't currently intended to work for Scribble.

[greghendershott]

Lastly, I thought it would be fun to spawn the resize processes asynchronously to enable parallel resizing (should speed things up on multicore machines). But it would probably be a good idea to merge the current (synchronous) version first.

Although I agree that sounds neat, I also agree it would be good to merge synchronous first.

Before merging I'd like to do one last sanity check: Run my own blog through this and diff the output. I'd expect it not to change at all, or only in trivial ways. Does this at least "do no harm if I'm not using it". :)

Assuming that's OK, part of me feels like I should just merge it. OTOH part of me feels like I ought to try enabling the responsive images and using ImageMagick. But if I don't have time to do that, today, I'm delaying this even longer....

[greghendershott]

OK it passed the do-no-harm test on my own blog.

Next, I'm trying to actually use responsive-images? = true in my .frogrc.

I am an ImageMagick n%b -- which is bad for you, but good for the instructions I suppose we ought to create for other n%bs. :)

I did brew update && brew install ImageMagick. It appears to have succeeded.

But magick or which magick don't work, therefore neither is (find-executable-path "magick").

$ brew install ImageMagick
sed: .git/GITHUB_HEADERS: No such file or directory
==> Installing dependencies for imagemagick: libtool, xz, jpeg, libpng, libtiff, freetype
==> Installing imagemagick dependency: libtool
==> Downloading https://homebrew.bintray.com/bottles/libtool-2.4.6_1.el_capitan.
######################################################################## 100.0%
==> Pouring libtool-2.4.6_1.el_capitan.bottle.tar.gz
==> Caveats
In order to prevent conflicts with Apple's own libtool we have prepended a "g"
so, you have instead: glibtool and glibtoolize.
==> Summary
🍺  /usr/local/Cellar/libtool/2.4.6_1: 70 files, 3.7M
==> Installing imagemagick dependency: xz
==> Downloading https://homebrew.bintray.com/bottles/xz-5.2.2.el_capitan.bottle.
######################################################################## 100.0%
==> Pouring xz-5.2.2.el_capitan.bottle.tar.gz
🍺  /usr/local/Cellar/xz/5.2.2: 91 files, 1.4M
==> Installing imagemagick dependency: jpeg
==> Downloading https://homebrew.bintray.com/bottles/jpeg-8d.el_capitan.bottle.2
######################################################################## 100.0%
==> Pouring jpeg-8d.el_capitan.bottle.2.tar.gz
🍺  /usr/local/Cellar/jpeg/8d: 19 files, 713.8K
==> Installing imagemagick dependency: libpng
==> Downloading https://homebrew.bintray.com/bottles/libpng-1.6.25.el_capitan.bo
######################################################################## 100.0%
==> Pouring libpng-1.6.25.el_capitan.bottle.tar.gz
🍺  /usr/local/Cellar/libpng/1.6.25: 25 files, 1.2M
==> Installing imagemagick dependency: libtiff
==> Downloading https://homebrew.bintray.com/bottles/libtiff-4.0.6_2.el_capitan.
######################################################################## 100.0%
==> Pouring libtiff-4.0.6_2.el_capitan.bottle.tar.gz
🍺  /usr/local/Cellar/libtiff/4.0.6_2: 261 files, 3.4M
==> Installing imagemagick dependency: freetype
==> Downloading https://homebrew.bintray.com/bottles/freetype-2.7.el_capitan.bot
######################################################################## 100.0%
==> Pouring freetype-2.7.el_capitan.bottle.tar.gz
🍺  /usr/local/Cellar/freetype/2.7: 61 files, 2.4M
==> Installing imagemagick
==> Downloading https://homebrew.bintray.com/bottles/imagemagick-6.9.5-10.el_cap
######################################################################## 100.0%
==> Pouring imagemagick-6.9.5-10.el_capitan.bottle.tar.gz
🍺  /usr/local/Cellar/imagemagick/6.9.5-10: 1,464 files, 22.5M

$ magick
-bash: magick: command not found

$ which magick
$

$ ls /usr/local/bin/ 
Magick++-config     lzcat               thumbnail
Magick-config       lzcmp               tiff2bw
MagickCore-config   lzdiff              tiff2pdf
MagickWand-config   lzegrep             tiff2ps
Wand-config         lzfgrep             tiff2rgba
animate             lzgrep              tiffcmp
aspell              lzless              tiffcp
aspell-import       lzma                tiffcrop
bmp2tiff            lzmadec             tiffdither
brew                lzmainfo            tiffdump
cjpeg               lzmore              tiffinfo
compare             mogrify             tiffmedian
composite           montage             tiffset
conjure             pal2rgb             tiffsplit
convert             png-fix-itxt        unlzma
display             pngfix              unxz
djpeg               ppm2tiff            word-list-compress
fax2ps              precat              wrjpgcom
fax2tiff            preunzip            xz
freetype-config     prezip              xzcat
gif2tiff            prezip-bin          xzcmp
glibtool            pspell-config       xzdec
glibtoolize         pygmentize          xzdiff
identify            ras2tiff            xzegrep
import              raw2tiff            xzfgrep
jpegtran            rdjpgcom            xzgrep
lein                rgb2ycbcr           xzless
libpng-config       run-with-aspell     xzmore
libpng16-config     stream              

However I do have a convert.

$ convert
Version: ImageMagick 6.9.5-10 Q16 x86_64 2016-09-21 http://www.imagemagick.org
Copyright: Copyright (C) 1999-2016 ImageMagick Studio LLC
License: http://www.imagemagick.org/script/license.php
Features: Cipher DPC Modules 
Delegates (built-in): bzlib freetype jng jpeg ltdl lzma png tiff xml zlib
Usage: convert [options ...] file [ [options ...] file ...] [options ...] file

....

1. What should I do?
2. We should probably document this in the README and .frogrc (similar to what I tried to do for Pygments)?

[gerdint]

I am using ImageMagick 7 installed with pkgin. It may be that magick is a v7+ command, I think convert should be fine, will look into it since there should be no reason to depend on very recent versions of ImageMagick.

[gerdint]

Earlier you mentioned you had some code-level comments? You have more Racket experience than I do so I'm all ears here.

[greghendershott]

I am using ImageMagick 7 installed with pkgin. It may be that magick is a v7+ command, I think convert should be fine, will look into it since there should be no reason to depend on very recent versions of ImageMagick.

Thanks in advance for looking at that.

Earlier you mentioned you had some code-level comments? You have more Racket experience than I do so I'm all ears here.

I'd be happy to give the feedback -- but it's nothing that would prevent me from merging. So with that preface -- don't feel you need to act on it, even if you agree with my opinion -- I'll try to leave a few comments soon.

[greghendershott]

I left some more code comments/opinions. But only because you asked. I'd be happy to merge without any of that. Instead the only "must-have" IMO is to figure out magick vs. convert.

[gerdint]

I believe I have attended to the issues you raised.

However it seems like tests are failing now for some reason. From the Travis log it looks like the tests are triggered again. I don't see why, but then again it's way past bedtime for me and I this has been a post-party hacking session so I'm probably missing something obvious.

[greghendershott]

Thanks! I think there are two issues related to paths:

1. When using paths relative to .rkt soure files -- as in some of your tests -- you can't use relative paths that rely on the current directory being the same as the .rkt file. It might work by accident but won't in general. Instead you can use Racket's define-runtime-path. That's probably the answer to making sure the tests work on Travis CI, too.
2. In responsive-images.rkt, you should require "paths.rkt" and use the provided www/img-path function to get the root "img" directory, then use build-path* to append "resized". I should have thought of this earlier, but alas it's been some months since I worked on frog myself; sorry!

I'll use the inline code comments to provide some examples, next ....

[gerdint]

Thanks for pointing me too paths.rkt, it certainly had plenty of very useful functionality!

[gerdint]

Tests are still failing though. Are there other executables called identity and convert installed?

[gerdint]

Do you have any idea of what's going on here? The tests that are not supposed to run are being run. Since I cannot reproduce this on my machine it would be nice to be able to log in to the test environment to investigate, but I assume this is not possible?

Are the tests passing on your machine?

[gerdint]

Ping!

Unless you have any idea of why the tests are being run I think I would need to try to replicate the Travis environment on my machine to try to understand what is going on.

[greghendershott]

@tger Unfortunately no quick ideas. I've been swamped. When I have time, I'll try some things hands-on, see if I can come up with any ideas.

[gerdint]

No problem. I tried installing ImageMagick on Travis but no difference so there must be something basically wrong. Do you know if there is an easy way to reproduce the Travis environment locally?

-Tobias

12 okt. 2016 kl. 02:03 skrev Greg Hendershott [email protected]:

@tger https://github.com/tger Unfortunately no quick ideas. I've been swamped. When I have time, I'll try some things hands-on, see if I can come up with any ideas.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/greghendershott/frog/pull/166#issuecomment-253082268, or mute the thread https://github.com/notifications/unsubscribe-auth/AAA6dlPLOUCR9gf3I6BXZUv8wmCJJpZ9ks5qzCPWgaJpZM4Ju_sO.

[gerdint]

Could be that things started failing when I switched from system to system*, I'll investigate.