From 3fa7a16d212c7d1b6252294cadf3e5d35e47ec69 Mon Sep 17 00:00:00 2001 From: simonbachmann Date: Sun, 30 Apr 2017 09:04:00 +0000 Subject: Fix [e4336bef5d] (Unexpected result when copying a photo image to itself): if source and destination image are the same, make a local copy of image data before the actual copy/zoom/subsample. --- generic/tkImgPhoto.c | 166 +++++++++++++++++++++++++++++++++++++-------------- tests/imgBmap.test | 3 +- tests/imgPhoto.test | 73 ++++++++++++++++++++++ 3 files changed, 195 insertions(+), 47 deletions(-) diff --git a/generic/tkImgPhoto.c b/generic/tkImgPhoto.c index 1bd0142..8b7350d 100644 --- a/generic/tkImgPhoto.c +++ b/generic/tkImgPhoto.c @@ -2698,7 +2698,7 @@ Tk_PhotoPutBlock( * messages, or NULL. */ Tk_PhotoHandle handle, /* Opaque handle for the photo image to be * updated. */ - register Tk_PhotoImageBlock *blockPtr, + Tk_PhotoImageBlock *blockPtr, /* Pointer to a structure describing the pixel * data to be copied into the image. */ int x, int y, /* Coordinates of the top-left pixel to be @@ -2709,6 +2709,8 @@ Tk_PhotoPutBlock( * transparent pixels. */ { register PhotoMaster *masterPtr = (PhotoMaster *) handle; + Tk_PhotoImageBlock sourceBlock; + unsigned char *memToFree; int xEnd, yEnd, greenOffset, blueOffset, alphaOffset; int wLeft, hLeft, wCopy, hCopy, pitch; unsigned char *srcPtr, *srcLinePtr, *destPtr, *destLinePtr; @@ -2736,11 +2738,43 @@ Tk_PhotoPutBlock( return TCL_OK; } + /* + * Fix for bug e4336bef5d: + * + * Make a local copy of *blockPtr, as we might have to change some + * of its fields and don't want to interfere with the caller's data. + * + * If source and destination are the same image, create a copy of the + * source data in our local sourceBlock. + * + * To find out, just comparing the pointers is not enough - they might have + * different values and still point to the same block of memory. (e.g. + * if the -from option was passed to [imageName copy]) + */ + sourceBlock = *blockPtr; + memToFree = NULL; + if (sourceBlock.pixelPtr >= masterPtr->pix32 + && sourceBlock.pixelPtr <= masterPtr->pix32 + masterPtr->width + * masterPtr->height * 4) { + sourceBlock.pixelPtr = attemptckalloc(sourceBlock.height + * sourceBlock.pitch); + if (sourceBlock.pixelPtr == NULL) { + if (interp != NULL) { + Tcl_SetObjResult(interp, Tcl_NewStringObj( + TK_PHOTO_ALLOC_FAILURE_MESSAGE, -1)); + Tcl_SetErrorCode(interp, "TK", "MALLOC", NULL); + } + return TCL_ERROR; + } + memToFree = sourceBlock.pixelPtr; + memcpy(sourceBlock.pixelPtr, blockPtr->pixelPtr, sourceBlock.height + * sourceBlock.pitch); + } + + xEnd = x + width; yEnd = y + height; if ((xEnd > masterPtr->width) || (yEnd > masterPtr->height)) { - int sameSrc = (blockPtr->pixelPtr == masterPtr->pix32); - if (ImgPhotoSetSize(masterPtr, MAX(xEnd, masterPtr->width), MAX(yEnd, masterPtr->height)) == TCL_ERROR) { if (interp != NULL) { @@ -2748,11 +2782,7 @@ Tk_PhotoPutBlock( TK_PHOTO_ALLOC_FAILURE_MESSAGE, -1)); Tcl_SetErrorCode(interp, "TK", "MALLOC", NULL); } - return TCL_ERROR; - } - if (sameSrc) { - blockPtr->pixelPtr = masterPtr->pix32; - blockPtr->pitch = masterPtr->width * 4; + goto errorExit; } } @@ -2771,14 +2801,14 @@ Tk_PhotoPutBlock( * components, mark it as a color image. */ - greenOffset = blockPtr->offset[1] - blockPtr->offset[0]; - blueOffset = blockPtr->offset[2] - blockPtr->offset[0]; - alphaOffset = blockPtr->offset[3]; - if ((alphaOffset >= blockPtr->pixelSize) || (alphaOffset < 0)) { + greenOffset = sourceBlock.offset[1] - sourceBlock.offset[0]; + blueOffset = sourceBlock.offset[2] - sourceBlock.offset[0]; + alphaOffset = sourceBlock.offset[3]; + if ((alphaOffset >= sourceBlock.pixelSize) || (alphaOffset < 0)) { alphaOffset = 0; sourceIsSimplePhoto = 1; } else { - alphaOffset -= blockPtr->offset[0]; + alphaOffset -= sourceBlock.offset[0]; } if ((greenOffset != 0) || (blueOffset != 0)) { masterPtr->flags |= COLOR_IMAGE; @@ -2798,13 +2828,13 @@ Tk_PhotoPutBlock( * pixelSize == 3 and alphaOffset == 0. Maybe other cases too. */ - if ((blockPtr->pixelSize == 4) + if ((sourceBlock.pixelSize == 4) && (greenOffset == 1) && (blueOffset == 2) && (alphaOffset == 3) - && (width <= blockPtr->width) && (height <= blockPtr->height) + && (width <= sourceBlock.width) && (height <= sourceBlock.height) && ((height == 1) || ((x == 0) && (width == masterPtr->width) - && (blockPtr->pitch == pitch))) + && (sourceBlock.pitch == pitch))) && (compRule == TK_PHOTO_COMPOSITE_SET)) { - memmove(destLinePtr, blockPtr->pixelPtr + blockPtr->offset[0], + memmove(destLinePtr, sourceBlock.pixelPtr + sourceBlock.offset[0], ((size_t)height * width * 4)); /* @@ -2820,11 +2850,11 @@ Tk_PhotoPutBlock( */ for (hLeft = height; hLeft > 0;) { - int pixelSize = blockPtr->pixelSize; + int pixelSize = sourceBlock.pixelSize; int compRuleSet = (compRule == TK_PHOTO_COMPOSITE_SET); - srcLinePtr = blockPtr->pixelPtr + blockPtr->offset[0]; - hCopy = MIN(hLeft, blockPtr->height); + srcLinePtr = sourceBlock.pixelPtr + sourceBlock.offset[0]; + hCopy = MIN(hLeft, sourceBlock.height); hLeft -= hCopy; for (; hCopy > 0; --hCopy) { /* @@ -2835,10 +2865,10 @@ Tk_PhotoPutBlock( if ((pixelSize == 4) && (greenOffset == 1) && (blueOffset == 2) && (alphaOffset == 3) - && (width <= blockPtr->width) + && (width <= sourceBlock.width) && compRuleSet) { memcpy(destLinePtr, srcLinePtr, ((size_t)width * 4)); - srcLinePtr += blockPtr->pitch; + srcLinePtr += sourceBlock.pitch; destLinePtr += pitch; continue; } @@ -2849,7 +2879,7 @@ Tk_PhotoPutBlock( destPtr = destLinePtr; for (wLeft = width; wLeft > 0;) { - wCopy = MIN(wLeft, blockPtr->width); + wCopy = MIN(wLeft, sourceBlock.width); wLeft -= wCopy; srcPtr = srcLinePtr; @@ -2939,7 +2969,7 @@ Tk_PhotoPutBlock( destPtr += 4; } } - srcLinePtr += blockPtr->pitch; + srcLinePtr += sourceBlock.pitch; destLinePtr += pitch; } } @@ -3055,7 +3085,15 @@ Tk_PhotoPutBlock( Tk_ImageChanged(masterPtr->tkMaster, x, y, width, height, masterPtr->width, masterPtr->height); + + ckfree(memToFree); + return TCL_OK; + + errorExit: + ckfree(memToFree); + + return TCL_ERROR; } /* @@ -3082,7 +3120,7 @@ Tk_PhotoPutZoomedBlock( * messages, or NULL. */ Tk_PhotoHandle handle, /* Opaque handle for the photo image to be * updated. */ - register Tk_PhotoImageBlock *blockPtr, + Tk_PhotoImageBlock *blockPtr, /* Pointer to a structure describing the pixel * data to be copied into the image. */ int x, int y, /* Coordinates of the top-left pixel to be @@ -3097,6 +3135,8 @@ Tk_PhotoPutZoomedBlock( * transparent pixels. */ { register PhotoMaster *masterPtr = (PhotoMaster *) handle; + register Tk_PhotoImageBlock sourceBlock; + unsigned char *memToFree; int xEnd, yEnd, greenOffset, blueOffset, alphaOffset; int wLeft, hLeft, wCopy, hCopy, blockWid, blockHt; unsigned char *srcPtr, *srcLinePtr, *srcOrigPtr, *destPtr, *destLinePtr; @@ -3132,12 +3172,42 @@ Tk_PhotoPutZoomedBlock( if (width <= 0 || height <= 0) { return TCL_OK; } + + /* + * Fix for Bug e4336bef5d: + * Make a local copy of *blockPtr, as we might have to change some + * of its fields and don't want to interfere with the caller's data. + * + * If source and destination are the same image, create a copy of the + * source data in our local sourceBlock. + * + * To find out, just comparing the pointers is not enough - they might have + * different values and still point to the same block of memory. (e.g. + * if the -from option was passed to [imageName copy]) + */ + sourceBlock = *blockPtr; + memToFree = NULL; + if (sourceBlock.pixelPtr >= masterPtr->pix32 + && sourceBlock.pixelPtr <= masterPtr->pix32 + masterPtr->width + * masterPtr->height * 4) { + sourceBlock.pixelPtr = attemptckalloc(sourceBlock.height + * sourceBlock.pitch); + if (sourceBlock.pixelPtr == NULL) { + if (interp != NULL) { + Tcl_SetObjResult(interp, Tcl_NewStringObj( + TK_PHOTO_ALLOC_FAILURE_MESSAGE, -1)); + Tcl_SetErrorCode(interp, "TK", "MALLOC", NULL); + } + return TCL_ERROR; + } + memToFree = sourceBlock.pixelPtr; + memcpy(sourceBlock.pixelPtr, blockPtr->pixelPtr, sourceBlock.height + * sourceBlock.pitch); + } xEnd = x + width; yEnd = y + height; if ((xEnd > masterPtr->width) || (yEnd > masterPtr->height)) { - int sameSrc = (blockPtr->pixelPtr == masterPtr->pix32); - if (ImgPhotoSetSize(masterPtr, MAX(xEnd, masterPtr->width), MAX(yEnd, masterPtr->height)) == TCL_ERROR) { if (interp != NULL) { @@ -3145,11 +3215,7 @@ Tk_PhotoPutZoomedBlock( TK_PHOTO_ALLOC_FAILURE_MESSAGE, -1)); Tcl_SetErrorCode(interp, "TK", "MALLOC", NULL); } - return TCL_ERROR; - } - if (sameSrc) { - blockPtr->pixelPtr = masterPtr->pix32; - blockPtr->pitch = masterPtr->width * 4; + goto errorExit; } } @@ -3168,14 +3234,14 @@ Tk_PhotoPutZoomedBlock( * components, mark it as a color image. */ - greenOffset = blockPtr->offset[1] - blockPtr->offset[0]; - blueOffset = blockPtr->offset[2] - blockPtr->offset[0]; - alphaOffset = blockPtr->offset[3]; - if ((alphaOffset >= blockPtr->pixelSize) || (alphaOffset < 0)) { + greenOffset = sourceBlock.offset[1] - sourceBlock.offset[0]; + blueOffset = sourceBlock.offset[2] - sourceBlock.offset[0]; + alphaOffset = sourceBlock.offset[3]; + if ((alphaOffset >= sourceBlock.pixelSize) || (alphaOffset < 0)) { alphaOffset = 0; sourceIsSimplePhoto = 1; } else { - alphaOffset -= blockPtr->offset[0]; + alphaOffset -= sourceBlock.offset[0]; } if ((greenOffset != 0) || (blueOffset != 0)) { masterPtr->flags |= COLOR_IMAGE; @@ -3186,21 +3252,21 @@ Tk_PhotoPutZoomedBlock( * subsampling and zooming. */ - blockXSkip = subsampleX * blockPtr->pixelSize; - blockYSkip = subsampleY * blockPtr->pitch; + blockXSkip = subsampleX * sourceBlock.pixelSize; + blockYSkip = subsampleY * sourceBlock.pitch; if (subsampleX > 0) { - blockWid = ((blockPtr->width + subsampleX - 1) / subsampleX) * zoomX; + blockWid = ((sourceBlock.width + subsampleX - 1) / subsampleX) * zoomX; } else if (subsampleX == 0) { blockWid = width; } else { - blockWid = ((blockPtr->width - subsampleX - 1) / -subsampleX) * zoomX; + blockWid = ((sourceBlock.width - subsampleX - 1) / -subsampleX) * zoomX; } if (subsampleY > 0) { - blockHt = ((blockPtr->height + subsampleY - 1) / subsampleY) * zoomY; + blockHt = ((sourceBlock.height + subsampleY - 1) / subsampleY) * zoomY; } else if (subsampleY == 0) { blockHt = height; } else { - blockHt = ((blockPtr->height - subsampleY - 1) / -subsampleY) * zoomY; + blockHt = ((sourceBlock.height - subsampleY - 1) / -subsampleY) * zoomY; } /* @@ -3208,12 +3274,12 @@ Tk_PhotoPutZoomedBlock( */ destLinePtr = masterPtr->pix32 + (y * masterPtr->width + x) * 4; - srcOrigPtr = blockPtr->pixelPtr + blockPtr->offset[0]; + srcOrigPtr = sourceBlock.pixelPtr + sourceBlock.offset[0]; if (subsampleX < 0) { - srcOrigPtr += (blockPtr->width - 1) * blockPtr->pixelSize; + srcOrigPtr += (sourceBlock.width - 1) * sourceBlock.pixelSize; } if (subsampleY < 0) { - srcOrigPtr += (blockPtr->height - 1) * blockPtr->pitch; + srcOrigPtr += (sourceBlock.height - 1) * sourceBlock.pitch; } pitch = masterPtr->width * 4; @@ -3363,7 +3429,15 @@ Tk_PhotoPutZoomedBlock( Tk_ImageChanged(masterPtr->tkMaster, x, y, width, height, masterPtr->width, masterPtr->height); + + ckfree(memToFree); + return TCL_OK; + + errorExit: + ckfree(memToFree); + + return TCL_ERROR; } /* diff --git a/tests/imgBmap.test b/tests/imgBmap.test index 5ffd7c4..e7f2c7e 100644 --- a/tests/imgBmap.test +++ b/tests/imgBmap.test @@ -380,7 +380,8 @@ test imageBmap-7.9 {ImgBmapCmd procedure} -body { test imageBmap-7.10 {ImgBmapCmd procedure} -body { i1 gorp } -returnCodes error -result {bad option "gorp": must be cget or configure} - +# Clean it up after use!! +imageCleanup test imageBmap-8.1 {ImgBmapGet/Free procedures, shared instances} -setup { destroy .c diff --git a/tests/imgPhoto.test b/tests/imgPhoto.test index e93dab4..ff53576 100644 --- a/tests/imgPhoto.test +++ b/tests/imgPhoto.test @@ -953,6 +953,44 @@ test imgPhoto-10.1 {Tk_ImgPhotoPutBlock procedure} -setup { photo1 put "{#00ff00 #00ff00}" -to 2 0 list [photo1 get 2 0] [photo1 get 3 0] [photo1 get 4 0] } -result {{0 255 0} {0 255 0} {255 0 0}} +test imgPhoto-10.2 {Tk_ImgPhotoPutBlock, same source and dest img} -constraints { + hasTeapotPhoto +} -setup { + imageCleanup +} -body { + # Test for bug e4336bef5d + image create photo photo1 -file $teapotPhotoFile + image create photo photo2 -file $teapotPhotoFile + photo2 copy photo1 -to 1 2 + photo1 copy photo1 -to 1 2 + string equal [photo1 data] [photo2 data] +} -cleanup { + imageCleanup +} -result {1} +test imgPhoto-10.3 {Tk_ImgPhotoPutBlock, same source and dest img} -constraints { + hasTeapotPhoto +} -setup { + imageCleanup +} -body { + # Test for bug e4336bef5d + image create photo photo1 -file $teapotPhotoFile + image create photo photo2 -file $teapotPhotoFile + photo2 copy photo1 -from 2 1 -to 4 5 300 300 + photo1 copy photo1 -from 2 1 -to 4 5 300 300 + string equal [photo1 data] [photo2 data] +} -cleanup { + imageCleanup +} -result {1} +test imgPhoto-10.4 {Tk_ImgPhotoPutBlock, empty image} -setup { + imageCleanup +} -body { + image create photo photo1 + photo1 copy photo1 -to 0 5 10 20 + list [image width photo1] [image height photo1] +} -cleanup { + imageCleanup +} -result {0 0} + test imgPhoto-11.1 {Tk_FindPhoto} -setup { imageCleanup @@ -972,6 +1010,41 @@ test imgPhoto-12.1 {Tk_PhotoPutZoomedBlock} -constraints hasTeapotPhoto -body { } -cleanup { image delete p3 } -result {{19 92 192} {169 117 90} 512 512 {19 92 192}} +test imgPhoto-12.2 {Tk_ImgPhotoPutZoomedBlock, same source and dest img} -constraints { + hasTeapotPhoto +} -setup { + imageCleanup +} -body { + # Test for bug e4336bef5d + image create photo photo1 -file $teapotPhotoFile + image create photo photo2 -file $teapotPhotoFile + photo2 copy photo1 -to 0 1 200 200 -zoom 2 3 + photo1 copy photo1 -to 0 1 200 200 -zoom 2 3 + string equal [photo1 data] [photo2 data] +} -cleanup { + imageCleanup +} -result {1} +test imgPhoto-12.3 {Tk_ImgPhotoPutZoomedBlock, same source and dest img} -setup { + imageCleanup +} -body { + # Test for bug e4336bef5d + image create photo photo1 -file $teapotPhotoFile + image create photo photo2 -file $teapotPhotoFile + photo2 copy photo1 -from 1 0 -to 4 5 300 300 -zoom 1 2 + photo1 copy photo1 -from 1 0 -to 4 5 300 300 -zoom 1 2 + string equal [photo1 data] [photo2 data] +} -cleanup { + imageCleanup +} -result {1} +test imgPhoto-12.4 {Tk_ImgPhotoPutZoomedBlock, empty image} -setup { + imageCleanup +} -body { + image create photo photo1 + photo1 copy photo1 -to 0 5 10 20 + list [image width photo1] [image height photo1] +} -cleanup { + imageCleanup +} -result {0 0} test imgPhoto-13.1 {check separation of images in different interpreters} -setup { imageCleanup -- cgit v0.12