Performance concerns regarding size calculation

[tractorcow]

At the moment, this extension has to generate an Image_Backend for every image render, even if the file generated exists on the filesystem.

If you look at core code, you can see the pattern used here:

public function Fit($width, $height)
    {
        $width = $this->castDimension($width, 'Width');
        $height = $this->castDimension($height, 'Height');
        // Item must be regenerated
        $variant = $this->variantName(__FUNCTION__, $width, $height);
        return $this->manipulateImage($variant, function (Image_Backend $backend) use ($width, $height) {
            // Check if image is already sized to the correct dimension
            $currentWidth = $backend->getWidth();
            $currentHeight = $backend->getHeight();
            if (!$currentWidth || !$currentHeight) {
                return null;
            }

However focuspoint extension calls getWidth() / getHeight() outside of a manipulateImage() wrapper.

//Only resize if necessary
        if ($image->isSize($width, $height) && !Config::inst()->get(DBFile::class, 'force_resample')) {
            return $image;
        } elseif ($cropData = $this->calculateCrop($width, $height, $imgW, $imgH)) {
            $variant = $image->variantName(__FUNCTION__, $width, $height, $cropData['CropAxis'], $cropData['CropOffset']);
            $cropped = $image->manipulateImage($variant, function (Image_Backend $backend) use ($width, $height, $cropData) {
                $img = null;

This means that rendering a page with lots of images is very slow, and has the potential to run out of memory, even if those images have already been generated.

Suggestion is to flip all code and ensure that we wrap all generators inside a manipulateImage()

[jonom]

@tractorcow I think you probably meant to open this on the silverstripe-focuspoint repo 😄

I think this is a duplicate of this issue - do you want to continue the conversation there? Loz provided a fix but I had to revert it as it broke the ability to update the focus point coordinates on the resampled image.

I've tried to fix the performance problem a few times while retaining that feature but couldn't crack it. You could potentially use Loz's fork as a performance fix in your projects, or maybe you could work out a permanent solution since you are smarter than me :). From memory I think this was part of the puzzle too.

If it can't be fixed I'll probably need to do a new major version that includes Loz's fix and drops support for passing through the focus point coordinates. I've used that in several projects myself but I doubt it's used a lot by other people (who knows though).

[tractorcow]

Oh oops, sorry I posted this on the wrong repo. No wonder I couldn't find it in the issues list!

I will look at the resolution Loz suggested and suggest another fix.

[tractorcow]

you are smarter than me :)

Absolutely not. I am just more desperate and thus gives rise to solutions. :)