Skip to content
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

analyse() returns NULL #2

Open
seltix5 opened this issue Nov 29, 2018 · 3 comments
Open

analyse() returns NULL #2

seltix5 opened this issue Nov 29, 2018 · 3 comments

Comments

@seltix5
Copy link
Contributor

seltix5 commented Nov 29, 2018

hello,
when running this code with image of 483x600px the analyse() returns ['topCrop' => NULL] :

        $smartcrop = new xymak\image\smartcrop($file_img_resource, ['width' => 300, 'height' => 225]);
        //Analyse the image and get the optimal crop scheme
        $res = $smartcrop->analyse();

After some tests I could find out the analyse() returns this because generateCrops() returns null.
And generateCrops() returns null because $cropWidth = 301 instead of 300 and because of that it never enters the 3rd FOR.
And $cropWidth is that value because in canvasImageScale() the $this->options ['cropWidth'] is set to 301px.

What should be the best solution here?
Thanks.

@johnabela
Copy link

In my own testing of this, which, btw, I have been able to duplicate in the past, analyse() returning null, as you put it, seems to be a result of it not being able to generate any legit 'smart crops' (ie: recommendations).

An easy way to validate that is to place this code (should not be used in production, just using it to show you... some type of try/catch/error should be used in your production code) within the 'function analyse' after the line: $crops = $this->generateCrops();

if ( empty($crops) ) {
	//
	die('We could not generate any suggested crop from this image.');
	//
}

There is clearly something happening inside of generateCrops() that is causing this error, I just have not spent the time to debug it fully. But, at least doing the above could help you put into place some error checking/reporting to keep things from going wonkers on you until @xymak gets around to fixing this.

@seltix5
Copy link
Contributor Author

seltix5 commented Dec 2, 2018

I dont think the problem is inside generateCrops(), I think the problem is in canvasImageScale() because of rounding.
If the cropWidth is set to 300px instead of the 301px generateCrops() works just fine as expected.
I could solve the problem changing canvasImageScale() from :

$this->options ['cropWidth'] = ceil( $this->options ['width'] * $scale );
$this->options ['cropHeight'] = ceil( $this->options ['height'] * $scale );

to:

$this->options ['cropWidth'] = floor ( $this->options ['width'] * $scale );
$this->options ['cropHeight'] = floor ( $this->options ['height'] * $scale );

but I dont know if this change in rounding will cause any problem elsewhere...

@johnabela
Copy link

@seltix5 - I can confirm that the change you noted did resolve the issue on my end as well.

Previously, this image, at 960x720, would fail, but the same image, only slightly smaller, at 600x450, would work.

With the change from ceil() to floor() now they both work.

It is just a matter of rounding up, versus rounding down, and my guess is that in this type of a situation, it is probably not going to matter much. The score() does not seem to be affected much, in my testing over the last few minutes of different images, using both methods. Pretty much identical outputs using both.

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

No branches or pull requests

2 participants