We have 3 presets on the site:
preset_A
preset_A_horizontal
preset_A_vertical

in preset_A, the only action configured is the aspect switcher.

When preset_A settings are edited, all cached images under preset_A are flushed.
But they should also be flushed when preset_A_horizontal or preset_A_vertical edited.

Comments

dman’s picture

Good point! I hadn't thought of that.

dman’s picture

Status: Active » Postponed

I've tried looking at that ... the code would need to be quite custom - more custom than imagecache currently allows.
There's no hook or trigger to use to send to the parent presets.
I'll just have to leave that as a known limitation.

fietserwin’s picture

Version: 6.x-1.5 » 7.x-1.x-dev
Component: Canvas Actions Module » Code
Status: Postponed » Active

D7 does:
- provide a hook when saving image styles: hook_image_style_save
- provide an API function to flush a style: image_style_flush()

So easy to solve in D7. Should also be solved for the subroutine effect from custom actions. For D6 this will be a won't fix anymore: there is a workaround by manually flushing a preset (not existing anymore in D7...) and changing styles is not that common, it is more a 1-time action.

fietserwin’s picture

Title: Aspect Switcher -- Need to Flush Presets » Aspect Switcher / Subroutine: Need to Flush image style on changes in called style
Status: Active » Fixed

Turned out that I needed to implement hook_image_style_flush(). hook_image_style_save() is not called at all moments we would want to flush other styles as well.

@dman: can you look into the @todo's and questions (and answer them) I added to canvasactions.inc?

dman’s picture

In testing from the investigation below, it looks like the presets are getting flushed correctly now. That's cool.

// @todo:
// - should we allow user to select between > 1 or >= 1? (i.e. let the user
// decide what to do when the image has equal dimensions)

That's what the extra ratio adjustment is for.

If given an image 100x100, and you set the ratio adjustment to 0.9, squarish images will tend to be treated as portrait.
Set it to 0.75, and everything more upright than a 4:3 landscape will be treated as a portrait.
(Actually, I found a bug there where the ratio was inexplicably cast to an int. Fixing that now)

If you really really need to have three choices, there are instructions for chaining two effects in the file help/aspect_switcher.html
The overload that providing and explaining all the options would put on the UI was really difficult, and greater and more flexible and unpredictable results could be built from allowing chaining, so that's there. This way also allows you to define the margin of error - how square is mostly square - rather than pixel perfect.

// - form field are not required: this suggests that if the image has the
// orientation for which no image style is defined, nothing will be executed
// This seems a good feature to me. However the code below logs an error.

An oversight if anything. I don't think allowing nothing was intentional at the time. It's unexpected that

$styles = image_style_options(TRUE);

would give us a "none" option, so may have been a FAPI update that introduced that.
However, yeah, there is no reason why we can't just allow none and return immediately.

I've tested that, it works. Added a note to the form to explain that it's allowed and nothing happens.

// - The code below checks for an empty style NAME after loading the STYLE
// data. This does not seem logical to me.

Maybe an errant search & replace on upgrade. Yes that should have been checking if the style still existed.
Fixing that meant that ^ could work as described.

// - AFAIK, the effects do implement the percent and keyword filter?

Um.. it tried to pass them through.
That was copied code from the original internals as there was no way (at the time) to invoke a process cleanly. the old action harness used to do that before calling the scale/crop action setc, so I had to also. If the parameters for that have changed (it only knows about the xoffset, etc that the common core processes used), then it may well need reviewing.
Looks like now that image_effect_apply() is available (wasn't in D6) we don't need to and shouldn't do that ourselves any more.
.. Yeah, that just can be deleted. I can't come up with a case where it's needed any more.
. done.

... Man I find hook_image_dimensions annoying...
I've added the passthrough as needed now. Should work. *sigh*

OK, so that's all addressed. Pushing it up now.

Tested only on aspect-switcher + core scale/crop and imagecache_actions define_canvas so far. May be side effects elsewhere? Should be fine though.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.