thephpleague / color-extractor

Extract colors from an image like a human would do.
thephpleague.com
MIT License
1.3k stars 159 forks source link

Basic PHP 8 support #65

Closed sandrolain closed 2 years ago

sandrolain commented 3 years ago

Added usage verification of GD image source as GdImage class instance. Ref issue #64

carestad commented 3 years ago

@MatTheCat could you perhaps review this at your earliest convenience? :smiley:

QuadVector commented 3 years ago

Hello, can it help? https://php.watch/versions/8.0/gdimage

I've just had a problem with object types.

MatTheCat commented 3 years ago

Hey it's been a long time since I came on this repository..!

I believe instanceof does not trigger any error if you test against an undefined class so it would make the condition simpler. 🤔

QuadVector commented 3 years ago

Hello! I use simple code from example for extracting colors from GD resource:

$palette = Palette::fromGD($image);
$colors = $palette->getMostUsedColors($count);

But php 8 throws exception - Image must be a gd resource. I've add check for instanceof \GdImage, if current php version is 8.0 or higher in my pull request - #67

if (PHP_VERSION_ID >= 80000) {
            if (!(is_resource($image) || $image instanceof \GdImage)) {
                throw new \InvalidArgumentException('Image must be a gd resource');
            }
        } else {
            if (!is_resource($image) || get_resource_type($image) != 'gd') {
                throw new \InvalidArgumentException('Image must be a gd resource');
            }
        }

This code helped me. I tested on php 7.4, this also works fine. I think this might be useful.)

QuadVector commented 3 years ago

Yes, it works, thank you! It's much simplier solution.

if (!$image instanceof \GDImage && (!is_resource($image) || 'gd' !== get_resource_type($image))) 
                throw new \InvalidArgumentException('Image must be a gd resource');
            }

Can I add this code into my pull?

MatTheCat commented 3 years ago

Yes go ahead 🙂 Maybe the error message could be changed based on PHP version though?

QuadVector commented 3 years ago

Maybe, but now it's working fine, thanks)) I made pull request #68

colinodell commented 2 years ago

Closing this in favor of #74 since that also comes with working tests to verify the fix. Appreciate the help here, though!