Update ext/gd parameter names #6308
Conversation
|
Soo we're almost done!
ext/gd has a nice track record for this :) I can remember about 1-2 similar cases :D |
There was a problem hiding this comment.
Thanks! Looks good to me.
Regarding the ignoretransparent parameter of imagerotate(): this has been added as "bug" fix, but basically removed when the improved rotate functions became available which don't support this "feature" any longer, because they enforce conversion to true color, where the transparent color is not relevant. There is a comment in the code which points out that palette color support could be added at a later time, but I have doubts that will ever happen; I'd rather get rid of internal palette image support at all (IMO the tighter memory footprint of palette images is not worth the trouble for userland developers as well as libgd maintainers).
ext/gd/gd.stub.php
Outdated
| @@ -52,13 +52,13 @@ function imagegrabscreen(): GdImage|false {} | |||
|
|
|||
| #endif | |||
|
|
|||
| function imagerotate(GdImage $im, float $angle, int $bgdcolor, int $ignoretransparent = 0): GdImage|false {} | |||
| function imagerotate(GdImage $image, float $angle, int $bgdcolor, int $ignoretransparent = 0): GdImage|false {} | |||
There was a problem hiding this comment.
Maybe just $bgcolor instead of $bgdcolor?
There was a problem hiding this comment.
Hrm, it looks like I missed to commit part of my changes :/ This was supposed to be $background_color/$foreground_color. Though if those are too long, I'm also fine with $bg_color and $fg_color.
There was a problem hiding this comment.
I'm fine with $background_color/$foreground_color.
Any thoughts on what we should do with the parameter? Should we just keep it as is (ignored), or should we drop it? |
Hmm, not sure if we can still drop it for PHP 8.0. If not probably better go through deprecation for 8.1. |
I've also changed a couple of boolean
intparameters tobool. While doing that I noticed thatimagerotate()has an$ignore_transportargument that is ... ignored :) What's up with that?