Menu

#12 Fix icon setting logic to use windows rc file

2.2
closed
None
2015-09-27
2013-05-21
No

When I added the logic to set the wxIcon for the main window, I copied it from FMSLogo. Later someone reported a problem with the icon in FMSLogo not showing up on Vista in high contrast mode, which I fixed by using the icon resource from the compiled .RC file, instead of from an XPM file.

The same fix should be applied to GalaXQL.

Discussion

  • Jay Satiro

    Jay Satiro - 2014-08-19

    If there is a way for me to reproduce please let me know. Do you mean high dpi maybe not high contrast? I tried in 120 dpi on Windows XP but the icon appears in ALT-TAB fine for me regardless of whether I build with the legacy app.sln or the solution I contributed for VS2010+.

    I can look into this if you like but can you find the revision and the bug, and I will try to apply similar changes to GalaXQL.

     
  • David Costanzo

    David Costanzo - 2014-08-22

    Yes, I likely meant High DPI and was probably thinking of this bug.

    https://sourceforge.net/p/fmslogo/bugs/421/

    And the current GalaXQL code uses the incorrect fix which I described in

    https://sourceforge.net/p/fmslogo/bugs/409/

    I was never able to reproduce it either and I don't think it can be reproduced on Windows XP. It might require Vista. Vista uses a completely different system for High DPI than XP does. It would not surpise me if this is really a bug in Vista that was fixed in Windows 7.

    The likelihood and impact of this bug are pretty low, especially when you consider that GalaXQL only gets a few downloads per week. The only reason I opened the bug is that I knew it would be easy to fix and it's possible that someone else would copy this code into a program where the bug is more likely.

    I don't think it's related to the compiler, but I didn't use VS to build FMSLogo or GalaXQL, as I prefer using free tools for open source projects. The .sln file is Jari's.

     
  • Jay Satiro

    Jay Satiro - 2014-08-22

    For FMSLogo I was able to reproduce using the version described in the bug. For GalaXQL I was able to reproduce using your 2.1 version from sourceforge but not any of my Visual Studio builds. I used a Vista x86 VM with dpi set to 120. I will leave it to you if you want to pursue this further.

     
  • David Costanzo

    David Costanzo - 2014-08-23

    That's interesting. Well, I don't think it's worth your time, but if you don't mind attaching one of your builds to this thread, I'll try to disassemble the resources and see if there's an obvious difference.

    I see in the .vcproj file, the "resources" section references both the .rc file and the icon. The .rc file also references the icon, so I'm not sure that's significant. It's also possible that MingGW's windres.exe doesn't compile the resources in the same way that Microsoft's RC.EXE does.

    For reference, here's the code in the fixed FMSLogo, which I copied from Audacity.

    #ifdef __WXMSW__
        // On Windows use the .ico in the resource.
        // This ensures that an icon is created, even at
        // high DPI.  See bug #421.
        wxIcon icon(wxICON(fmslogo));
    
        TopLevelWindow.SetIcon(icon);
    #else
        #include "fmslogo-16x16.xpm"
        #include "fmslogo-32x32.xpm"
        #include "fmslogo-64x64.xpm"
    
        wxIcon icon16x16(fmslogo_16x16_xpm);
        wxIcon icon32x32(fmslogo_32x32_xpm);
        wxIcon icon64x64(fmslogo_64x64_xpm);
    
        wxIconBundle icons;
        icons.AddIcon(icon16x16);
        icons.AddIcon(icon32x32);
        icons.AddIcon(icon64x64);
    
        TopLevelWindow.SetIcons(icons);
    #endif
    
     
  • Jay Satiro

    Jay Satiro - 2014-08-23

    Ok I've attached a build from today that I tested at 120 dpi on Vista.

     
    • David Costanzo

      David Costanzo - 2014-08-24

      Thanks, Jay. I didn't see any difference in the resources that I would think could cause this. Your binary includes a manifest that mine doesn't, but I didn't see any configuration within the manifest that would explain the difference. So maybe Visual Studio somehow makes the icon work. Or maybe it was a bug that got fixed in wxWidgets 3.0. I'll probably just make the code change that I think should fix this and close the bug.

      By the way, I wasn't able to run your version because my OS doesn't have a MSVCP100.dll. When Jari created released GalaXQL 2.0, he included the C++ runtime redistributable DLL in the release. I chose to statically link the GNU C++ runtime into 2.1, so my release doesn't need one. I don't know if you plan to redistribute your release, but if you do and you want it to run on machines like mine, you'll have to address this.

       
  • David Costanzo

    David Costanzo - 2014-08-26
    • status: open --> closed
     
  • David Costanzo

    David Costanzo - 2014-08-26

    I have committed the fix, ported from FMSLogo.

     
  • David Costanzo

    David Costanzo - 2015-09-27
    • status: closed --> open
     
  • David Costanzo

    David Costanzo - 2015-09-27

    I noticed a problem with the FMSLogo icon in the task manager on XP which made me dig deeper into wxWidgets and Windows API. In short, I made a mistake in FMSLogo, then copied that mistake to GalaXQL. I am reopening this bug so that it can be fixed in what I now believe is the right way.

    To summarize the relevant bit from FMSLogo bug #421, Windows assigns two icons to each window: a small one and a normal one and uses them in different situations. Depending on DPI, the dimensions of the small and normal icons are different. When you direct wxWidgets to a particilar XPM for the icon, it assigns an icon of those dimensions to the window, and Windows picks the best icon for its needs. When you assign the icon by an .ico file, wxWidgets only reads the "normal" icon from the .ico, scaling it as appropriate to meet what Windows wants.

    The MSDN guidance for icon suggests putting many difference sizes so that the developer makes the "normal" and "small" icon look nice at every possible icon size (16, 20, 24, 32, 40, 48, 64) instead of letting the operating system scale a larger icon down.

     
  • David Costanzo

    David Costanzo - 2015-09-27
    • status: open --> closed
     

Log in to post a comment.