spatie / statamic-responsive-images

Responsive images for Statamic 3
MIT License
99 stars 29 forks source link

Configurable class for DimensionCalculator #204

Closed bengs closed 1 year ago

bengs commented 1 year ago

Hi, for some reason I've not been able to override DimensionCalculator::class using the example in the recently merged pull request #193 in my AppServiceProvider.php file (maybe the package boot binding order prevents this?) so this pull request changes its binding to use a configurable class, similar to how Spatie Media Library does it. I was then able to override it with a custom class configurable in config/responsive-images.php eg:

    /*
    |--------------------------------------------------------------------------
    | Dimension Calculator Class
    |--------------------------------------------------------------------------
    |
    | The class that will be used for the breakpoint srcset calculations
    |
    */

    'dimension_calculator' => \App\Media\MyDimensionCalculator::class,
ncla commented 1 year ago

I am interested what went wrong for you. Could you please share your code on how you tried to bind your implementation (Service Provider and DimensionCalculator class)?

bengs commented 1 year ago

Sure here's a public repo I just created you can use for testing using a clean Statamic Starter Kit Multisimplicity install. I then copied the code from your pull request #193 in to AppServiceProvider.php and tested with returning null at the top of calculateForBreakpoint() - it doesn't seem to have any effect on the image srcset outputs if you visit the /rooms page. Did I override it it incorrectly?

/**
 * Bootstrap any application services.
 *
 * @return void
 */
public function boot()
{
      $this->app->bind(DimensionCalculator::class, function () {
          return new class extends ResponsiveDimensionCalculator {
              public function calculateForBreakpoint(Source $source): Collection
              {
                  // return anything here it doesn't seem to have any effect eg return null;

                  // Disable JPG sources
                  if ($source->getFormat() === 'original') {
                      return collect([]);
                  }

                  // Create widths from 100 to 2000 in 100px increments with fixedHeight passed from params
                  return collect(range(100, 2000, 100))
                      ->map(function ($width) use ($source) {
                          return new Dimensions($width, $source->breakpoint->params['fixedHeight']);
                      });
              }
          };
      });
}
bengs commented 1 year ago

In my private app (using this pull request with configurable class) I copied the ResponsiveDimensionCalculator class to App\Media and modified the calculateDimensions() and finishedCalculating() methods to use 2 new config variables:

'predicted_filesize' => 0.4,
'min_srcset_width' => 300,

\App\Media\ResponsiveDimensionCalculator.php

<?php

namespace App\Media;

use Illuminate\Support\Collection;
use Spatie\ResponsiveImages\Source;
use Statamic\Contracts\Assets\Asset;
use Spatie\ResponsiveImages\Breakpoint;
use Spatie\ResponsiveImages\Dimensions;
use Spatie\ResponsiveImages\DimensionCalculator;

/**
 * The original, file-size, aspect-ratio based dimension calculator.
 */
class ResponsiveDimensionCalculator implements DimensionCalculator
{
    public function calculateForBreakpoint(Source $source): Collection
    {
        $asset = $source->breakpoint->asset;
        $width = $asset->width();
        $height = $asset->height();
        $fileSize = $asset->size();

        $ratio = $this->breakpointRatio($asset, $source->breakpoint);
        $glideParams = $source->breakpoint->getImageManipulationParams();

        return $this
            ->calculateDimensions($fileSize, $width, $height, $ratio)
            ->sort()
            // Filter out widths by max width
            ->when((isset($glideParams['width']) || config('statamic.responsive-images.max_width') !== null), function ($dimensions) use ($glideParams, $ratio) {
                $maxWidth = $glideParams['width'] ?? config('statamic.responsive-images.max_width');

                $filtered = $dimensions->filter(function (Dimensions $dimensions) use ($maxWidth) {
                    return $dimensions->getWidth() <= $maxWidth;
                });

                // We want at least one width to be returned
                if (! $filtered->count()) {
                    $filtered = collect([
                        new Dimensions($maxWidth, round($maxWidth / $ratio)),
                    ]);
                }

                return $filtered;
            });
    }

    public function calculateForImgTag(Breakpoint $breakpoint): Dimensions
    {
        $maxWidth = ($breakpoint->parameters['glide:width'] ?? config('statamic.responsive-images.max_width') ?? null);

        $ratio = $this->breakpointRatio($breakpoint->asset, $breakpoint);

        $width = $maxWidth ?? $breakpoint->asset->width();

        return new Dimensions($width, round($width / $ratio));
    }

    public function calculateForPlaceholder(Breakpoint $breakpoint): Dimensions
    {
        return new Dimensions(32, 32 / $this->breakpointRatio($breakpoint->asset, $breakpoint));
    }

    public function breakpointRatio(Asset $asset, Breakpoint $breakpoint): float
    {
        return $breakpoint->parameters['ratio'] ?? ($asset->width() / $asset->height());
    }

    private function calculateDimensions(int $assetFilesize, int $assetWidth, int $assetHeight, $ratio): Collection
    {
        $dimensions = collect();

        $dimensions->push(new Dimensions($assetWidth, round($assetWidth / $ratio)));

        // For filesize calculations
        $ratioForFilesize = $assetHeight / $assetWidth;
        $area = $assetHeight * $assetWidth;

        $predictedFileSize = $assetFilesize;
        $pixelPrice = $predictedFileSize / $area;

        while (true) {
            // $predictedFileSize *= 0.7;
            $predictedFileSize *= config('statamic.responsive-images.predicted_filesize', 0.7);

            $newWidth = (int) floor(sqrt(($predictedFileSize / $pixelPrice) / $ratioForFilesize));

            if ($this->finishedCalculating($predictedFileSize, $newWidth)) {
                return $dimensions;
            }

            $dimensions->push(new Dimensions($newWidth, round($newWidth / $ratio)));
        }
    }

    private function finishedCalculating(float $predictedFileSize, int $newWidth): bool
    {
        if ($newWidth < config('statamic.responsive-images.min_srcset_width', 20)) {
            return true;
        }

        if ($predictedFileSize < (1024 * 10)) {
            return true;
        }

        return false;
    }
}
ncla commented 1 year ago

In AppServiceProvider you had not imported DimensionCalculator through use statement.

use Spatie\ResponsiveImages\DimensionCalculator;

This will then reveal that you cannot return null value, it must be Collection, even if it is empty.

This works and the end result I see is that there are no <source> tags in the <picture>.

ncla commented 1 year ago

I am gonna for now close this PR however. You should be able bind it now with success. I do not feel that confident yet to publicly document this feature, including in a config file, if that makes sense (even if I documented this feature in CHANGELOG.. :sweat_smile: ). I definitely have thought about this when I was making the PR so maybe in the future.

But it is nice however to see that it is already being used and is found useful.

bengs commented 1 year ago

Ah sorry VS Code code usually highlights missing imports but didn't for that one 🤦 (I guess because inside bind it wasn't throwing an error). I was intentionally trying to cause an error with return null in the demo (instead of collect) to detect if if the binding was being run. Yes I'm totally fine with binding it like your example - thanks for your help and the new feature it's great - in a big multisite app I wanted to reduce the number of srcsets and your extensible calculator allowed me to do that.