Suffix image sizer option causes focus change to not generate new variation

[Toutouwai]

Short description of the issue

Normally every time the focus area for an image is changed in Page Edit a new variation is generated and visible on the front-end (browser cache may need to be cleared to see the changed variation).

This can be seen in these front-end screenshots before and after the focus area is changed (browser dev tools are open to disable caching).

<img src="<?= $page->image->size(200, 300)->url ?>" alt="">

But if a suffix is passed in as an image sizer option then the variation no longer updates after a focus change.

<img src="<?= $page->image->size(200, 300, ['suffix' => 'test'])->url ?>" alt="">

Expected behavior

Suffix has no impact on focus change generating new variation.

Optional: Suggestion for a possible fix

It would be great if the focus area was included (perhaps in an encoded form) as part of the variation file name so changing the focus area would reliably result in a new variation being loaded on the front-end regardless of suffix and would take care of cache-busting.

Setup/Environment

• ProcessWire version: 3.0.113

[ryancramerdesign]

When someone uses a suffix, the idea is that they've done so intentionally to create some kind of custom variation (via $options). At that point it's no longer managed by PW as an auto-generated variation, because it doesn't have the information necessary to re-create it.

[Toutouwai]

At that point it's no longer managed by PW as an auto-generated variation

I'm not sure what you mean by "auto-generated variation". I thought the admin thumbnail was the only variation automatically generated by PW. Other variations are created manually/deliberately by size() calls in template files, etc, possibly with various image sizer options passed in that are expected to affect the resulting variation.

I think that between this issue and the other one relating to changing the focus area there is a bug. Which part is the bug depends on what is supposed to happen if a user changes the focus area of an image.

Changing the focus area of an image could be treated in one of two ways:

1. The changed focus is an important change and should result in new variations being created and shown on the front-end. In which case unrelated image sizer options such as suffix shouldn't be a factor in whether this happens or not (there is already an image sizer option to ignore focus if that is desired). I'd prefer this option, but if the variation is to be regenerated as a result of a change in focus then it should be regenerated at the time it is requested by size(), where all the relevant image sizer options are known. At the moment it seems that PW is recreating (some) variations at the time the focus is changed, but I don't see how that is going to work well because PW cannot know the relevant image sizer options at that time.

2. The changed focus is not an important change and new variations do not need to be created if a variation at that size already exists. In which case the situation described in the other issue shouldn't occur. I think this option would be confusing for site editors, because if they are experimenting with the focus area to achieve a particular crop framing on the front-end then they need/expect to see those changes on the front-end.

It seems like going with option 1 would be simplified if the focus data is reflected in the variation filename. Then a change in focus automatically results in a new variation, which I think is what users and devs expect. And it takes care of cache-busting on the front-end too. As I've mentioned in an earlier request, I think it would be good to make it configurable to include more of the image sizer options that have affected a variation in the variation's filename. This could be done via $config settings, but maybe a config screen would be nicer:

That screenshot is from an unreleased custom module but I think it would be better to include at least some of this in the core.

[Toutouwai]

This issue is related: #702 (closed to minimise the number of open issues in the repo)

[teppokoivula]

My comment from the now closed related issue, for reference:

When focus change in admin triggers variation recreation, this happens without knowledge of settings that the developer provided via code when first creating that variation, and as such may result in a very different outcome than what was originally intended.

For this reason I think that we should either store every setting available via code somewhere so that we can reuse them for that new variation, or alternatively we should not attempt to recreate variations and rather let the (developer defined) code perform that task – otherwise we will end up with unexpected results.

[knoedelsalat]

This also seems to be an issue when using "cropping" => "false" in image size options. If I use

$img_large = $image->size(1200, 1200, $options = [ 'cropping' => false ]

in a template, the image will be generated within 1200x1200 px limits without being cropped, working as expected.

Then, if the focus point is being changed afterwards, the image variation will be instantly recreated as a 1200x1200 cropped version. Then on page call the cropped version will be displayed. Gave me a bit of a headache last night :)

In this case I actually resorted to using a suffix to prevent PW from recreating my image variations for that particular template

[reminders]

@netcarver set a reminder for Mar 12th 2019

[ryancramerdesign]

@Toutouwai @teppokoivula You've both mentioned including every option in the filename at some point, and I think that would resolve what @knoedelsalat mentioned as well. However, I can't just introduce that because it would potentially wreak havoc on existing installations. What I could do though is make that behavior another option to the size() call, and/or a $config option, and/or make it enabled for new installations automatically. I'm thinking most probably don't need this, and it might also result in some pretty verbose filenames, but I think it'd be a useful option to have so long as one can choose to enable it or disable it.

[reminders]

@netcarver set a reminder for Mar 15th 2019

[teppokoivula]

@ryancramerdesign, while I think that including every option in the filename would be a possible solution, I'd like to highlight one part from my comment:

... alternatively we should not attempt to recreate variations and rather let the (developer defined) code perform that task ...

What do you think about this – does it make any sense? Personally I think that this re-creation feature is a nice touch, but also not necessary – so unless there's an easy way to make it aware of all the options, it would be much better to simply disable it. I'm not sure if that's currently doable, but if not completely, at least we could provide a toggle for it, so that it's disabled by default in new setups.

If it was indeed configurable, it would be relatively safe to assume that if the developer has switched it on, they should be aware of potential side effects.

[Toutouwai]

I agree with @teppokoivula that regenerating variations at the time the focus is changed is not necessary - the variations should be recreated next time they are requested via a size() call.

I like the idea of a $config option to include more/all sizer options in a suffix to the variation name because as a general rule if I change any size() option I want to see that change reflected on the frontend. I already have my own solution for this (the module I mentioned above) but would welcome seeing it as an option in the core.

As for the filenames being overly verbose, my module produces a suffix like the one below which I don't think is excessively long:

frog.400x400-u0i1s1q90f1t420l710z0.jpg

That suffix would decode as:

• upscaling: false
• interlace: true
• sharpening: soft (I assigned an integer to represent each sharpening option)
• quality: 90
• use focus: true
• focus position top: 420
• focus position left: 710
• zoom: 0

[ryancramerdesign]

@Toutouwai Nice job with the module. I think this is good having this solution available as a module like this for the people that need it. If it turns out over time that the audience is significant then I'd suggest we copy your addVariationSuffix() method into Pageimage/Pageimages directly if you are open to it (enabled by another option or config('installed') date). Except that rather than using a selectable set of options to affect the suffix, it would just use any option that differs from a defined set of core defaults, or any in config('imageSizerOptions') that differs from those defaults.

[Toutouwai]

@ryancramerdesign, if you later decide to add something like this to the core it would be fine with me.

But if you do that I think it would still be good to somehow give the user the option to ignore certain ImageSizer options from the suffix. For example, a user might be undecided about what default sharpening level, quality or interlace option they will settle on. They might want to change these later without causing all existing variations to be regenerated.

Is the general consensus that this issue should now be closed and anyone wanting to change the core behaviour on this (and the other related issue closed previously) should install my module?

[teppokoivula]

Is the general consensus that this issue should now be closed and anyone wanting to change the core behaviour on this (and the other related issue closed previously) should install my module?

Honestly I still think that this behaviour should be disabled by default until it can be guaranteed to work as expected – i.e. take the settings provided by developer into account –  in as wide range of cases as possible.

@ryancramerdesign, if you could perhaps provide some feedback regarding aforementioned idea, or the alternative that maybe there could be a setting for enabling / disabling the regeneration behaviour completely on a case by case basis, I'd be happy to hear it :)

[ryancramerdesign]

@Toutouwai @teppokoivula I think the best bet is to monitor usage of the module over time. Toutouwai can let us know if it seems like usage is high enough to warrant inclusion of the option in the core down the road. Even if it were to be added to the core at some point then I agree it should be disabled by default, and perhaps one could enable and configure the behavior of it from the config.imageSizerOptions or one could also specify an option Pageimage::size() method to use it.

[knoedelsalat]

I think what @teppokoivula means is that right now all variations auto-regenerate on a focus change, producing unexpected results in some cases, and if it's a possibility to disable that for the time being.

The only problem I see with that is that PW would need to delete all variations on a change, and then only regenerate them on size() calls. But this would probably also affect variations from within RTEs..

[teppokoivula]

@knoedelsalat, correct. I'm getting the feeling that we're talking about two different things here :)

1. I'm saying that the auto-generated thumbnails are currently unreliable, so this feature should be either disabled (by default) for the time being, or at the very least the core should provide an option to disable it manually (via config, or via settings on module or field level).

2. The module, and adding all those variables in file name, is another solution for this issue. I do think that it, or something similar, should be in the core – but until then, we should have some way of dealing with this issue (see 1.)

My main point is that until the re-creation feature is reliable enough, it probably shouldn't be an (enabled by default) core feature. Currently it can cause issues that are hard to figure out, and may go unnoticed by content editors and admins while still lowering the usability or UX of the site.

Anyway, I don't want to drag this issue on. If Ryan feels that the feature is solid enough, we'll just have to agree to disagree about that – whatever his decision is, we'll roll with that. Currently I'm just not sure that we're actually talking about the same thing :)

[szabeszg]

the idea is that they've done so intentionally to create some kind of custom variation

I would like to point out that the docs does not help us to figure out if there is an "idea behind" the usage of the suffix or not. All we are informed is: "Suffix word to identify the new image..."

Needles to say, the docs should be clear on what the suffix is for and what are its limitations.

I also started to use the suffix in order to identify the variations generated by the admin or by any other source. In config.php I include 'suffix' => 'thumb' in $config->adminThumbOptions and on the frontend I also add the same suffix so that I use the same variations for a gallery. This is to save on the number of generated variations (eg. storage space / file nodes).
My other intention was to use the suffix to identify where a given variation is used.

[horst-n]

@ryancramerdesign , @teppokoivula , @Toutouwai Sorry, I missed this one, so be a bit late to it.

A) I think automaticaly recreation when changing the focus point should be optional, means should be possible to enable/disable (maybe on image field basis, or site wide?).

B) What about storing the complete final options array, that was applied for variation creation, into a reserved IPTC tag? (serialized) It is used with the CroppableImage settings too. And we already have the code in imagesizer for more than 5 years. The options array get stored with the variation file, so if it exists and need to be automatically recreated, it holds all relevant data by itself, regardless of filename parts and suffixes! :)

[ryancramerdesign]

@horst-n

1. Makes sense, I agree.
2. I like the idea of using an IPTC tag (JPG/PNG) but my understanding is that it's not universally supported in all image formats? For instance, I don't think GIF supports IPTC tags?

[horst-n]

@ryancramerdesign

2. Ah, damn!
   Yes it is only supported with JPEGs. :(

[horst-n]

@ryancramerdesign

2. Sidecarfiles? Not as nice as IPTC storage, but maybe a combined solution:
   IPTC for JPEGs and sidecarfiles for PNG & GIF

[matjazpotocnik]

Why not sidecarfiles for all?

[horst-n]

@matjazpotocnik Storing the data physically within the imagefiles is more robust then sidecar files. And this functionality is already included into the imagesizers. Therefore I think it is a good solution to use IPTC where applicable and sidecar files where IPTC is missing.
And I believe that in most sites the amount of JPEG files is multiple times of that for PNGs or GIFs. What would keep the amount of sidecar files low.

[adrianbj]

Has anyone noticed with cropping based on the focus point just stop working recently? The crop coordinates are being set in the images json and I am clearing the image variations and the browser cache, but it keeps cropping on the center of the image, not based on the focus point. If I force with 'cropping' => 'n' it crops at the top, so cropping is working, it just seems to be ignoring the focus point. The docs at https://processwire.com/api/ref/pageimage/size/ state that both center and true are the default value.

I ended up having to do some really weird hacks:

if($page->image->focus['top'] != '50' && $page->image->focus['left'] != '50') {
    $cropCoords = [round(($page->image->width * $page->image->focus['left'])/100), round(($page->image->height * $page->image->focus['top'])/100)];
}
else {
    $cropCoords = 'center';
}

$page->image->size(330, 330, ['cropping' => $cropCoords])->url;

The check for 50 is not ideal, but that is what is returned when the crop isn't set. Also, regarding the code above that converts % to px - I had to do that because % cropping wasn't working either - it was always generating a p100x100 variation.

Can anyone confirm these current issues I am seeing?

[Toutouwai]

Can anyone confirm these current issues I am seeing?

Hmm, no, it seems to be working for me here (so long as you're not setting a suffix of course, as per this issue).

And just to link in some discussion related to this general issue: #1301

[adrianbj]

Thanks for checking @Toutouwai - I am NOT setting a suffix, so I don't know what's going on at the moment. My hack is working for now. Perhaps I'll try to dig in deeper tomorrow to see if I can narrow it down to something.

[jaromic]

Has anyone noticed with cropping based on the focus point just stop working recently? The crop coordinates are being set in the images json and I am clearing the image variations and the browser cache, but it keeps cropping on the center of the image, not based on the focus point. If I force with 'cropping' => 'n' it crops at the top, so cropping is working, it just seems to be ignoring the focus point. The docs at https://processwire.com/api/ref/pageimage/size/ state that both center and true are the default value.

I ended up having to do some really weird hacks:

if($page->image->focus['top'] != '50' && $page->image->focus['left'] != '50') {
    $cropCoords = [round(($page->image->width * $page->image->focus['left'])/100), round(($page->image->height * $page->image->focus['top'])/100)];
}
else {
    $cropCoords = 'center';
}

$page->image->size(330, 330, ['cropping' => $cropCoords])->url;

The check for 50 is not ideal, but that is what is returned when the crop isn't set. Also, regarding the code above that converts % to px - I had to do that because % cropping wasn't working either - it was always generating a p100x100 variation.

Can anyone confirm these current issues I am seeing?

@adrianbj Did you learn anything new on this? Actually I am experiencing similar issues with v3.0.165 in my test environment (Linux), while it works perfectly on my dev machine (Windows) with the same source and database state. Focus coordinates are correct when output directly, but the image is just not cropped according to them.

[adrianbj]

@jaromic - I honestly did look any further. Those hacks got things working for me and I had to move on. Interesting that you noticed it was environment dependent, but also glad to see that someone else is experiencing it also, so it seems like a valid issue that needs fixing. @ryancramerdesign - can you reproduce?

[jaromic]

@adrianbj The problem disappeared when reverting from PHP 8.0 to PHP 7.4 in that test environment which is the version in use on my dev machine as well. So it may be actually specific to either language or PHP config rather than the OS.

                {% set image2 = image.size(1920, 400, {'cropping': true, 'focus': true, 'forceNew': true}) %}
                <img src="{{ image2.URL }}" width="1920" height="400" alt="{{ image2.description }}">

If I can isolate it further, I'll file an issue, but I'll have to go with PHP 7.4 for now.

[adrianbj]

Ah, good catch on the PHP version. This probably does deserve a new issue now we know more. Unfortunately I don't think Ryan has a PHP8 setup yet, so we might need to figure it out for him. I have a feeling it's going to be related to some of the strict (===) checks for either 1 or 0 in https://github.com/processwire/processwire/blob/dev/wire/core/Pageimage.php

but I'd need to do some debugging to figure out where.

[jaromic]

In a minimal test project with the current ProcessWire dev version, I was unable to reproduce the problem on my windows machine so far.

Next, I'll try my project's slightly older ProcessWire version, then a linux test environment, and then both.

[jaromic]

@adrianbj I could reproduce the focus problem with 3.0.165, but not with 3.0.185 on my windows machine. So I am pretty confident that it has been fixed.

3.0.165:

3.0.185: