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

GD 2.1.0 support for PHP 5.5 #334

Closed
wants to merge 7 commits into from
Closed

GD 2.1.0 support for PHP 5.5 #334

wants to merge 7 commits into from

Conversation

oerdnj
Copy link
Contributor

@oerdnj oerdnj commented Apr 25, 2013

This adds almost all support for GD ext features (minus antialias functionality) using GD-2.1.0 library.

The extension compiles just fine with this patch, but it might need a review for GD-2.0 and GD-BUNDLED. Didn't have a time yet to try all variants.

@remicollet
Copy link
Member

Thanks for this great work !

Using a single HAVE_LIBGD21 seems a better idea than a specific flag for each function.

Some comments

  • build with bundled copy doesn't work

    undefined symbol: gdSetErrorMethod

If we use, (defined(HAVE_GD_BUNDLED) || defined(HAVE_LIBGD21)) we don't need to define HAVE_LIBGD21 with builtin library (which explain the above issue)

Having HAVE_LIBGD21 with bundled will allow to make conditional stuff simpler, but we need a placeholder for gdSetErrorMethod

We can probably also use HAVE_LIBGD21 instead of HAVE_GD_IMAGE_FLIP (need to check if it is really new in 2.1)

  • gd_compat.h should be clean of overflow2, getmbi and skipheader as removed from gd_compat.c
  • I haven't look deeply, but is getbmi + skipheader really the same that _php_ctx_getmbi ?
  • missing return in _php_ctx_getmbi .
  • we loose the imagecreatefromxbm (seems some conditional missing)

We have tons of failed tests with the external library need to compare with the bundled.

@remicollet
Copy link
Member

Note, with bundled library I only have 2 failed tests

  • Bug #43073 (TrueType bounding box is wrong for angle<>0) freetype < 2.4.10 [ext/gd/tests/bug43073.phpt]
  • Bug #48801 (Problem with imagettfbbox) freetype < 2.4.10 [ext/gd/tests/bug48801.phpt]

@dsp
Copy link
Member

dsp commented Apr 26, 2013

I would love to see GD 2.1 in PHP, but I think we cannot aim for 5.5.0. I haven't tested the patch itself, but I think we should aim for 5.6.0.

@remicollet
Copy link
Member

@dsp of course I agree with 5.6 as a target for bundled libgd 2.1 (or 2.2)

But this patch (and some already committed changes) should (must) not change anything in 5.5 when using bundled libgd. Goal is, I think, just to allow (and improve) the possible use of system libgd when 2.1 is available.

@oerdnj
Copy link
Contributor Author

oerdnj commented Apr 26, 2013

Yup, no change in PHP bundled libgd, but at the same time allow building with libgd 2.1.0.

For 5.6.0 I think we can rip-out bundled libgd/ completely, since we have Pierre acting as upstream for libgd, so we can sync the needed changes very quickly. Or at least keep the changes in bundle libgd to minimal patch.

@pierrejoye
Copy link
Contributor

@dsp we already have 2.1 in 5.5, this is only about supporting system 2.1 in 5.5+. These changes are only about config and #ifdef, no new code in ext/gd

@pierrejoye
Copy link
Contributor

@remicollet @oerdnj you both have access to ext/gd, go ahead to make system's gd works out of the box with 5.5. That's something you have been waiting for too long already :)

@oerdnj
Copy link
Contributor Author

oerdnj commented May 3, 2013

@remicollet Hi Remi, could you review this last bunch of updates?

This is no longer very cosmetic, but major surgery. GD 2.0.x support is broken in PHP 5.5.0 anyway, so I see no harm in supporting only external GD 2.1.0.

Otherwise I can build with --with-gd=yes (BUNDLED) and --with-gd=/usr (SYSTEM).

However you are correct that the number of failed tests is much higher:
=====================================================================
FAILED TEST SUMMARY
---------------------------------------------------------------------
Bug #22544 (TrueColor transparency in PNG images). [tests/bug22544.phpt]
Bug #39780 (PNG image with CRC/data error raises a fatal error) [tests/bug39780_extern.phpt]
Bug #43073 (TrueType bounding box is wrong for angle<>0) freetype < 2.4.10 [tests/bug43073.phpt]
Bug #43073 (TrueType bounding box is wrong for angle<>0) freetype >= 2.4.10 [tests/bug43073_1.phpt]
Bug #43121 (gdImageFill with IMG_COLOR_TILED crashes httpd) [tests/bug43121.phpt]
Bug #45799 (imagepng() crashes on empty image). [tests/bug45799.phpt]
Bug #48732 (TTF Bounding box wrong for letters below baseline) [tests/bug48732.phpt]
Bug #48801 (Problem with imagettfbbox) freetype < 2.4.10 [tests/bug48801.phpt]
Bug #48801 (Problem with imagettfbbox) freetype >= 2.4.10 [tests/bug48801_1.phpt]
Bug #51671 (imagefill does not work correctly for small images) [tests/bug51671.phpt]
Bug #60160 (imagefill does not work correctly for small images) @see bug51671 [tests/bug60160.phpt]
imagecreatefromwbmp with invalid wbmp [tests/createfromwbmp2_extern.phpt]
Testing imagearc() of GD library [tests/imagearc_basic.phpt]
Testing wrong param passing imagearc() of GD library [tests/imagearc_error1.phpt]
Testing passing negative end angle to imagearc() of GD library [tests/imagearc_variation1.phpt]
Testing passing negative start angle to imagearc() of GD library [tests/imagearc_variation2.phpt]
Testing imagechar() of GD library [tests/imagechar_basic.phpt]
Testing imagecharup() of GD library [tests/imagecharup_basic.phpt]
Testing imagecolorallocatealpha() [tests/imagecolorallocatealpha_basic.phpt]
Test imagecolorset() function : basic functionality [tests/imagecolorset_basic.phpt]
Testing imageconvolution() of GD library [tests/imageconvolution_basic.phpt]
Testing imagecreatetruecolor() of GD library [tests/imagecreatetruecolor_basic.phpt]
Testing imagecropauto() [tests/imagecrop_auto.phpt]
Testing imageellipse() of GD library [tests/imageellipse_basic.phpt]
Testing imagefilledarc() of GD library [tests/imagefilledarc_basic.phpt]
Testing wrong param passing imagefilledarc() of GD library [tests/imagefilledarc_error1.phpt]
Testing passing negative end angle to imagefilledarc() of GD library [tests/imagefilledarc_variation1.phpt]
Testing passing negative start angle to imagefilledarc() of GD library [tests/imagefilledarc_variation2.phpt]
Testing imagefilltoborder() of GD library [tests/imagefilltoborder_basic.phpt]
Testing imagegammacorrect() of GD library [tests/imagegammacorrect_basic.phpt]
Testing imagegammacorrect() of GD library with non TrueColor image [tests/imagegammacorrect_variation1.phpt]
Testing imagerectangle() of GD library [tests/imagerectangle_basic.phpt]
Test imagesetbrush() function : basic functionality [tests/imagesetbrush_basic.phpt]
Testing imagetruecolortopalette() of GD library [tests/imagesetthickness_basic.phpt]
Testing imagestring() of GD library [tests/imagestring_basic.phpt]
Testing imagestringup() of GD library [tests/imagestringup_basic.phpt]
Testing imagetruecolortopalette() of GD library [tests/imagetruecolortopalette_basic.phpt]
libgd #86 (Possible infinite loop in imagecreatefrompng) [tests/libgd00086_extern.phpt]
=====================================================================

But that's probably something which needs to be fixed in GD library itself.

@remicollet
Copy link
Member

Sorry, I have miss the commit added a few days ago (no notification received)
I will check this ASAP

@oerdnj
Copy link
Contributor Author

oerdnj commented May 3, 2013

The github is wrong, I have pushed it to github today.

@oerdnj
Copy link
Contributor Author

oerdnj commented May 3, 2013

@remicollet I have also killed whole libgd/gd_compat.*

And I hope we never divert so much (and fail so hard) ever again :(.

@remicollet
Copy link
Member

Hmm...

/dev/shm/BUILD/php5.5-201305041230/ext/gd/gd.c: In function 'gdPngGetVersionString':
/dev/shm/BUILD/php5.5-201305041230/ext/gd/gd.c:68:9: error: 'PNG_LIBPNG_VER_STRING' undeclared (first use in this function)
/dev/shm/BUILD/php5.5-201305041230/ext/gd/gd.c:68:9: note: each undeclared identifier is reported only once for each function it appears in
/dev/shm/BUILD/php5.5-201305041230/ext/gd/gd.c: In function 'gdJpegGetVersionInt':
/dev/shm/BUILD/php5.5-201305041230/ext/gd/gd.c:75:9: error: 'JPEG_LIB_VERSION' undeclared (first use in this function)
/dev/shm/BUILD/php5.5-201305041230/ext/gd/gd.c: In function 'gdJpegGetVersionString':
/dev/shm/BUILD/php5.5-201305041230/ext/gd/gd.c:80:9: error: 'JPEG_LIB_VERSION' undeclared (first use in this function)
/dev/shm/BUILD/php5.5-201305041230/ext/gd/gd.c: In function 'zm_info_gd':
/dev/shm/BUILD/php5.5-201305041230/ext/gd/gd.c:1367:3: warning: implicit declaration of function 'XpmLibraryVersion' [-Wimplicit-function-declaration]
/dev/shm/BUILD/php5.5-201305041230/ext/gd/gd.c: In function 'gdJpegGetVersionString':
/dev/shm/BUILD/php5.5-201305041230/ext/gd/gd.c:96:1: warning: control reaches end of non-void function [-Wreturn-type]
/dev/shm/BUILD/php5.5-201305041230/ext/gd/gd.c: In function 'gdJpegGetVersionInt':
/dev/shm/BUILD/php5.5-201305041230/ext/gd/gd.c:76:1: warning: control reaches end of non-void function [-Wreturn-type]
/dev/shm/BUILD/php5.5-201305041230/ext/gd/gd.c: In function 'gdPngGetVersionString':
/dev/shm/BUILD/php5.5-201305041230/ext/gd/gd.c:69:1: warning: control reaches end of non-void function [-Wreturn-type]

@remicollet
Copy link
Member

for PNG_LIBPNG_VER_STRING and JPEG_LIB_VERSION, just need to move the include "after" php ones (to have php_config.h)

@remicollet
Copy link
Member

Also need to add, for XpmLibraryVersion

#ifdef HAVE_GD_XPM
# include <X11/xpm.h>
#endif

@remicollet
Copy link
Member

Commited.

@remicollet
Copy link
Member

@oerdnj can you please close this PR ?

@oerdnj oerdnj closed this May 4, 2013
@laruence
Copy link
Member

laruence commented May 5, 2013

ext/gd/tests/imageloadfont_invalid.phpt starts fails

$ cat ext/gd/tests/imageloadfont_invalid.diff
001- Warning: imageloadfont(): gd warning: product of memory allocation multiplication would exceed INT_MAX, failing operation gracefully
002-  in %simageloadfont_invalid.php on line %d
003-

not sure whether due to this change. didn't look into it deepy

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.

5 participants