-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Allow cloning of GdImage from gd 2.2.3 #10241
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but maybe @cmb69 wants to have a look?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the PR!
I fully agree that it makes sense to expose gdImageClone()
to userland, but not 100% sure about whether via clone
or a new function (imageclone()
). Since we're having objects now, the former probably makes most sense.
Regarding the libgd version requirements: there are several known bugs in that function, and some have not even been fixed yet (albeit quite serious). Maybe we should require a newer version right away? On the other hand, bugs in external libraries are not uncommon, so we can stick with libgd ≥ 2.2.
The bundled version of gd is at version 2.0.35 so cloning is only supported with
--with-external-gd
ext/gd is lying. :) There have been several updates (bugfixes and features), but the bundled gd never conformed to any particular libgd version. We might update these version numbers, although the long term goal is still to unbundle libgd (what is still blocked by libgd/libgd#335).
Anyhow, we could port gdImageClone()
from libgd right away, so more users could use the new feature. This could alternatively be done via a separate follow-up PR.
The function |
I agree (although devs also may wonder why there are no methods for this class). :) |
With #10225, I agree that the clone operator is the right choice. |
From version 2.2.3 gd supports cloning https://libgd.github.io/manuals/2.2.3/files/gd-c.html#gdImageClone
The bundled version of gd is at version 2.0.35 so cloning is only supported with
--with-external-gd