Improve image testing on macOS.

2 files changed, 49 insertions, 38 deletions

diff --git a/generic/tkTest.c b/generic/tkTest.c
index e80f488..c22e649 100644
--- a/generic/tkTest.c
+++ b/generic/tkTest.c
@@ -71,6 +71,8 @@ typedef struct TImageInstance {
     TImageMaster *masterPtr;	/* Pointer to master for image. */
     XColor *fg;			/* Foreground color for drawing in image. */
     GC gc;			/* Graphics context for drawing in image. */
+    Bool displayFailed;         /* macOS display attempted out of drawRect. */
+    char buffer[200 + TCL_INTEGER_SPACE * 6]; /* message to log on display. */
 } TImageInstance;
 
 /*
@@ -1517,6 +1519,7 @@ ImageGet(
     instPtr->fg = Tk_GetColor(timPtr->interp, tkwin, "#ff0000");
     gcValues.foreground = instPtr->fg->pixel;
     instPtr->gc = Tk_GetGC(tkwin, GCForeground, &gcValues);
+    instPtr->displayFailed = False;
     return instPtr;
 }
 
@@ -1551,41 +1554,50 @@ ImageDisplay(
 				 * imageX and imageY. */
 {
     TImageInstance *instPtr = (TImageInstance *) clientData;
-    char buffer[200 + TCL_INTEGER_SPACE * 6];
 
     /*
      * The purpose of the test image type is to track the calls to an image
-     * display proc and record the parameters passed in each call.  On macOS
-     * a display proc must be run inside of the drawRect method of an NSView
-     * in order for the graphics operations to have any effect.  To deal with
+     * display proc and record the parameters passed in each call.  On macOS a
+     * display proc must be run inside of the drawRect method of an NSView in
+     * order for the graphics operations to have any effect.  To deal with
      * this, whenever a display proc is called outside of any drawRect method
-     * it schedules a redraw of the NSView by calling [view setNeedsDisplay:YES].
-     * This will trigger a later call to the view's drawRect method which will
-     * run the display proc a second time.
+     * it schedules a redraw of the NSView.
      *
-     * This complicates testing, since it can result in more calls to the display
-     * proc than are expected by the test.  It can also result in an inconsistent
-     * number of calls unless the test waits until the call to drawRect actually
-     * occurs before validating its results.
-     *
-     * In an attempt to work around this, this display proc only logs those
-     * calls which occur within a drawRect method.  This means that tests must
-     * be written so as to ensure that the drawRect method is run before
-     * results are validated.  In practice it usually suffices to run update
-     * idletasks (to run the display proc the first time) followed by update
-     * (to run the display proc in drawRect).
-     *
-     * This also has the consequence that the image changed command will log
-     * different results on Aqua than on other systems, because when the image
-     * is redisplayed in the drawRect method the entire image will be drawn,
-     * not just the changed portion.  Tests must account for this.
+     * In an attempt to work around this, each image instance maintains it own
+     * copy of the log message which gets written on the first call to the
+     * display proc.  This usually means that the message created on macOS is
+     * the same as that created on other platforms.  However it is possible
+     * for the messages to differ for other reasons, namely differences in
+     * how damage regions are computed.
      */
 
     if (LOG_DISPLAY(drawable)) {
-	sprintf(buffer, "%s display %d %d %d %d",
-		instPtr->masterPtr->imageName, imageX, imageY, width, height);
+	if (instPtr->displayFailed == False) {
+
+	    /*
+	     * Drawing is possible on the first call to DisplayImage.
+	     * Log the message.
+	     */
+
+	    sprintf(instPtr->buffer, "%s display %d %d %d %d",
+	    instPtr->masterPtr->imageName, imageX, imageY, width, height);
+	}
 	Tcl_SetVar2(instPtr->masterPtr->interp, instPtr->masterPtr->varName,
-		    NULL, buffer, TCL_GLOBAL_ONLY|TCL_APPEND_VALUE|TCL_LIST_ELEMENT);
+		    NULL, instPtr->buffer,
+		    TCL_GLOBAL_ONLY|TCL_APPEND_VALUE|TCL_LIST_ELEMENT);
+	instPtr->displayFailed = False;
+    } else {
+
+	/*
+         * Drawing is not possible on the first call to DisplayImage.
+	 * Save the message, but do not log it until the actual display.
+	 */
+	
+	if (instPtr->displayFailed == False) {
+	    sprintf(instPtr->buffer, "%s display %d %d %d %d",
+		    instPtr->masterPtr->imageName, imageX, imageY, width, height);
+	}
+	instPtr->displayFailed = True;
     }
     if (width > (instPtr->masterPtr->width - imageX)) {
 	width = instPtr->masterPtr->width - imageX;
diff --git a/tests/canvImg.test b/tests/canvImg.test
index 1abea78..b57e21d 100644
--- a/tests/canvImg.test
+++ b/tests/canvImg.test
@@ -174,7 +174,7 @@ test canvImg-4.2 {ConfigureImage procedure} -constraints testImageType -setup {
     while {"timed out" ni $y && [lindex $y end 1] ne "display"} {
         vwait y
     }
-    after cancel timer
+    after cancel $timer
     list $x $y [.c bbox i1]
 } -cleanup {
     .c delete all
@@ -739,7 +739,6 @@ test canvImg-10.1 {TranslateImage procedure} -constraints testImageType -setup {
     foo changed 2 4 6 8 30 15
     vwait x
     after cancel $timer
-    update
     return $x
 } -cleanup {
     .c delete all
@@ -756,7 +755,7 @@ test canvImg-11.1 {TranslateImage procedure} -constraints testImageType -setup {
     set x {}
     set timer [after 500 {lappend x "timed out"}]
     foo changed 2 4 6 8 40 50
-        vwait x
+    vwait x
     after cancel $timer
     update
     return $x
@@ -779,17 +778,18 @@ test canvImg-11.2 {ImageChangedProc procedure} -constraints {
 	.c delete all
 	image delete foo
 } -result {30 75 70 125}
-if {[tk windowingsystem] == "aqua" && $tcl_platform(osVersion) > 18} {
-    # Aqua >= 10.14 will redraw the entire image.
+if {[tk windowingsystem] == "aqua"} {
+#     # For this test only the 20x40 upper left corner of foo2 needs
+#     # to be redrawn, but Aqua redraws the entire image.
     set result_11_3 {{foo2 display 0 0 80 60}}
-} else {
-    set result_11_3 {{foo2 display 0 0 20 40}} 
+ } else {
+   set result_11_3 {{foo2 display 0 0 20 40}} 
 }
 test canvImg-11.3 {ImageChangedProc procedure} -constraints {
 	testImageType 
 } -setup {
     .c delete all
-    update
+    update idletasks
 } -body {
     image create test foo -variable x
     image create test foo2 -variable z
@@ -797,16 +797,15 @@ test canvImg-11.3 {ImageChangedProc procedure} -constraints {
     foo2 changed 0 0 0 0 80 60
     .c create image 50 100 -image foo -tags image -anchor nw
     .c create image 70 110 -image foo2 -anchor nw
-    update idletasks
     set z {}
     set timer [after 500 {lappend z "timed out"}]
-    image create test foo -variable x
-    vwait x
+    image delete foo
+    vwait z
     after cancel $timer
     return $z
 } -cleanup {
     .c delete all
-    image delete foo foo2
+    image delete foo2
 } -result $result_11_3
 
 # cleanup