Page 1 of 1

Repeatedly using DrawSetClipPath shows corruption

Posted: 2014-05-24T07:07:45-07:00
by Danack
Hi,

I think I've found a pretty serious bug where DrawSetClipPath shows data corruption. The code example below is doing the following:

* Create a DrawingWand.
* Defining a clip path which is either just a rectangle in the top left quadrant of the image, or the same but with extra clip regions across the DrawingWand.
* Setting that clip path.
* Drawing a rectangle into the centre of the DrawingWand.
* Saving that drawing wand to a file.

That codes is called multiple times.

It starts out producing the images as expected:

ImageImage

But after several runs, each of which shouldn't affect each other, the images start looking like:

ImageImage

i.e. the data from the previous images is leaking through to the current image.

Code: Select all

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <wand/MagickWand.h>


void ThrowWandException(MagickWand *wand) {
    char *description; 
    ExceptionType severity; 
    description = MagickGetException(wand, &severity); 
    (void) fprintf(stderr,"%s %s %lu %s\n",GetMagickModule(),description); 
    description=(char *) MagickRelinquishMemory(description); 
    exit(-1); 
}


PixelWand *makePixelWand(char *string) {

	PixelWand *pixel_wand;
	pixel_wand = NewPixelWand();

	if (PixelSetColor (pixel_wand, string) == MagickFalse) {
		printf("Failed to set color");
		exit(-1);
	}

	return pixel_wand;
}

void drawClipTest(int extraClipping, int writeFile, char *filename) {
    
    MagickBooleanType status;
    
    DrawingWand *drawing_wand;
    PixelWand *fill_color_wand, *stroke_color_wand;
    char *clip_mask;
    char *clip_mask_duplicate;
    int x;
    
    char *backgroundColor = "rgb(225, 225, 225)";
    char *strokeColor = "rgb(0, 0, 0)";
    char *fillColor = "DodgerBlue2";
    PixelWand *destroyed_wand;

	//	$draw = new \ImagickDraw();
    drawing_wand = NewDrawingWand();

	//    $draw->setStrokeColor($strokeColor);
    stroke_color_wand = makePixelWand(strokeColor);
    DrawSetStrokeColor(drawing_wand, stroke_color_wand);
    //destroyed_wand = DestroyPixelWand (stroke_color_wand);
    //clearWand(stroke_color_wand);
    
    //    $draw->setFillColor($fillColor);
    fill_color_wand = makePixelWand(fillColor);
    DrawSetFillColor(drawing_wand, fill_color_wand);
    //destroyed_wand = DestroyPixelWand (fill_color_wand);
    //clearWand(fill_color_wand);
    
    
    //    $draw->setStrokeOpacity(1);
   	DrawSetStrokeOpacity(drawing_wand, 1);
    //    $draw->setStrokeWidth(2);
    DrawSetStrokeWidth(drawing_wand, 2);


	//    $draw->pushClipPath($clipPathName);
	DrawPushClipPath(drawing_wand, "testClipPath");

	//    $draw->rectangle(0, 0, 250, 250);
	DrawRectangle(drawing_wand, 0, 0, 250, 250);


	if (extraClipping) {
		for (x=0 ; x<1000 ; x++) {
			int posX = (x * 23 * 53 ) % 500;
			int posY = (x * 7) % 500;
			DrawRectangle(drawing_wand, posX, posY, posX + 10, posY + 10);
		}
	}

	//    $draw->popClipPath();
	DrawPopClipPath(drawing_wand);

	//DrawSetClipRule(internd->drawing_wand, fill_rule);
	status = DrawSetClipPath(drawing_wand, "testClipPath");


	if (status == MagickFalse) {
		printf("Failed to DrawSetClipPath ");
		exit(-1); 
	}

	//    $draw->rectangle(200, 200, 300, 300);
	//This is the only thing that should be drawn into the final image
    DrawRectangle(drawing_wand, 100, 100, 400, 400);

    MagickWand *magick_wand;
    
	magick_wand = NewMagickWand();

	PixelWand *background_color_wand = makePixelWand(backgroundColor);

	
		status = MagickNewImage(magick_wand, 500, 500, background_color_wand);

	if (writeFile) {
		MagickSetImageFormat(magick_wand, "png");
	
		MagickDrawImage(magick_wand, drawing_wand);
	
		status = MagickWriteImages(magick_wand, filename, MagickTrue);
	
		if (status == MagickFalse) {
			ThrowWandException(magick_wand);
		}
	}

	magick_wand = DestroyMagickWand(magick_wand);
 
    return;
}


int main(int argc,char **argv) {
    
    int x;
    char filename[1024];
    
    MagickWandGenesis();
    
    for (x=0 ; x<10 ; x++) {
    	sprintf(filename, "clipFirst_%d.png", x);
		drawClipTest(1, 1, filename);

		sprintf(filename, "clipSecond_%d.png", x);
		drawClipTest(0, 1, filename);
    }
    
    
    MagickWandTerminus();
    return (0);
}


Re: Repeatedly using DrawSetClipPath shows corruption

Posted: 2014-05-24T08:20:49-07:00
by dlemstra
What is your ImageMagick version?

And is this the code that was used for your tests? I am getting the same image each time but it looks different from yours.

Re: Repeatedly using DrawSetClipPath shows corruption

Posted: 2014-05-24T08:27:27-07:00
by Danack
I'm currently on the beta of 6.8.9-2 from a few days ago, but this also happens on 6.8.8-9
/usr/local/bin/convert --version
Version: ImageMagick 6.8.9-2 Q16 x86_64 2014-05-21 http://www.imagemagick.org
Copyright: Copyright (C) 1999-2014 ImageMagick Studio LLC
Features: DPC OpenMP
Delegates: bzlib freetype jng jpeg png xml zlib
And is this the code that was used for your tests? I am getting the same image each time but it looks different from yours.
Yes, the images I posted were produced from the code posted. Can you post your image?

Re: Repeatedly using DrawSetClipPath shows corruption

Posted: 2014-05-24T08:33:42-07:00
by Danack
For completeness, the issue was found originally through the PHP Imagick extension under PHP-FPM. That process manager processes multiple requests before stopping and restarting the PHP worker instance. The test for setClipPath works a couple of times, and then goes wonky.

Original code:

Code: Select all

function setClipPath($strokeColor, $fillColor, $backgroundColor) {

    $draw = new \ImagickDraw();
    $draw->setStrokeColor($strokeColor);
    $draw->setFillColor($fillColor);
    $draw->setStrokeOpacity(1);
    $draw->setStrokeWidth(2);

    $clipPathName = 'testClipPath';

    $draw->pushClipPath($clipPathName);
    $draw->rectangle(0, 0, 250, 250);
    $draw->popClipPath();
    $draw->setClipPath($clipPathName);
    $draw->rectangle(200, 200, 300, 300);

    $imagick = new \Imagick();
    $imagick->newImage(500, 500, $backgroundColor);
    $imagick->setImageFormat("png");

    $imagick->drawImage($draw);

    header("Content-Type: image/png");
    echo $imagick->getImageBlob();
}
Which may be easier for people to move to play with.

Re: Repeatedly using DrawSetClipPath shows corruption

Posted: 2014-05-24T08:44:36-07:00
by dlemstra
I am getting the following images: https://www.dropbox.com/sh/cad19t1ipjmh ... dcuGuTyAMa with your C code. Did you use the php code to create the images? Your code does not compile under Windows because you don't declare 'PixelWand *background_color_wand' at the beginning of the method.

I am using the latest beta from svn:

C:\Users\Dirk>convert -version
Version: ImageMagick 6.8.9-2 Q16 x86 2014-05-20 http://www.imagemagick.org
Copyright: Copyright (C) 1999-2014 ImageMagick Studio LLC
Features: DPC Modules OpenMP Debug
Delegates: bzlib cairo freetype jbig jng jp2 jpeg lcms lqr pangocairo png ps rsvg tiff webp xml zlib

Re: Repeatedly using DrawSetClipPath shows corruption

Posted: 2014-05-24T09:14:03-07:00
by Danack
dlemstra wrote:Did you use the php code to create the images?
No. I used the code I said I was using. For the record it was tested on :
CentOS release 6.4
gcc version 4.4.7 20120313 (Red Hat 4.4.7-4) (GCC)

I am getting the following images:
Just to note, those images also appear incorrect. Although they don't have the weird corruption that I'm seeing, the background color is created on the image, and the DrawingWand should be drawn on top - but you're getting the clipping of the draw image affect the background. Which I don't think should happen.

Re: Repeatedly using DrawSetClipPath shows corruption

Posted: 2014-05-26T01:18:27-07:00
by dlemstra
I did not look at your code if the result was correct but you are right. The black color outside the clipping path should be your background color. We will have to investigate this and fix it.

I am however still unable to reproduce your problem. Can you try if the problem still occurs if you don't do the 'extraClipping'? Some of your coordinates are outside the 500x500 image.

Re: Repeatedly using DrawSetClipPath shows corruption

Posted: 2014-05-26T06:46:42-07:00
by Danack
Hi Dlemstra,

The problem does still occur when the 'extraClipping' is totally disabled, e.g. http://www.basereality.com/image/2409/clipSecond_2.png

Perhaps I didn't make one thing clear though - in the code half the images are produced with the 'extraClipping' enabled and half are not, depending on whether the extraClipping flag is on when calling drawClipTest, and that this image:

Image

Was produced with the 'extraClipping' disabled, i.e. the previous image generation is affecting the current image generation.

cheers
Dan

Re: Repeatedly using DrawSetClipPath shows corruption

Posted: 2014-05-26T13:28:03-07:00
by dlemstra
We have found the issue and are working on a fix. Can you reproduce the problem when you change the output format to tiff instead of png?

Re: Repeatedly using DrawSetClipPath shows corruption

Posted: 2014-05-26T17:09:19-07:00
by Danack
dlemstra wrote:Can you reproduce the problem when you change the output format to tiff instead of png?
Nope, it doesn't seem to occur.

Re: Repeatedly using DrawSetClipPath shows corruption

Posted: 2014-05-27T05:57:47-07:00
by Danack
btw I'm not sure how the code is handled differently for PNG format and TIFF format, but it does look like running the program through valgrind detects the use of uninitialized memory when the format is set to PNG. e.g "valgrind ./setClipPath"

Code: Select all

==17541== 
==17541== Use of uninitialised value of size 8
==17541==    at 0x696D9F9: crc32 (in /lib64/libz.so.1.2.3)
==17541==    by 0x61E6B7C: ??? (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61EF736: png_write_chunk (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61F05A1: ??? (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61F09A1: ??? (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61F0AB3: ??? (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61F0E4F: ??? (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61F4EA8: png_write_row (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x53A958A: WriteOnePNGImage (png.c:11167)
==17541==    by 0x53ACED1: WritePNGImage (png.c:12311)
==17541==    by 0x51C837E: WriteImage (constitute.c:1181)
==17541==    by 0x51CA8CF: WriteImages (constitute.c:1330)
==17541== 
==17541== Use of uninitialised value of size 8
==17541==    at 0x696DA0B: crc32 (in /lib64/libz.so.1.2.3)
==17541==    by 0x61E6B7C: ??? (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61EF736: png_write_chunk (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61F05A1: ??? (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61F09A1: ??? (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61F0AB3: ??? (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61F0E4F: ??? (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61F4EA8: png_write_row (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x53A958A: WriteOnePNGImage (png.c:11167)
==17541==    by 0x53ACED1: WritePNGImage (png.c:12311)
==17541==    by 0x51C837E: WriteImage (constitute.c:1181)
==17541==    by 0x51CA8CF: WriteImages (constitute.c:1330)
==17541== 
==17541== Use of uninitialised value of size 8
==17541==    at 0x696DA0F: crc32 (in /lib64/libz.so.1.2.3)
==17541==    by 0x61E6B7C: ??? (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61EF736: png_write_chunk (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61F05A1: ??? (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61F09A1: ??? (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61F0AB3: ??? (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61F0E4F: ??? (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61F4EA8: png_write_row (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x53A958A: WriteOnePNGImage (png.c:11167)
==17541==    by 0x53ACED1: WritePNGImage (png.c:12311)
==17541==    by 0x51C837E: WriteImage (constitute.c:1181)
==17541==    by 0x51CA8CF: WriteImages (constitute.c:1330)
==17541== 
==17541== Use of uninitialised value of size 8
==17541==    at 0x696DA16: crc32 (in /lib64/libz.so.1.2.3)
==17541==    by 0x61E6B7C: ??? (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61EF736: png_write_chunk (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61F05A1: ??? (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61F09A1: ??? (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61F0AB3: ??? (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61F0E4F: ??? (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61F4EA8: png_write_row (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x53A958A: WriteOnePNGImage (png.c:11167)
==17541==    by 0x53ACED1: WritePNGImage (png.c:12311)
==17541==    by 0x51C837E: WriteImage (constitute.c:1181)
==17541==    by 0x51CA8CF: WriteImages (constitute.c:1330)
==17541== 
==17541== Syscall param write(buf) points to uninitialised byte(s)
==17541==    at 0x57D8650: __write_nocancel (in /lib64/libc-2.12.so)
==17541==    by 0x576ED52: _IO_file_write@@GLIBC_2.2.5 (in /lib64/libc-2.12.so)
==17541==    by 0x5770304: _IO_do_write@@GLIBC_2.2.5 (in /lib64/libc-2.12.so)
==17541==    by 0x576F507: _IO_file_sync@@GLIBC_2.2.5 (in /lib64/libc-2.12.so)
==17541==    by 0x5763C19: fflush (in /lib64/libc-2.12.so)
==17541==    by 0x519146F: CloseBlob (blob.c:511)
==17541==    by 0x53ACEDB: WritePNGImage (png.c:12313)
==17541==    by 0x51C837E: WriteImage (constitute.c:1181)
==17541==    by 0x51CA8CF: WriteImages (constitute.c:1330)
==17541==    by 0x4E9F48F: MagickWriteImages (magick-image.c:13117)
==17541==    by 0x40111D: drawClipTest (in /home/github/imagemagicktest/setClipPath)
==17541==    by 0x4011AB: main (in /home/github/imagemagicktest/setClipPath)
==17541==  Address 0x4021937 is not stack'd, malloc'd or (recently) free'd
==17541== 
==17541== Use of uninitialised value of size 8
==17541==    at 0x696DA57: crc32 (in /lib64/libz.so.1.2.3)
==17541==    by 0x61E6B7C: ??? (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61EF736: png_write_chunk (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61F05A1: ??? (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61F09A1: ??? (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61F0AB3: ??? (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61F0E4F: ??? (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61F4EA8: png_write_row (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x53A958A: WriteOnePNGImage (png.c:11167)
==17541==    by 0x53ACED1: WritePNGImage (png.c:12311)
==17541==    by 0x51C837E: WriteImage (constitute.c:1181)
==17541==    by 0x51CA8CF: WriteImages (constitute.c:1330)
==17541== 
==17541== Use of uninitialised value of size 8
==17541==    at 0x696D97F: crc32 (in /lib64/libz.so.1.2.3)
==17541==    by 0x61E6B7C: ??? (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61EF736: png_write_chunk (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61F05A1: ??? (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61F09A1: ??? (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61F0AB3: ??? (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61F0E4F: ??? (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61F4EA8: png_write_row (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x53A958A: WriteOnePNGImage (png.c:11167)
==17541==    by 0x53ACED1: WritePNGImage (png.c:12311)
==17541==    by 0x51C837E: WriteImage (constitute.c:1181)
==17541==    by 0x51CA8CF: WriteImages (constitute.c:1330)
==17541== 
==17541== Use of uninitialised value of size 8
==17541==    at 0x696D990: crc32 (in /lib64/libz.so.1.2.3)
==17541==    by 0x61E6B7C: ??? (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61EF736: png_write_chunk (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61F05A1: ??? (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61F09A1: ??? (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61F0AB3: ??? (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61F0E4F: ??? (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61F4EA8: png_write_row (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x53A958A: WriteOnePNGImage (png.c:11167)
==17541==    by 0x53ACED1: WritePNGImage (png.c:12311)
==17541==    by 0x51C837E: WriteImage (constitute.c:1181)
==17541==    by 0x51CA8CF: WriteImages (constitute.c:1330)
==17541== 
==17541== Use of uninitialised value of size 8
==17541==    at 0x696D993: crc32 (in /lib64/libz.so.1.2.3)
==17541==    by 0x61E6B7C: ??? (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61EF736: png_write_chunk (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61F05A1: ??? (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61F09A1: ??? (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61F0AB3: ??? (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61F0E4F: ??? (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61F4EA8: png_write_row (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x53A958A: WriteOnePNGImage (png.c:11167)
==17541==    by 0x53ACED1: WritePNGImage (png.c:12311)
==17541==    by 0x51C837E: WriteImage (constitute.c:1181)
==17541==    by 0x51CA8CF: WriteImages (constitute.c:1330)
==17541== 
==17541== Use of uninitialised value of size 8
==17541==    at 0x696D99A: crc32 (in /lib64/libz.so.1.2.3)
==17541==    by 0x61E6B7C: ??? (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61EF736: png_write_chunk (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61F05A1: ??? (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61F09A1: ??? (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61F0AB3: ??? (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61F0E4F: ??? (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x61F4EA8: png_write_row (in /usr/lib64/libpng12.so.0.49.0)
==17541==    by 0x53A958A: WriteOnePNGImage (png.c:11167)
==17541==    by 0x53ACED1: WritePNGImage (png.c:12311)
==17541==    by 0x51C837E: WriteImage (constitute.c:1181)
==17541==    by 0x51CA8CF: WriteImages (constitute.c:1330)
==17541== 
This does not appear to happen when the format is set to TIFF.

This is very similar to the issue where gradients are being corrupted - which unfortunately I don't have a reproducible test case for. So I will be very interested to see if the fix for this issue affects that issue, or if the same fix could be applied in other places in the ImageMagick code base.

Re: Repeatedly using DrawSetClipPath shows corruption

Posted: 2014-05-27T06:30:18-07:00
by dlemstra
The problem here is related to the clipping mask you added. If you set it to null before you write the PNG it will be created properly. I don't think your code in the other example set a clip mask.

Re: Repeatedly using DrawSetClipPath shows corruption

Posted: 2014-06-13T05:24:20-07:00
by Danack
Hi, was just wondering if there was an update on this issue?

Re: Repeatedly using DrawSetClipPath shows corruption

Posted: 2014-06-13T05:59:31-07:00
by dlemstra
The problem with the clipping mask has been fixed in the latest release.