[RESOLVED] Grayscaling images causes mangled image + segfault!

ppowell

/**
 * Class for grayscaling image
 *
 * @author Phil Powell
 * @version 1.2.1
 * @package IMAGE_CATALOG::IMAGE
 */
class ImageGrayscaleGenerator extends ImageResizeComponents {

/*----------------------------------------------------------------------------------------------------------------------------------------------------------
	This class exists due to the rather poor performance of the imagecopymergegray() command
	Borrowing code snippet from http://us3.php.net/manual/en/function.imagecopymergegray.php first line comment
	Will perform actual grayscaling of image one pixel at a time.
-----------------------------------------------------------------------------------------------------------------------------------------------------------*/


// REMEMBER TO USE REFERENCE POINTER ONTO $image OBJECT TO ENSURE "static" CHANGE TO OBJECT AND NOT TO INSTANCE

/**
 * Constructor.  Set all properties dynamically
 *
 * @access public
 * @param resource $image (reference)
 * @param resource $newImage (reference)
 * @param int $image_width
 * @param int $image_height
 */
function ImageGrayscaleGenerator(&$image, &$newImage, $image_width, $image_height) { 	// CONSTRUCTOR
	global $section;
	foreach (array($section, 'newImage', "${section}_width", "${section}_height") as $val) $this->$val =& ${$val};
}

/**
 * Make the image grayscale
 *
 * @access protected
 */
function makeGray() {						// VOID METHOD
	global $section;
	for ($i = 0; $i <= 255; $i++) $colorNDX[$i] = @imagecolorallocate($this->newImage, $i, $i, $i);

	if (is_array($colorNDX) && @sizeof($colorNDX) > 0) {
	 for ($y = 0; $y < $this->{$section . '_height'}; $y++) {
	  for ($x = 0; $x < $this->{$section . '_width'}; $x++) {
	   $ndx = @imagecolorat($this->image, $x, $y);
	   $ndxColorArray = @imagecolorsforindex($this->image, $ndx);
	   $avg = floor(($ndxColorArray['red'] + $ndxColorArray['green'] + $ndxColorArray['blue']) / 3);
	   @imagesetpixel($this->newImage, $x, $y, $colorNDX[$avg]);
	  }
	 }
	}
}
}

if ($this->isSuccessful && $image && $willGrayscale) {			 // GRAYSCALE IMAGE
		 $igg =& new ImageGrayscaleGenerator($image, $newImage, $image_width, $image_height);
		 $igg->makeGray();
		 $igg = null;
		}

Whenever I run this class to grayscale an image, I wind up with the resulting image being horribly mangled to the point of its original format completely irreparable, furthermore, I wind up with an Apache segfault and having to reboot the server to correct the problem.

Um, why?

Phil

Weedpacket

I don't see why it's necessary to copy every single pixel one by one. For it to work with palette-based images, imagecopy needs to map the colours of the source image to the palette of the destination image.

http://www.phpbuilder.com/board/showthread.php?t=10268016

ppowell

Sorry, but I tried your exact same methodology to grayscale the image, but instead I got a timeout error followed by a chopped up color piece of an image instead.

I coped your code:

global $section;
		for ($i = 0; $i <= 255; $i++) $colorNDX[$i] = @imagecolorallocate($this->newImage, $i, $i, $i);

	/*if (is_array($colorNDX) && @sizeof($colorNDX) > 0) {
	 for ($y = 0; $y < $this->{$section . '_height'}; $y++) {
	  for ($x = 0; $x < $this->{$section . '_width'}; $x++) {
	   $ndx = @imagecolorat($this->image, $x, $y);
	   $ndxColorArray = @imagecolorsforindex($this->image, $ndx);
	   $avg = floor(($ndxColorArray['red'] + $ndxColorArray['green'] + $ndxColorArray['blue']) / 3);
	   @imagesetpixel($this->newImage, $x, $y, $colorNDX[$avg]);
	  }
	 }
	}
	*/
@imagecopy($this->newImage, $this->$section, 0, 0, 0, 0,
			   $this->{$section . '_width'}, $this->{$section . '_height'});

Taking out the @ warning suppressors makes no difference, no errors, warnings, no nothing, just mangled color images instead of mangled grayscale images (w/ segfault), so sorry it looks like it works worse than the loopy code I had 🙁

Weedpacket

The value of $section doesn't happen to be 'newImage', does it? I don't know what would happen if the source image and destination image happened to be the same, but it sounds unhealthy.

ppowell

The value of $section is the word "image" in some cases, not necessarily the actual image, sorry that wasn't more clear, but at least I didn't get this response:

for all the 'performance' reasons the brain-dead author gives for creating
this beast,

Wow, why do programmers feel the need to be so nasty and unhelpful like that is beyond me, but thanx Weed for not being like that!

Weedpacket

I didn't see that, but then I didn't see that whole function when I came up with what I came up with (it's bad enough remembering to have to search for "color" than have to remember to search for "gray" as well).

Oh, one thing that occurs to me; I've been assuming that the new image is palette-based (being greyscale it's only going to have at most 256 colours in it anyway). If it's a truecolour image then allocating palette colours isn't going to have much effect (then again, it won't cause mangling or a segfault either...).

ppowell

I'm not totally following you, but I think that means that I need to determine if the image is truecolor or not, so how will that affect grayscaling?

ppowell

Weedpacket wrote:
I didn't see that, but then I didn't see that whole function when I came up with what I came up with (it's bad enough remembering to have to search for "color" than have to remember to search for "gray" as well).

Oh, one thing that occurs to me; I've been assuming that the new image is palette-based (being greyscale it's only going to have at most 256 colours in it anyway). If it's a truecolour image then allocating palette colours isn't going to have much effect (then again, it won't cause mangling or a segfault either...).

Turns out the constructor was the culprit after all! I am assuming that PHP, at least version 5, behaves in such a way that if a variable is not declared outside of a constructor or method, it only has method scope, so instead of doing the smart thing


var $image;

function ImageGrayscaleGenerator(&$image) {
  $this->image =& $image;
}

I left out the "var $image" part, so $this->image ceased to exist upon constructor end. At least that's what I'm thinking, but nonetheless everything works perfectly now!

Phil