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

[WIP][dont merge] Fix build with image magick 7 #12343

Closed
wants to merge 1 commit into from

Conversation

Izopi4a
Copy link
Member

@Izopi4a Izopi4a commented Oct 21, 2016

If you are using latest image magic 7 build fails because setImageOpacity doesnt exists

build fails with


Zephir\CompilerException: Class 'Imagick' does not implement method: 'setImageOpacity' in /home/izopi4a/cphalcon/phalcon/image/adapter/imagick.zep on line 362

          watermark->setImageOpacity(opacity);
        -------------------------------------^

https://github.com/mkoppanen/imagick/blob/bd9548c426391878905b2a6bc36accc4c79f7ef7/php_imagick_defs.h#L428

i used this to replace it

http://stackoverflow.com/questions/3538851/php-imagick-setimageopacity-destroys-transparency-and-does-nothing

@Jurigag
Copy link
Contributor

Jurigag commented Oct 21, 2016

But this is gonna broke for php5 ? Doesn't it ? Or extension for php5 was changed too ?

@Izopi4a
Copy link
Member Author

Izopi4a commented Oct 21, 2016

the stackoverflow post is from 2010 so it should work.

this method exists in php5 i think. But since the method itself sux and there is a better alternative, why not use the better one instead

@Izopi4a
Copy link
Member Author

Izopi4a commented Oct 21, 2016

actually ... was wrong, this method doesnt exists in image magick 7 .. otherwise it exists always

@sergeyklay sohuld we play it safe and add if / else there

http://php.net/manual/en/imagick.getversion.php

or just leave it like this ?

@Jurigag
Copy link
Contributor

Jurigag commented Oct 21, 2016

Well. Maybe if else. I guess we can't in minor version deprecate only some related extension version or something ? Anyway i doubt that everyone is going to update imagick.

@Izopi4a
Copy link
Member Author

Izopi4a commented Oct 21, 2016

well since the guy says that this method is not ok, and since it has been removed, probably this new implementation is better ... dont know really, i am not an image magick expert. I do compile it from souce and upgrade it very often cuz of the CVEs that it has from time to time, and I use it for processing many many things :D

But i don't know, as you decide i will update the PR accordingly

@sergeyklay
Copy link
Contributor

Class '\Imagick' does not have a constant called: 'EVALUATE_MULTIPLY' in C:\projects\phalcon\phalcon\image\adapter\imagick.zep on line 362

@Izopi4a
Copy link
Member Author

Izopi4a commented Oct 22, 2016

ok i will dive deep into it to see whats the best solution there and let you know when its ok

@Izopi4a Izopi4a changed the title fix php7 build with latest imagick [WIP] Fix build with image magick 7 Oct 22, 2016
@Izopi4a Izopi4a changed the title [WIP] Fix build with image magick 7 [WIP][dont merge] Fix build with image magick 7 Oct 22, 2016
@Izopi4a
Copy link
Member Author

Izopi4a commented Oct 24, 2016

ok its just typo in php imagick.

Imagick/imagick#180 (comment)

@Izopi4a Izopi4a closed this Oct 24, 2016
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 this pull request may close these issues.

3 participants