Tk Source Code

Merged. Thanks to all participants!

The Github repository is a mirror of the main fossil repository. The fix is in branch bug-17b509d78f of the fossil repository, and in branch bug-17b509d78f of the Github mirror. These branches are off core-8-6-branch in each repo. If you took something else, in particular off trunk, then yes you must have a very different tkWinWm.c file.

See [eeaf3858da].

If nobody objects I plan to merge in a few days from now.

Clean-up is always good !

• Tk 9.1: Win 10
• Tk 9.0: Win 8

I would prefer, if you do the clean-up from 9.0 and the other method on 8.6.

Thanks for all, Harald

Right.

We could simply remove that entire snippet from the codebase, on the grounds that Windows NT is very very obsolete and CreateIconFromResourceEx should not fail.

Or less intrusive: remove the check on the bit count only, so that CreateIconFromResource can keep beking tried in case CreateIconFromResourceEx failed just above. Who could still be using 16-bits resources in icons nowadays?

I would just go for the second possibility. Opinions?

/*
 * It failed, odds are good we're on NT so try the non-Ex way.
 */

if (hIcon == NULL) {
    /*
     * We would break on NT if we try with a 16bpp image.
     */

    if (lpIcon->lpbi->bmiHeader.biBitCount != 16) {
        hIcon = CreateIconFromResource(lpIcon->lpBits, lpIcon->dwNumBytes,
 	    isIcon, 0x00030000);
    }
}

I have proposed a patch [0176f7c06a], implementing this idea that was found on Stackoverflow by Paul Obermeier. See branch bug-17b509d78f.

Could the OP please test?

Could someone (Paul perhaps?) review the patch please? (With commented out debug printfs intentionally left).

Thanks!

OK, so the situation now is clear: PNG images in ico files are not (yet) supported by Tk. The images get interpreted as BMP, which results in totally wrong sizes, which in turn lead to GetIcon() to select the first image of the ico file whatever it is and without taking size into account (reason why it looks blurry in the app.ico example case). The image nevertheless shows up because the Windows OS loads the ico file and the OS known about both BMP and PNG (since Vista) images in ico files.

So the workarounds for the OP are either of:

1. Use BMP images in ico files instead of PNG images

2. Revert the order of the PNG images in the ico file such that the largest image comes first (it will get selected, which results in a not blurred image)

Now, about fixing this and making Tk aware of PNG images in ico files: The .ico format has no embedded switch to let a parser know the format of the images it provides. Paul has provided some hints below about how we could circumvent the problem.

Yes, png was added later on the Windows platform. It features a full alpha channel, instead of only 0/100% like bmp, what is a real change with hidden consequences.

This is not taken into account for TCL.

I allowed me to change the title of the ticket, as this is a valuable and quite old request. We may find this issue in different forms in other tickets.

Paul, would it possible that you have an implementation proposal? I have seen that you have done a similar change in the Img package...

Thanks for all, Harald

The app.ico contains images in PNG format. 
It generates an error message when reading with the ICO parser of Img, because the ICO parser of Img does not support PNG.

Tk uses the Windows function CreateIconFromResourceEx (in MakeIconOrCursorFromResource), so it can read both BMP and PNG images.
But before reading the image, it tries to retrieve image information in function AdjustIconImagePointers.
The problem is in that function, because it is assumed to read a bitmap header (ex. lpImage->lpbi->bmiHeader.biWidth), while in fact there is a PNG header.
That's why it gets garbage regarding image size.
Do not know about a correct solution right now.
One possibility might be to check for the existence of a PNG header in AdjustIconImagePointers and retrieve the needed image information from that header.

What you describe, unless I miss something, points to the creation tool to be faulty, isn't it? You tell the following two things that seem to prove it:

* redketchup.io/icon-converter creates a blurry icon

* using another online tool and this time the icon was not blurry

Yes the icon file opens with GIMP. This does not mean it is fully correct, it only means the GIMP may be tolerant to the ico format being imperfectly filled in.

The ico files format is simple. It is composed of a header and an image directory. The image directory has entries (ICONDIRENTRY structures), and each such entry encodes an image. Your app.ico encodes 6 images, each of them in BMP format. The BMP header contains garbage.

I'm attaching a debugging session snapshot with your app.ico, and another one with WindowsUpdateSeeker2.ico, see the difference?

icotool -l app.ico
--icon --index=1 --width=16 --height=16 --bit-depth=32 --palette-size=0
--icon --index=2 --width=32 --height=32 --bit-depth=32 --palette-size=0
--icon --index=3 --width=48 --height=48 --bit-depth=32 --palette-size=0
--icon --index=4 --width=64 --height=64 --bit-depth=32 --palette-size=0
--icon --index=5 --width=128 --height=128 --bit-depth=32 --palette-size=0
--icon --index=6 --width=256 --height=256 --bit-depth=32 --palette-size=0

One more note: The WindowsUpdateSeeker.ico I have attached has images in reverse size order (larger first) compared to your app.ico. I'm attaching WindowsUpdateSeeker2.ico which is just WindowsUpdateSeeker.ico with the image order reversed (smallest first) as in your app.ico, and this WindowsUpdateSeeker2.ico works OK for me. So the problem is not due to the order of the images within the ico file.

Thanks for your input. Using your app.ico file I can reproduce.

The code here returns the first image because the values of lpIR->IconImages[i].Height|Width a few lines above look like garbage for all 6 images of the icon file.

Checking then how the ico file is read in ReadIconOrCursorFromFile(), I see that the image icon size is set here. And there is garbage in lpImage->lpbi->bmiHeader.biWidth|biHeight.

I'm starting to suspect your app.ico file to be a bit incorrect. Where does this ico file come from? Are you 100 % sure its format it 100% correct? Can you perhaps try with another multi-resolution ico file coming from another icon designer or .ico file source?

I have tried other random multiple resolution icons I have found on my Win11 disk and I'm not seeing the issue. I'm attaching one of those random icons form your test and review of differences with respect to yours (see WindowsUpdateSeeker.ico).

Thank you for taking a look at my issue. I attached an app.ico file and also added two pictures - what im currently seeing and what I would expect. On my Laptop where the DPI is set to 1.5 it is especially noticable.
The low resolution ico can also be seen in the top left corner of the window but that is not so easy to see in a snapshot.

The code snippet you linked seems to reflect exactly what I am seeing. But I am not sure what the best way is to fix this. Tk could let Windows load the .ico itself. Then it could handle DPI-aware selection natively. Or maybe Tk could replicate the resolution-selection logic based on current DPI? Both seem not that easy... But I'm just brainstorming.
I hope you can come up with a good solution. I would appreciate it a lot. :-)

As a first very quick analysis I believe the issue is here.

Would you be so kind as to provide (attach) an example app.ico file containing multiple resolutions, that we could use to reproduce the problem (and later check that the fix is correct)?

In addition, a snapshot of the current result and of the expected result of [wm iconbitmap] would be great.