[drupal-devel] [bug] image.gd.inc isn't compatible with gd 1
walkah
drupal-devel at drupal.org
Wed Mar 23 19:41:01 UTC 2005
Issue status update for http://drupal.org/node/18700
Project: Drupal
Version: cvs
Component: base system
Category: bug reports
Priority: critical
Assigned to: stefan nagtegaal
Reported by: stefan nagtegaal
Updated by: walkah
Status: patch
Attachment: http://drupal.org/files/issues/image-inc_1.patch (5.39 KB)
OK - attached is said patch. It also includes a fix to the recent
system.module patch (where it was reporting that no toolkits were
installed if only the built-in one was).
I've also included a link on admin/settings to
http://drupal.org/project/image where I will post information about
getting other toolkits, etc. (I think that makes sense).
comments?
walkah
Previous comments:
------------------------------------------------------------------------
March 10, 2005 - 13:13 : stefan nagtegaal
The idea behind the image.gd.inc file is that it should be usable for
people who are using GD 1 and GD 2.
Unfortunatly, the way it is written now, only support for GD 2 can be
given because:
Current way of determining GD 2, is not right:
if (function_exists('imageCreateTrueColor')) {
// GD 2 Handling;
}
else {
// GD 1 Handling
}
this doesn't work.. imageCreateTrueColor() was already implemented in
GD 1, only it failed to work properly..
See these pages:
http://www.php.net/imagecratetruecolor#25234 &
http://www.php.net/imagecratetruecolor#25487
According to this comments, we should implement this like this:
// Silence errors using the @
$image = @imageCreateTrueColor(......);
if (!$image) {
// GD 1 Handling;
}
------------------------------------------------------------------------
March 11, 2005 - 13:03 : stefan nagtegaal
Attachment: http://drupal.org/files/issues/image-inc.patch (2.26 KB)
Attached patch improves:
- the way which deals with the detection of GD 1 or GD 2;
- error messages as reported by Steven here:
http://drupal.org/node/17645;
After applying this to HEAD/Drupal 4.6 RC, we can close the following
issues:
- http://drupal.org/node/17645;
- http://drupal.org/node/13027;
Please test/comment and apply attached patch..
------------------------------------------------------------------------
March 12, 2005 - 03:41 : Dries
Isn't the approach taking in http://drupal.org/node/13027 nicer? It
doesn't surpress warnings/errors. Maybe that approach doesn't work?
------------------------------------------------------------------------
March 12, 2005 - 03:50 : Bèr Kessels
-1 on supressing errors with @. Its ugly, and hardly used in Drupal. We
should use our own error handling at all times.
------------------------------------------------------------------------
March 12, 2005 - 04:42 : stefan nagtegaal
I will make a new patch, with the more elegant approach you mentioned
Dries...
------------------------------------------------------------------------
March 12, 2005 - 12:05 : stefan nagtegaal
As noticed before in this issue.. The function imageCreateTrueColor
_does_ exist, even in versions of GD before 1.8 only it was broken..
So, function_exist() doesn't work on that the function, because it
really _does_ exist, but the fact is that it's simply broken..
So unfortunatly, the more elegant solution doesn't seem to work..
------------------------------------------------------------------------
March 12, 2005 - 13:01 : pz
Stupid question, why not just use the gd_info function?
------------------------------------------------------------------------
March 13, 2005 - 03:54 : Dries
What versions of PHP have those old GD toolkits?
PHP 4.3.10 comes with GD 2.0.28, it seems, as does PHP 5.0.3.
PHP 4.3.3, Drupal's current minimum PHP requirement, comes with GD
2.0.15.
>From the looks, we don't need GD 1 handling at all ... (but we might
need a global version check).
Thoughts?
------------------------------------------------------------------------
March 13, 2005 - 05:32 : stefan nagtegaal
PHP 4.1.2 comes with GD 1.62..
The nasty/hackish code with the build-in GD toolkit, is exactly why I
proposed to split the image.inc into a separate image.gd1.inc and
image.gd2.inc.
I would propose a patch which does:
- remove all the GD < 2 function calls like imageCreate() and
imageCopyResized() and have a real GD 2 compatible image.inc;
- introduce a new image.gd1.inc file into Walkahs sandbox; If the
module is being moved to the contributions, the image.gd1.inc is also
available for people without proper GD 2 support;
- more userfriendly and helpfull error reporting for the image-api..
If we agree about this, I'll make patches for inclusion into 4.6..
Which does contain the before mentioned problems with the current
usage..
Please, I would like to have some feedback about this...
------------------------------------------------------------------------
March 13, 2005 - 07:55 : stefan nagtegaal
Attachment: http://drupal.org/files/issues/image-inc_0.patch (2.99 KB)
As proposed above, patch for image.inc..
Please comment or apply...
The image.gd1.inc file wil be posted after this post...
------------------------------------------------------------------------
March 13, 2005 - 07:57 : stefan nagtegaal
Attachment: http://drupal.org/files/issues/image.gd1.inc (2.07 KB)
And the image.gd1.inc file for Walkahs sandbox...
Tell me what you all think about this..
------------------------------------------------------------------------
March 13, 2005 - 10:11 : Dries
I'm OK with this! I'd like to await James' input though.
------------------------------------------------------------------------
March 15, 2005 - 05:16 : stefan nagtegaal
Walkah, are you out there???
Ber, do you agree with Dries that this is a better approach?
------------------------------------------------------------------------
March 15, 2005 - 06:15 : Bèr Kessels
I like this approach because it: uses a good default, and uses that
well. Provides flexibility for those not wanting that default, without
extra complexity. Uses modules to add extra functionality instead of
configuration options.
big +1 from me.
------------------------------------------------------------------------
March 16, 2005 - 09:33 : walkah
Hey guys, sorry I've been silentlly watching this thread. Here's my
take:
+1 - I like the approach, and I agree that (especially now that we're
requiring 4.3.x) making the "default" support gd2 only isn't a bad
idea.
I'm working on some additions to this patch, however, to basically make
the toolkit checking a bit more robust (working on top of Stefan's).
Great job Stefan, keep it up!
More information about the drupal-devel
mailing list