claviska / SimpleImage

A PHP class that makes working with images and GD as simple as possible.
MIT License
1.38k stars 382 forks source link

PHP 8.0+ version #316

Closed bnomei closed 1 year ago

bnomei commented 1 year ago

this PR

claviska commented 1 year ago

Thanks for bringing the library up to speed with current PHP versions! I'm perfectly fine accepting this PR, but it's going to require a version bump to 4.0 to prevent breaking things for existing users.

Before we proceed, are there any other breaking changes you think should be made?

bnomei commented 1 year ago

like you said in https://github.com/claviska/SimpleImage/issues/284 i would also v4 mainly to upgrade php version. the api in its current state does not need changing imho - it get the job done well.

bnomei commented 1 year ago

EXIF personally i would love to see the exif https://github.com/claviska/SimpleImage/issues/157 issue solved but that would also mean to introduce an option for people that depend on the data being removed to keep it that way. maybe copying exif also has and performance impact (when a bulk of images is created via a CMS etc)

bnomei commented 1 year ago

FORMAT SPECIFIC OPTIONS some GD image methods would accept additional options that use defaults but can not be set right now. maybe all toXXX() api methods could get a new generic $option value that gets expanded as params into the functions if it exists?

example:

    public function toString($mimeType = null, $quality = 100, $options = null)
    {
        return $this->generate($mimeType, $quality, $options)['data'];
    }

    protected function generate($mimeType = null, $quality = 100, $options = null)
    {
        // OTHER CODE
        is_array($options) ? 
           imageavif($this->image, null, $quality, ...$options) :
           imageavif($this->image, null, $quality);
    }

used like this...

   echo $image->toScreen('image/avif', 80, ['speed' => 8]);
claviska commented 1 year ago

maybe all toXXX() api methods could get a new generic $option value that gets expanded as params into the functions if it exists?

Should we also combine $quality into $options since it varies with some formats? Do you want to take this on, or would you rather me do it?

bnomei commented 1 year ago

i will add a suggestion to my PR during the next few days.

bnomei commented 1 year ago

@claviska updated the PR with toXXX() adjustments and added type declarations.

decided that i will not work on the exif issue. feel free to release v4 without it.

claviska commented 1 year ago

I just wanted to say that this PR looks really good. I only noticed that one issue with bestFit() passing a float to resize(), breaking the image. Again, the same problem might be affecting other methods so we may want to check for that.

It's been awhile, but IIRC most of the GD functions require integers so we should probably use round() whenever we divide. Do you want to take a stab at getting those updated as well?

bnomei commented 1 year ago

sure. i can do that.

bnomei commented 1 year ago

there are lots remaining related resource/GDImage/false and since some methods return false instead of throwing an error. but I think it should be good enough for now.

claviska commented 1 year ago

Thanks so much for this. A 4.0.0 tag has been created.