Tk Source Code

Copy of my message from comp.lang.tcl (https://groups.google.com/d/msg/comp.lang.tcl/pgi3lDa1yQM/HPOHaNx7CAAJ)

I've been told that this check in from 2004: https://core.tcl.tk/tk/info/0eb7a8a6ca8d4336
... is where the problem originated and that this was in response to this bug: https://core.tcl.tk/tk/tktview?name=223689ffff

------------------------------------------
I've been working in the Tk canvas and I wanted to obtain an XImage from the Pixmap the canvas is drawing into, and then copy that to an image using the Tk_Tk_PhotoPutBlock() and Tk_PhotoPutZoomedBlock(). This all works now but I found some problems along the way.

The conversion between XImage and Tk_PhotoImageBlock is where most of the work has to be done. I wrote the conversion code with reference to Volume One: Xlib Programming Manual 1992 and some other references. Using the XImage bitmap_unit as the pixel bit unit width and bytes_per_line as the byte length of a line. I obtained the RGB masks from the Visual (Tk_Visual()).

Works well on my Linux machine and I tested it in 32 and 16 bit colour mode.

Across on the Windows machine things were a little different. I had sketched some circles and rectangles in various colours on the canvas for the Unix tests. When I tested on Windows I had both geometry problems (looked like raster tearing) and colour problems.

Examining the XImage data on Linux and Windows revealed this:

Linux:
# ximagePtr { width 10 height 10 xoffset 0 format 2 data { 0x80 0x00 0x00 0xff ... } byte_order 0 bitmap_unit 32 bitmap_bit_order 0 bitmap_pad 32 depth 24 bytes_per_line 40 bits_per_pixel 32 red_mask 0x00000000 green_mask 0x00000000 blue_mask 0x00000000 }

Windows:
# ximagePtr { width 10 height 10 xoffset 0 format 2 data { 0x80 0x00 0x00 0x00 ... } byte_order 0 bitmap_unit 8 bitmap_bit_order 0 bitmap_pad 0 depth 32 bytes_per_line 40 bits_per_pixel 32 red_mask 0x00000000 green_mask 0x00000000 blue_mask 0x00000000 }

On Linux I had a bitmap_unit of 32 (which it should be for a 32BPP display). On windows, also with a 32BPP display, I instead had 8! Further to this the Windows XImage->bitmap_pad is 0 when it should be 32. The rest of the information is correct.

To correct the geometry problem on Windows I had to ignore XImage->bitmap_unit and use XImage->bits_per_pixel. This does work on both platforms but I think the XImage should return much the same information on both platforms which means a 32 for bitmap_unit and bitmap_pad.

My second problem was colour. On Windows it looked like the RED and GREEN were swapped. This surprised me as I went to some effort to examine the colour masks, and work out the shift counts. I figured this would work universally. Not so.

I then examined the colour masks returned in the Visual and found the red and blue to be opposite on Windows and Linux:

Linux:
# visualPtr { red_mask 0x00ff0000 green_mask 0x0000ff00 blue_mask 0x000000ff }

Windows:
# visualPtr { red_mask 0x000000ff green_mask 0x0000ff00 blue_mask 0x00ff0000 }

But I pulled out a few of the bytes from the XImage data on both systems. You can see it above but I'll reproduce it here:

Linux: { 0x80 0x00 0x00 0xff ... }
Windows: { 0x80 0x00 0x00 0x00 ... }

Ignoring the alpha value on the RHS, the pixel data is exactly the same. This is the first pixel of a 10x10 image with blue on the left edge, so the 0x80 is the very first byte of blue data. Now looking at the pixel masks, the Linux blue_mask is 0x0000ff and picks up the blue properly. The Windows blue mask is 0x00ff0000 so instead it picks up the 0x80 as a red colour.

This is where my colour troubles came from. In the end I left this code the same and patched it later when I stored the bytes in the Tk_PhotoImageBlock:

#ifdef _WIN32
#define   R_OFFSET 2
#define   B_OFFSET 0
#else
#define   R_OFFSET 0
#define   B_OFFSET 2
#endif
            blockPtr.pixelPtr[blockPtr.pitch * y + blockPtr.pixelSize * x + R_OFFSET] = (pixel & visualPtr->red_mask) >> rshift;
            blockPtr.pixelPtr[blockPtr.pitch * y + blockPtr.pixelSize * x +1] = (pixel & visualPtr->green_mask) >> gshift;
            blockPtr.pixelPtr[blockPtr.pitch * y + blockPtr.pixelSize * x + B_OFFSET] = (pixel & visualPtr->blue_mask) >> bshift;
            blockPtr.pixelPtr[blockPtr.pitch * y + blockPtr.pixelSize * x +3] = 0xFF;

And because of the problem with the masks I have done what I was trying to avoid which is add an #ifdef _WIN32. But this does correct the problem and the tests are now the same on both Windows and Linux.

But why is the Visual colour masks wrong on Windows? I think they are hard coded that way:

TkWinDisplayChanged(
    Display *display)
{
.........
    } else if (screen->root_depth >= 24) {
        screen->root_visual->class = TrueColor;
        screen->root_visual->map_entries = 256;
        screen->root_visual->red_mask = 0xff;
        screen->root_visual->green_mask = 0xff00;
        screen->root_visual->blue_mask = 0xff0000;

And then I wondered why the rest of the Tk system functions properly. Looking at the tkCanvPs.c module it appears to call XGetPixel(). On Windows ImageGetPixel() and PutPixel() just ignore the masks :

ImageGetPixel(
    XImage *image,
    int x, int y)
{
......
    switch (image->bits_per_pixel) {
    case 32:
    case 24:
        pixel = RGB(srcPtr[2], srcPtr[1], srcPtr[0]);
        break;


Perhaps the Visual masks in TkWinDisplayChanged() should be changed?

Unfortunately the fix had to be reverted in trunk, see ticket [d66e6fabad].

This is now merged to trunk.

A huge thanks to Scott who provided such a clear analysis and adequate patch!

Thanks to you both!

I have committed the updated patch in the bugfix branch. The Tk test suite now runs fully OK for me.

I've attached another diff file with the new changes. I've tested these on both the tk test suite and my test script.

| Regarding the crash, could it be caused by wrong
| allocation size in the RGB case. See this link

Yes, good catch! I'm accessing memory that has not been allocated (...sizeof(BITMAPINFOHEADER))+1)... etc)! I'll include this fix. Thanks :)

Francois, thanks for testing the fix and sorting out the fossil side of things :)

Answers:

1. Any functions that need to render an image must call TkPutImage() or XPutImage() (which on Windows calls TkPutImage() anyway - see win/tkWinDraw.c). For example TkImgPhotoDisplay(), which is used to draw any photo image, calls TkPutImage(). The demos probably are not sufficient by themselves - I should have run the tk test suite (see 4 below).

2. Removing NO_WINRGBFIX is fine yes. I added it because I compile and test from a single source directory into different build and install directories for patched and unpatched builds. I will checkout the repo again, update to the new branch and then I'll remove the #if's and old code.

3. You are correct about the comment in tkCanvas.c. I'll remove that after I do #2 above.

4. I ran the test suits but I get exactly the same result for both patched and unpatched:

Tests ended at Sun Dec 30 11:18:08 +1100 2018
all.tcl:        Total   470     Passed  456     Skipped 14      Failed  0
Sourced 17 Test Files.
Number of tests skipped for each constraint:
NA
coreEntry
coreScrollbar
nyi
stderr32/c/Users/scottyw/Desktop/src/tcltk/core.tcl.tk/tk-rgbmask-fix
Done!

I'm building in an MSYS shell. Are you building with Mingw or with MSVC?

Regarding the crash, could it be caused by wrong
allocation size in the RGB case. See this link

  https://www.androwish.org/index.html/artifact?ln=555-561&name=0fd71b2908793521

for a proper solution (IMHO).

Thank you so much Scott, for this valuable work!!

I have pushed your fix into a bugfix branch.

I'm in the process of looking at the detail of your proposed fix.

Right now I have a few questions:

1. You have identified three ways of rendering an image, which are: an image on a button (to see if Tk can render a simple image from a file properly), a canvas with some colour rectangles as a control, and then lastly an image on a button which used the DrawCanvas() function via "canvas image....". Are we sure that there is no other way than these three? How can we make sure that there is no further change required in some other area of Tk? Do you think the demos are sufficient?

2. Shouldn't we just completely get rid of the NO_WINRGBFIX define, which is anyway not defined, unless specifically when testing this patch? Now that the fix is in a bugfix branch from/to which can easily switch I think we should do it.

3. Did you go through the comments in the code, to check whether they are still correct? For instance in tkCanvas.c around line 2772 I think this should be removed.

4. Running the test suite on Windows, I got a crash "alloc: invalid block: 0421D018: ef ef ff" in canvPs-3.1 (and another one later in the test suite, same symptom). Does the test suite run without crash nor failure for you (on Win)? I may have made a mistake when applying the patch but I couldn't find where.

4. And I verified that images are working fine with the Tk demos (see images 4.png and 5.png)

3. The last change is the important one to TkPutImage() in tkWinDraw.c. As I mentioned previously I use BI_BITFIELDS bit map mode, which allows me to insert the colour masks and let Windows render the colours properly. Now the red and blue do need to be different to the tk visual but this is the difference between ARGB on Windows and ABGR on Unix. And after this last change is applied we now have matching images: (see image 3.png)

2. Now I applied the second change to tkCanvas.c. Because if Tk is not rendering images properly then DrawImage() should also be incorrect. SO I undid the hack I inserted for Windows, to swap the red and blue. Now the test result looks like below. And you can see both images now incorrect. The canvas looks fine as it should (no images there): (see image 2.png)

Details of the changes:

1. Firstly in win/tkWinX.c I changed the actual masks in the tkWinDisplayChanged() function. This corrects the visual masks on the Windows platform.  Now once I did this then some other parts of Tk draw images incorrectly. Here my test script uses an image on a button (to see if Tk can render a simple image from a file properly), a canvas with some colour rectangles as a control, and then lastly an image on a button which used the DrawCanvas() function via "canvas image....". The two images and the canvas should look the same. After applying this first change my result looks like this image. You can see that Tk is now NOT rendering loaded images correctly. (see following image attachment)

I've corrected the visual masks in TkWinDisplayChanged() (in tkWinX.c), and also corrected DrawCanvas() in tkCanvas.c, and corrected TkPutImage() in tkWinDraw.c to use the BI_BITFIELDS mode for CreateDIB(). I'll attached a diff file to this ticket.