The patch below "fixes" the black & white icon bug for me. But I am still not completely happy with the code, because I don't understand yet why some icons are saved inside CachedPixmaps with full colors and others are only black & white. From Brad's argument, I thought wIconStore() saves black & white pixmaps by using the WM_HINTS stuff.
Furthermore, I removed the part where the pixmap was being saved inside CachedPixmaps/. This saving must be restored from some other place, and hopefully using the color icon image inside wwin->net_icon_image (if present) instead of calling RCreateImageFromDrawable(). The commit log is long in the attempt to explain what was going on and to put my own thoughts into place. If there are mistakes with my understanding of the bug, hopefully someone will notice by reading the log. The patch is on 'next', but I think it needs more fixes/tweaks on top of it. Needless to say, test it. ---8<--- >From e9ba93f58b384d7f3cdeb69c1af0d3fad961de75 Mon Sep 17 00:00:00 2001 From: Carlos R. Mafra <[email protected]> Date: Mon, 20 Sep 2010 10:35:55 +0200 Subject: [PATCH] Avoid black & white icon issue When an application starts, the code in wApplicationCreate() which deals with the icon creation/display was doing some redundant loading of the icon image in the final steps. The story goes like this. The icon supplied by the application is stored early in the birth process inside wwin->net_icon_image by wNETWMCheckInitialClientState(). After checking that the icon is not already in the dock or clip, the code in wApplicationCreate() calls wAppIconCreate(), which does some housekeeping and calls wIconCreate(). wIconCreate() honors the early client-supplied net_icon_image image if present. If not, it searches a previous saved image reading the "Icon" field from ~/GNUstep/Defaults/WMWindowAttribute, if (!icon->image && !WFLAGP(wwin, always_user_icon)) icon->image = RRetainImage(wwin->net_icon_image); if (!icon->image) icon->image = wDefaultGetImage(scr, wwin->wm_instance, wwin->wm_class); Everything OK so far. The appicon already has an image at this point. But close to the end of wApplicationCreate(), wmaker was checking again if it had already saved an icon for the application in the past. And if that was the case but the icon image wasn't currently inside CachedPixmaps/ its image would be created and saved again with wIconStore() and then loaded back. So all the work before this point regarding the image was pointless. The black & white bug was happening because wIconStore() happens to save a black & white pixmap despite the net_icon_image already having full color glory. And then that last redundant loading was discarding the color supplied image to load this black & white icon. Therefore, removing the redundant pixmap creation + loading avoids the black & white icon issue. In my case I had a test case with 'dolphin'. If the black & white pixmap was already inside CachedPixmaps, the icon would appear colored (because there was no need to recreate + load the icon in the redundant phase). If I removed the icon from CachedPixmaps, wmaker would detect that it had already created one icon for it in the past by looking at its "Icon" in WMWindowAttributes. Then it would do the broken "create black & white pixmap and use its image". To explain Bento Loewenstein's test procedure - open a QT4 app (dolphin, konqueror, kopete, whatever). see if the icon is black and white; - if it is, right click on the title bar and select "Attributes..."; - choose "Application Specific" on the drop-down menu; - set "No application icon" and save; - UNset "No application icon" and save; - the icon should come back full color; it suffices to note that the code in winspector.c was calling makeAppIconFor() when the settings were applied. And makeAppIconFor() calls wAppiconCreate() --> wIconCreate(), which retains the net_icon_image and does not do the redundant stuff from wApplicationCreate(). So no black & white icon is created there. Signed-off-by: Carlos R. Mafra <[email protected]> --- src/application.c | 35 +---------------------------------- 1 files changed, 1 insertions(+), 34 deletions(-) diff --git a/src/application.c b/src/application.c index af0fe61..c5f2c56 100644 --- a/src/application.c +++ b/src/application.c @@ -358,42 +358,9 @@ WApplication *wApplicationCreate(WWindow * wwin) XMapWindow(dpy, icon->core->window); } - if (wPreferences.auto_arrange_icons && wapp->app_icon && !wapp->app_icon->attracted) { + if (wPreferences.auto_arrange_icons && wapp->app_icon && !wapp->app_icon->attracted) wArrangeIcons(scr, True); - } - if (wapp->app_icon) { - char *tmp, *path; - struct stat dummy; - RImage *image; - - tmp = wDefaultGetIconFile(scr, wapp->app_icon->wm_instance, wapp->app_icon->wm_class, True); - - /* If the icon was saved by us from the client supplied icon, but is - * missing, recreate it. */ - if (tmp && strstr(tmp, "Library/WindowMaker/CachedPixmaps") != NULL && - stat(tmp, &dummy) != 0 && errno == ENOENT) { - wmessage(_("recreating missing icon '%s'"), tmp); - path = wIconStore(wapp->app_icon->icon); - if (path) { - wfree(path); - } - image = wDefaultGetImage(scr, wapp->app_icon->wm_instance, wapp->app_icon->wm_class); - if (image) { - wIconChangeImage(wapp->app_icon->icon, image); - wAppIconPaint(wapp->app_icon); - /* TODO: - * wIconChangeImage() should be rewriten to use retain/release - * The way it is now is too confusing about where the icon is - * finally released. -Dan */ - /* --this is wrong at the moment-- RReleaseImage(image); */ - } - } - - /* if the displayed icon was supplied by the client, save the icon */ - if (!tmp) - extractClientIcon(wapp->app_icon); - } return wapp; } -- 1.7.2.2.119.gf9c33 -- To unsubscribe, send mail to [email protected].
