Skip to content

[Lazy-Image] Support for intervention/image v3 #1465

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Jibbarth opened this issue Feb 6, 2024 · 6 comments · Fixed by #1766
Closed

[Lazy-Image] Support for intervention/image v3 #1465

Jibbarth opened this issue Feb 6, 2024 · 6 comments · Fixed by #1766

Comments

@Jibbarth
Copy link
Contributor

Jibbarth commented Feb 6, 2024

Hi there !

While inspecting my dependencies, I just noticed that intervention/image had a new v3 version.

My project use ux-lazy-image. I was able to upgrade intervention/image to the last version, and few problems appears.

On LazyImageExtension, the instance of ImageManager need a driver now (Driver class or an instance of driver). Could be interesting to check if we have Imagick or GD available at this moment to pass with correct driver. However, if we need to support both v2 and v3, i think we should add more adjustment:

class LazyLoadExtension extends Extension implements PrependExtensionInterface
{
    public function load(array $configs, ContainerBuilder $container)
    {
        if (class_exists(ImageManager::class)) {
            $definition = new Definition(ImageManager::class);
            if (class_exists(\Imagick::class)) {
                $definition->addArgument(\Intervention\Image\Drivers\Imagick\Driver::class);
            } else if (class_exists(\GdImage::class)) {
                $definition->addArgument(\Intervention\Image\Drivers\Gd\Driver::class);
            }
            $container
                ->setDefinition('lazy_image.image_manager', $definition)
                ->setPublic(false)
            ;
        }
        // ...

Then, in the BlurHash service, there is few changes in the API provided by the imageManager.

I listed following :

  • make() -> read()
  • pixel() -> drawPixel() (and x,y in first args)
  • canvas() -> create()
  • pickColor return now a ColorInterface object, that can be converted in array with $color->toArray()
  • getWidth() and getHeight() -> width() and height() on Image object

After made all this changes, I ended up with a "Unable to decode input" exception 🙈

Possible solutions

As a quickfix and to avoid the issue for new user that directly install the latest version of intervention/image, we can add a conflic to that v3.

If we don't want to support both version, we can work on the BlurHash service to migrate with the new API (https://image.intervention.io/v3/introduction/upgrade)

If we wan't keep support of that version, and to avoid lot of "method_exist" in the BlurHash service, we maybe should add a InterventionV3BlurHash service and inject it in BlurHash interface when we detect such a version (don't know if it's possible to guess version in the bundle extension by the way).

WDYT ?

I can handle modifications after your feedback.

@smnandre
Copy link
Member

What about a conflict with intervention V3 in this version, and then moving to https://github.com/bepsvpt/blurhash ?

The current lib used seems not very active, and if we can use a single (small) library instead of Blurhash + Intervention2/3 + "method_exist" code... WDYT ?

@Jibbarth
Copy link
Contributor Author

Haha I forgot there was another lib between ux-lazy-image and intervention/image 😅

The only counterpart I see using the one you suggested is the dependency with illuminate/support that come with lot of dependency too.

The current lib was updated recently to explain how to use with latest version of intervention/image : kornrunner/php-blurhash@e3b9b36

@smnandre
Copy link
Member

Oh you're right... well if you feel to adapt the code for both Intervention2 & 3 that'd be ideal i guess

@Jibbarth
Copy link
Contributor Author

Thanks for feedback. I'll give it a try 😉

@smnandre
Copy link
Member

There is another possibility i find appealing : https://github.com/SRWieZ/thumbhash/

@Jibbarth
Copy link
Contributor Author

Yes, saw it and I wondering if it was a good alternative. I should try it 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants