-
Notifications
You must be signed in to change notification settings - Fork 7.9k
ext/gd: adding imagecopyrotated. #11519
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
FWIW, I tried this on my system and it didn't work because of a ZPP arginfo mismatch. I used the following patch to fix it: diff --git a/ext/gd/gd.c b/ext/gd/gd.c
index b2084dba3d..6309088d00 100644
--- a/ext/gd/gd.c
+++ b/ext/gd/gd.c
@@ -1062,9 +1062,9 @@ PHP_FUNCTION(imagecopyrotated)
zend_long SX, SY, SW, SH, A;
gdImagePtr im_dst, im_src;
int srcH, srcW, srcY, srcX, angle;
- double dstX, dstY;
+ zend_long dstX, dstY;
- if (zend_parse_parameters(ZEND_NUM_ARGS(), "OOddlllll", &DIM, gd_image_ce, &SIM, gd_image_ce, &dstX, &dstY, &SX, &SY, &SW, &SH, &A) == FAILURE) {
+ if (zend_parse_parameters(ZEND_NUM_ARGS(), "OOlllllll", &DIM, gd_image_ce, &SIM, gd_image_ce, &dstX, &dstY, &SX, &SY, &SW, &SH, &A) == FAILURE) {
RETURN_THROWS();
}
diff --git a/ext/gd/gd.stub.php b/ext/gd/gd.stub.php
index 93821b6182..c1e180bce7 100644
--- a/ext/gd/gd.stub.php
+++ b/ext/gd/gd.stub.php
@@ -521,7 +521,7 @@ function imagecolorexactalpha(GdImage $image, int $red, int $green, int $blue, i
function imagecopyresampled(GdImage $dst_image, GdImage $src_image, int $dst_x, int $dst_y, int $src_x, int $src_y, int $dst_width, int $dst_height, int $src_width, int $src_height): bool {}
#ifdef HAVE_GD_COPY_ROTATED
-function imagecopyrotated(GdImage $dst_image, GdImage $src_image, int $dst_x, int $dst_y, int $src_x, int $src_y, int $dst_width, int $dst_height, int $src_width, int $src_height, int $angle): bool {}
+function imagecopyrotated(GdImage $dst_image, GdImage $src_image, int $dst_x, int $dst_y, int $src_x, int $src_y, int $src_width, int $src_height, int $angle): bool {}
#endif
#ifdef PHP_WIN32
diff --git a/ext/gd/gd_arginfo.h b/ext/gd/gd_arginfo.h
index 1e3a0fb914..8671ee395d 100644
--- a/ext/gd/gd_arginfo.h
+++ b/ext/gd/gd_arginfo.h
@@ -1,5 +1,5 @@
/* This is a generated file, edit the .stub.php file instead.
- * Stub hash: 26b28dde3a4b4eeca374780d8972adb14bb6e795 */
+ * Stub hash: 8235f4095cd07d7b6936be153fa7c537bc860f8b */
ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_gd_info, 0, 0, IS_ARRAY, 0)
ZEND_END_ARG_INFO()
@@ -107,15 +107,13 @@ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_imagecopyresampled, 0, 10, _IS_B
ZEND_END_ARG_INFO()
#if defined(HAVE_GD_COPY_ROTATED)
-ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_imagecopyrotated, 0, 11, _IS_BOOL, 0)
+ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_imagecopyrotated, 0, 9, _IS_BOOL, 0)
ZEND_ARG_OBJ_INFO(0, dst_image, GdImage, 0)
ZEND_ARG_OBJ_INFO(0, src_image, GdImage, 0)
ZEND_ARG_TYPE_INFO(0, dst_x, IS_LONG, 0)
ZEND_ARG_TYPE_INFO(0, dst_y, IS_LONG, 0)
ZEND_ARG_TYPE_INFO(0, src_x, IS_LONG, 0)
ZEND_ARG_TYPE_INFO(0, src_y, IS_LONG, 0)
- ZEND_ARG_TYPE_INFO(0, dst_width, IS_LONG, 0)
- ZEND_ARG_TYPE_INFO(0, dst_height, IS_LONG, 0)
ZEND_ARG_TYPE_INFO(0, src_width, IS_LONG, 0)
ZEND_ARG_TYPE_INFO(0, src_height, IS_LONG, 0)
ZEND_ARG_TYPE_INFO(0, angle, IS_LONG, 0)
|
in fact it s the other way around the stub is wrong. |
673e7c3
to
fe9a41a
Compare
ext/gd/gd.stub.php
Outdated
@@ -520,6 +520,10 @@ function imagecolorexactalpha(GdImage $image, int $red, int $green, int $blue, i | |||
|
|||
function imagecopyresampled(GdImage $dst_image, GdImage $src_image, int $dst_x, int $dst_y, int $src_x, int $src_y, int $dst_width, int $dst_height, int $src_width, int $src_height): bool {} | |||
|
|||
#ifdef HAVE_GD_COPY_ROTATED | |||
function imagecopyrotated(GdImage $dst_image, GdImage $src_image, float $dst_x, float $dst_y, int $src_x, int $src_y, int $src_width, int $src_height, int $angle): bool {} |
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.
angle should be float
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.
I follow gd api here.
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.
I don't know what to think of the API, but then other GD functions do pass the destination object too...
ext/gd/gd.c
Outdated
PHP_FUNCTION(imagecopyrotated) | ||
{ | ||
zval *SIM, *DIM; | ||
zend_long SX, SY, SW, SH, A; | ||
gdImagePtr im_dst, im_src; | ||
int srcH, srcW, srcY, srcX, angle; | ||
double dstX, dstY; | ||
|
||
if (zend_parse_parameters(ZEND_NUM_ARGS(), "OOddlllll", &DIM, gd_image_ce, &SIM, gd_image_ce, &dstX, &dstY, &SX, &SY, &SW, &SH, &A) == FAILURE) { | ||
RETURN_THROWS(); | ||
} | ||
|
||
im_dst = php_gd_libgdimageptr_from_zval_p(DIM); | ||
im_src = php_gd_libgdimageptr_from_zval_p(SIM); | ||
|
||
srcX = SX; | ||
srcY = SY; | ||
srcH = SH; | ||
srcW = SW; | ||
angle = A; | ||
|
||
gdImageCopyRotated(im_dst, im_src, dstX, dstY, srcX, srcY, srcW, srcH, angle); | ||
RETURN_TRUE; | ||
} |
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.
It doesn't really make any sense to take the destination Image as an argument, as I frankly imagine one will not have pre allocated such an object.
How do other functions from the GD extension work?
PHP_FUNCTION(imagecopyrotated) | |
{ | |
zval *SIM, *DIM; | |
zend_long SX, SY, SW, SH, A; | |
gdImagePtr im_dst, im_src; | |
int srcH, srcW, srcY, srcX, angle; | |
double dstX, dstY; | |
if (zend_parse_parameters(ZEND_NUM_ARGS(), "OOddlllll", &DIM, gd_image_ce, &SIM, gd_image_ce, &dstX, &dstY, &SX, &SY, &SW, &SH, &A) == FAILURE) { | |
RETURN_THROWS(); | |
} | |
im_dst = php_gd_libgdimageptr_from_zval_p(DIM); | |
im_src = php_gd_libgdimageptr_from_zval_p(SIM); | |
srcX = SX; | |
srcY = SY; | |
srcH = SH; | |
srcW = SW; | |
angle = A; | |
gdImageCopyRotated(im_dst, im_src, dstX, dstY, srcX, srcY, srcW, srcH, angle); | |
RETURN_TRUE; | |
} | |
PHP_FUNCTION(imagecopyrotated) | |
{ | |
zval *SIM; | |
zend_long SX, SY, SW, SH, A; | |
gdImagePtr im_dst, im_src; | |
int srcH, srcW, srcY, srcX, angle; | |
double dstX, dstY; | |
if (zend_parse_parameters(ZEND_NUM_ARGS(), "Oddlllll", &SIM, gd_image_ce, &dstX, &dstY, &SX, &SY, &SW, &SH, &A) == FAILURE) { | |
RETURN_THROWS(); | |
} | |
im_dst = create_new_object_here(); | |
im_src = php_gd_libgdimageptr_from_zval_p(SIM); | |
srcX = SX; | |
srcY = SY; | |
srcH = SH; | |
srcW = SW; | |
angle = A; | |
gdImageCopyRotated(im_dst, im_src, dstX, dstY, srcX, srcY, srcW, srcH, angle); | |
RETURN_NEW_OBJECT(); | |
} |
EDIT: Okay I see that other GD functions already do this, but am I the only one to find this stupid or? In any case if the function can only fail when throwing an exception make the function void.
ac5dc7e
to
df651b7
Compare
cc @Girgias, possibly before the freeze 🙏🙂 |
|
||
gdImageCopyRotated(im_dst, im_src, dstX, dstY, srcX, srcY, srcW, srcH, angle); |
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.
Only thing would be to check that the zend_long
being passed fits into the value storable by a C int
and if not throw a ValueError like we do in other functions
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.
How does it look to you now ?
copying an image area from a defined center position with an angle.
df651b7
to
26fddf7
Compare
If we add this function, we should also add However, I'm not sure we should add more of the old-style |
copying an image area from a defined center position with an angle.