imagepolygon() – number of points

php.internals

Christoph Becker

9 years ago
Hi! This issue has been filed to our bugtracker years ago[1], but didn't deserve much attention, so I'm forwarding to the list. imagepolygon(), imagefilledpolygon() and imageopenpolygon() have the following signature: image*polygon(resource $im, array $points, int $num_points, int $col) Note the third parameter, $num_points, which doesn't make much sense for PHP, and has most likely being directly ported from the underlying libgd implementation of gdImage*Polygon[2]. Even worse, while the common implementation[3] checks that $points (which would better have been called $coordinate, by the way) has at least 6 elements, $num_points can be smaller than 3, what will cause gdImage*Polygon() to be called with only so many points. That is not an issue per se, as gdImage*Polygon() is capable of handling monogons and digons, but of course it doesn't make sense to force the programmer to construct a larger array with unused values to do so. Furthermore the imagefilledpolygon() man page[4] states that $num_points must be greater than or equal to 3, but neither of the other man pages document this. In the long run (i.e. for PHP 8) I'd suggest to purge the $num_points parameter altogether, relying solely on the number of $points given. For PHP 7.2 I propose to weaken the constraint on count($points) so that 2 would be sufficient instead of the 6 required now. Would the proposed change for PHP 7.2 require an RFC, or would a PR suffice? [1] <https://bugs.php.net/bug.php?id=55005> [2] <http://libgd.github.io/manuals/2.2.4/files/gd-c.html#gdImagePolygon> [3] <https://github.com/php/php-src/blob/PHP-7.1.1/ext/gd/gd.c#L3355-L3413> [4] <http://php.net/manual/en/function.imagefilledpolygon.php>
-- Christoph M. Becker

Rowan Collins

9 years ago
On 21/01/2017 17:42, Christoph M. Becker wrote:
> In the long run (i.e. for PHP 8) I'd suggest to purge the $num_points > parameter altogether, relying solely on the number of $points given.
Since there's a 4th parameter ($color), what would "purge" mean in this case? Reducing the number of parameters to 3 would be very confusing, because existing code passing $num_points would be interpreted as passing that value as $color. We could ignore the argument whatever value was provided; or we could document that it should be passed as null, and raise a warning if it's anything else; or we could ignore it if it matched the length of $points, and raise a warning or error if it didn't... Regards,
-- Rowan Collins [IMSoP]

Christoph Becker

9 years ago
On 21.01.2017 at 19:11, Rowan Collins wrote:
> On 21/01/2017 17:42, Christoph M. Becker wrote: > >> In the long run (i.e. for PHP 8) I'd suggest to purge the $num_points >> parameter altogether, relying solely on the number of $points given. > > Since there's a 4th parameter ($color), what would "purge" mean in this > case? Reducing the number of parameters to 3 would be very confusing, > because existing code passing $num_points would be interpreted as > passing that value as $color. > > We could ignore the argument whatever value was provided; or we could > document that it should be passed as null, and raise a warning if it's > anything else; or we could ignore it if it matched the length of > $points, and raise a warning or error if it didn't...
I haven't really though about how to do that, because PHP 8 is too far away, and for now I'm interested in a clean solution that can be applied to PHP 7(.2)
-- Christoph M. Becker