Calling all users of Tk_RedrawImage

[George Peter Staplin]

I'm here to notify you that it's been broken since 8.4.6 and in 8.5.
The core team is thinking of fixing it by essentially changing the usage
requirements of it.

We have a serious problem with it. Ever since the COMPLEX_ALPHA
handling was added to Tk photo images a serious bug has lurked, or 2
when using PNG images that have non-boolean transparency.

First it was added in haste without much testing, but it passed the Tk
test suite. Then I reported a priority 9 bug with the usage of
XGetImage that resulted in a fatal X error. It was fixed incorrectly.

There are several NEGATIVE implications of this bug. First, the current
behavior in the 8.4 and the HEAD (what will become 8.5.0) results in
scrolled images not displaying at all in text widgets, and other
widgets.

This demonstrates the bug:
http://www.xmission.com/~georgeps/tmp/clipping_bug.tcl

The bug is here in tkImgPhoto.c:
Tk_RedrawImage calls ImgPhotoDisplay():

handler = Tk_CreateErrorHandler(display, -1, -1, -1, NULL,
(ClientData) NULL);

/*
* Pull the current background from the display to blend with
*/

bgImg = XGetImage(display, drawable, drawableX, drawableY,
(unsigned int)width, (unsigned int)height, AllPlanes,
ZPixmap);
if (bgImg == NULL) {
Tk_DeleteErrorHandler(handler);
return;
}

ImgPhotoBlendComplexAlpha(bgImg, instancePtr, imageX, imageY,
width,
height);

This appears simple, it calls XGetImage to retrieve the background area,
to blend with. This has several problems.

1. if the drawableX/drawableY are negative, as with the case of a text
widget, the XGetImage causes an X error and returns NULL. Tk currently
doesn't draw anything, hence the bug in the test case.

2. if the width/height are out of bounds of the drawable, then an X
error will occur, and the same problem results as in case 1.

The proper fix for past compatibility that solves all of these problems
is to call XGetGeometry() using the drawable to get the valid geometry
of the drawable, and constrain the values. However, this I've been told
by Joe English isn't valid, because he and Jeff Hobbs have noticed it's
unusably slow over the network after that.

So, the direction Joe English is heading in is to require that all users
of Tk_RedrawImage constrain the drawable coordinates themselves. This
was never a requirement before, so it may break some old applications
that haven't been upgraded since the COMPLEX_ALPHA code was added.

How will this affect you? How has this already affected you? Do you
know of another way?

George

P.S. An alternative is to break the API for the image layer, and require
caching the drawable geometry, but that's not really an option this soon
prior to the 8.5.0 release.

[briang]

On Dec 8, 7:39 pm, George Peter Staplin

We're using 8.4.14 and have not seen any problems using Tk_RedrawImage
that I'm aware of. The nature of our use is such that it would never
get called with a negative drawableX/Y, but it can be out of bounds on
the other end. This could change if/when we switch to pixel scrolling
instead of the current line scrolling.

Our use is minimal (i.e. 4 instances total) and it would be no big
deal for me to put in the required clipping; a reasonable requirement
assuming it's well documented.

An alternative approach would be to provide a new API that requires
the user to supply the background image or color. I can easily supply
this as well without an XGetImage() call, but granted, my case is a
simple one. (Tk_RedrawImageWithBGImg, Tk_RedrawImageWithBGColor)

-Brian