I found a problem with the caching of the images.
Whenever I was reloading a page with an adaptive image, the image was completely rerendered and renewed.

I used to solve this by replacing line 100:
file_transfer($image->source, array('Content-Type' => $image->info['mime_type'], 'Cache-Control' => 'private, max-age=60*60*24*7', 'Expires' => gmdate('D, d M Y H:i:s', time()+60*60*24*7).' GMT', 'Content-Length' => $image->info['file_size']));

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

greensolid’s picture

Added a patch for this issue.

sanduhrs’s picture

I can't reproduce that and I'm not in favor of adding additional caching directives.

Could you provide detailed information on how to reproduce the issue, please.

sanduhrs’s picture

Status: Needs review » Postponed (maintainer needs more info)

Adjusting status.

greensolid’s picture

I am using the effect at the last position after scaling to a fixed width and in between cutting to the same width with adding a fixed height.
For me that means I always use images with a resolution of 1382x140 to adaptive scale.

Comprehensibly it takes some seconds to render an image, but for me, if I was loading any page, even if the page as it was cached, an adaptive image was rerendered instead of being cached by the browser as any other element or image.

Though I included a client side caching header.

But you mentioned, this would be a "additional" caching method.
Are there other ones included in adaptive images.
Possibly this ones aren't working for me.

greensolid’s picture

Version: 7.x-1.3 » 7.x-1.4
FileSize
815 bytes

I just tested Version 7.x-1.4 and the problem still seems to be unsolved.
Accordingly I add a renewed patch.

kerasai’s picture

Browser side caching is only going to benefit a scenario where a single browser requests an image many times. A case where many browsers request the image only once would require the Drupal site to generate the processed image each time it is requested.

I think a better solution to this would be to cache the image file and serve that file directly, as the image system is intended to do. This module does do this, but there are some cases where it's not working properly. I haven't been able to do an exhaustive work through to figure out what's going on I think there's an issue in adaptive_image_preprocess_image() that's causing a "bad" derived image URL to be generated in some cases.

My initial thought is that the derived URL is generated that the logic is having trouble determining specifics on the device that is requesting the page. When I load non-cached pages from my desktop or phone, I get the proper image URLs. Subsequent cached pages load the proper URL if the proper URL was cached. If I let nature run its course, eventually I notice "bad" image URLs in my cached pages--generated by who knows what kind of device/browser. Every time this "bad" image URL is then requested, the image is generated from scratch and not saved into the sites/default/files/styles cache directory.

Here's an example of a "bad" URL:
sites/default/files/styles/large/adaptive-image/public/my_image.jpg

Here's an example of the same image, but with the URL written properly:
sites/default/files/styles/large/public/1382/my-image.jpg

There is a whole different issue with page level (and Boost) caching that makes using this module not viable if you require using those. This isn't a knock on the module, it's just that the technique this module uses is not going to work.

Any input is welcomed.

jlporter’s picture

@kerasai - you've hit the nail on the head. With drupal cache on the request url doesn't match physical path which makes the webserver send the request off to drupal *every* time. Turn drupal cache off and the request url for images match physical path and assuming the image has been generated, the webserver will deliver the static file.

jlporter’s picture

I think the root of the problem would be when core collects the urls for caching it's hitting image api without running the hooks. I'm fairly certain the correct url as @kerasai shows is actually when some hook is run that adaptive mangles the url properly. When core caches the page and gets the url from image api, it's not getting the hook that mangles the url.

This very well could be a core bug....will keep researching.

shanefjordan’s picture

We are experiencing this same issue. Images regenerate on each call, rather than pulling the previous generated version. I have removed the adaptive image style from any image style that has a image size less than 300 wide. This now prevents these images from constantly regenerating.

Because of this, it seems more like an issue with the Adaptive Image module than with core.

kerasai’s picture

Just to reiterate, the mechanism that the module uses to decide which image sizes to use is ineffective when full page caching is happening. So if you've got primarily anonymous users and you're using Varnish, Boost, or Drupal's full page cache, then this module doesn't provide any benefit.

quantos’s picture

Following. Although @kerasai vis a vis "then this module doesn't provide any benefit" surely the module is at least serving images at the correct device size even if it isn't serving cached images?

Q.

kerasai’s picture

Surely the module is at least serving images at the correct device size even if it isn't serving cached images?

My answer is "it could be." If full page caching is in use you're likely getting images computed for someone else's screen size, which may or may not be the same size images as you need.

Although bandwidth is important, IMO the image size is a secondary concern to the server resources used to process the images. Missing the image cache is very expensive, and if the site has a considerable amount of traffic and/or many, large images being processed this will likely cause resource issues for the server.

quantos’s picture

Hm. Good luck debugging that then. Looking over my sites that use this module (all Omega/delta/context builds) the image URLs all seem to be behaving themselves but let me know if I can help.

jim_keller’s picture

A few notes on this issue:

We are experiencing this same issue. Images regenerate on each call, rather than pulling the previous generated version.

This is probably related to the bug report (and patch) I've posted here: https://drupal.org/node/2053047 . Cached images weren't being respected if a space or other non-URL friendly character was present in the filename.

the mechanism that the module uses to decide which image sizes to use is ineffective when full page caching is happening.

This is a bigger issue and one that's more difficult to solve - how to avoid caching the actual image size when Drupal caches the entire page.

The way this module works is that the first time around (on an image that hasn't yet been image-styled to its appropriate size), the "src" of the img tag is set to /styles/adaptive-image/public/filename. On subsequent calls, the adaptive images module will instead return a direct path to the appropriate image file.

The latter is, in fact, the behavior you want. You want the page to be cached with a link directly to the JPG, not to the image style URL, because you don't want to be bootstrapping Drupal each time the server requests an image. It would be horribly inefficient.

However, we also don't want the page to be cached with the styles/ URL as the img src (not even on the first pass), so what the module should do is generate the image on the fly, as necessary, and *always* return a link to the actual image file - the img src should never be set to the "styles/" URL. This should be easy enough to patch; I'll try to submit something myself if I can find some time to play with it.

There's still another problem with caching, though. Even if we return the correct size of the image in the url (e.g. "sites/default/files/styles/large/public/1382/my-image.jpg"), Drupal's just going to cache that whole img src line. So everyone who subsequently visits the page is going to get the size of the image at the time it was initially cached sent to their browser. So if the first person to visit the site after a cache clear is on a mobile device, every user on every device would then get served the mobile version of the image.

The module page for adaptive images states that there's "no need for additional rewrites" (I'm assuming that means .htaccess customizations), but if you want to use page caching (which you do), we will in fact need to implement the .htaccess and adaptive-images.php techniques described at: https://github.com/mattwilcox/Adaptive-Images and adaptive-images.com. It'd probably be best to include Drupal-compatible samples along with this module - essentially requests for JPGs, etc need to be re-routed through the adaptive images PHP script that handles determining the appropriate file to serve up.

Anonymous’s picture

Issue summary: View changes

I have a patch for this that is working for me so far.

Indeed, the module is reconstructing the image every request because the logic to detect the file has not been updated since the introduction of the "itok" parameter in the query string appended to every image style url, a query string which pathinfo thinks is a file extension.

Will post it here when I have time tomorrow.

Anonymous’s picture

Here's a patch against version 7.x-1.4

welly’s picture

Unfortunately the patch doesn't appear to be working for me.

ABaier’s picture

Any updates on this issue?

MastaP’s picture

From my point of view, you can't currently use this module if you want to cache pages.

Rob_Feature’s picture

I'm confirming what MastaP says...we're on the verge of launch and finding that these images load WAY too slow with caching enabled...

bluehead’s picture

Issue still exists.
#14 works for me

Joran Lafleuriel’s picture

#16 patched on 7.x-1.x-dev [ 9 Jun 2016 at 09:53 CEST]

that saved my life

thanks