On 21/06/12 00:20, Rodolfo kix Garcia wrote:
> On 20/06/12 23:50, Carlos R. Mafra wrote:
>
>> On Wed, 20 Jun 2012 at 22:47:52 +0200, Rodolfo kix Garcia wrote:
>>> On 20/06/12 22:12, Carlos R. Mafra wrote:
>>>
>>>> On Wed, 20 Jun 2012 at 21:17:15 +0200, Rodolfo kix Garcia wrote:
>>>>> On 20/06/12 16:01, Carlos R. Mafra wrote:
>>>>>
>>>>>> On Wed, 20 Jun 2012 at 14:54:05 +0100, Carlos R. Mafra wrote:
>>>>>>
>>>>>>> So I'd say the problem lies in the automatic start of applications
>>>>>>> which have no appicon.
>>>>>>>
>>>>>>> I have three xterms which automatically start (through the Save Session)
>>>>>>> and they all have no_appicon set. It seems that the space corresponding
>>>>>>> to three appicons is in use, and if I start some other application
>>>>>>> then its appicon is placed in a shifted position which is equivalent
>>>>>>> to 3 appicons.
>>>>>>>
>>>>>>> And I bisected the problem to this commit.
>>>>>>
>>>>>> It doesn't matter if the automatically start or not. My xterms
>>>>>> have no appicon. After I start one, the space which would be
>>>>>> occupied by its icon is not used by other appicons.
>>>>>>
>>>>>> So if I start one xterm than another app (with appicon) and
>>>>>> so forth, the icons of the application are placed 64 pixels
>>>>>> apart.
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>> I found the problem. The problem is that the wArrangeIcon function
>>>>> checks if the icon exists. Now, the icon always exists, then is added
>>>>> space for the appicon. The function now should check the flag:
>>>>>
>>>>> !wwin->user_flags.no_appicon
>>>>>
>>>>> instead "wwin->icon".
>>>>
>>>> No, the patch does not fix it. I will revert the patch which causes
>>>> this in the first place.
>>>
>>> Carlos,
>>>
>>> if you want to revert the patch, do it, no problem. But IMO, here, the
>>> patch solves the problem :-/
>>
>> You are seeing some other issue then. I also noticed something strange
>> related to changing workspaces and rearranging icons when trying to
>> reproduce the bug with a clean GNUstep/
>>
>> But it definitely does not solve what I'm seeing here.
>
>
> Yes, the wArrangeFunction is called when the wmaker start and when you
> make workspaces changes.
>
>>
>>>> It does not really make sense to do work if we know that no_appicon
>>>> is set, it is a waste.
>>>
>>>
>>> Now, the wApplicationCreate creates wapp and wapp->icon, always!
>>
>> I don't understand why you are saying this. Your patch was named
>> "Create WAppIcon always", wasn't it? What's the difference from
>> now? Look at the revert patch:
>>
>> diff --git a/src/appicon.c b/src/appicon.c
>> index 6cc2bca..c77633d 100644
>> --- a/src/appicon.c
>> +++ b/src/appicon.c
>> @@ -956,9 +956,6 @@ void create_appicon_from_dock(WWindow *wwin,
>> WApplication *wapp, Window main_win
>> {
>> WScreen *scr = wwin->screen_ptr;
>>
>> - /* Create the application icon */
>> - wapp->app_icon = NULL;
>> -
>> if (scr->last_dock)
>> wapp->app_icon = findDockIconFor(scr->last_dock, main_window);
>>
>> diff --git a/src/application.c b/src/application.c
>> index f0122f0..c98a3e8 100644
>> --- a/src/application.c
>> +++ b/src/application.c
>> @@ -143,13 +143,21 @@ WApplication *wApplicationCreate(WWindow * wwin)
>> /* application descriptor */
>> XSaveContext(dpy, main_window, wAppWinContext, (XPointer) wapp);
>>
>> - /* Create the application icon using the icon from docks
>> - * If not found in docks, create a new icon
>> - * using the function wAppIconCreate() */
>> - create_appicon_from_dock(wwin, wapp, main_window);
>> + /* Create the application icon */
>> + wapp->app_icon = NULL;
>>
>> - /* Save the app_icon in a file */
>> - save_app_icon(wapp);
>> + if (!WFLAGP(wapp->main_window_desc, no_appicon)) {
>> + /* Create the application icon using the icon from docks
>> + * If not found in docks, create a new icon
>> + * using the function wAppIconCreate() */
>> + create_appicon_from_dock(wwin, wapp, main_window);
>> +
>> + /* Now, paint the icon */
>> + paint_app_icon(wapp);
>> +
>> + /* Save the app_icon in a file */
>> + save_app_icon(wapp);
>> + }
>>
>> return wapp;
>> }
>>
>> You were creating the icon
>>
>> /* Create the application icon */
>> wapp->app_icon = NULL;
>
> No! I am initializing the icon to NULL, the icon doesn't exists. If you
> check it with "if (wapp->app_icon)" it fails.
>
>> from inside create_appicon_from_dock(), which btw was always called.
>>
>> But now the work is done inside the check if no_appicon is set, and
>> that makes a lot of sense to me.
>
>
> No, create_appicon_from_dock() is called if no_appicon is not set. If is
> set, the icon doesn't exists, and continue with NULL.
>
>>
>>> Then, the checks about "if (wwin->icon)" don't make sense.
>>
>> ?
>
> Yes! :-) if (wwin->icon) is NULL, we don't enter in the if. But in
> wArrangeIcon, with my patch, always exists, and we need check if the
> flag no_appicon is set.
>
>>> If the icon is created always, we don't need check if exists, we don't
>>> need create the icon or destroy,... the code is more easy, we need less
>>> code, and IMO better.
>>
>> I'm not following you. I just moved the
>>
>> create_appicon_from_dock()
>> paint_app_icon()
>> save_app_icon()
>>
>> to inside the no_appicon check.
>>
>> The wapp->app_icon = NULL is being always set, with or without the
>> revert patch.
>
> Yes, but if no_appicon is set, then is NULL.
>
>>
>>> Please, if you want revert the patch, perfect, but try to reproduce the
>>> bug, and I will try to do it too. I can launch the applications, restart
>>> wmaker,... and I don't have the hole. Your help is very appreciated.
>>
>> Right, I didn't fully understand why the bug was happening with your patch.
>> But seeing that you were calling the above three functions even if
>> the no_appicon was set I realized that it didn't make much sense.
>> So I didn't want to spend more time on this.
>>
>> But your reaction to the above patch makes me wonder if I'm missing
>> something.
>
> :)
>
> wapp->app_icon = NULL; /* sets the icon to null */
>
> if the no_appicon is set, then
>
> create_appicon_from_dock(wwin, wapp, main_window);
>
> is never called, then wapp->app_icon is NULL
>
> Then the appicon doesn't exists (is null), and we don't have icon.
>
>
> The old code:
>
>> - /* Create the application icon using the icon from docks
>> - * If not found in docks, create a new icon
>> - * using the function wAppIconCreate() */
>> - create_appicon_from_dock(wwin, wapp, main_window);
>
> Creates the icon always, then wapp->app_icon is not null. If no_appicon
> is set, the icon still exists. We can save the icon, the icon will be
> used in the switchpanel,...
>
> But with the new code:
>
>> + if (!WFLAGP(wapp->main_window_desc, no_appicon)) {
>> + /* Create the application icon using the icon from docks
>> + * If not found in docks, create a new icon
>> + * using the function wAppIconCreate() */
>> + create_appicon_from_dock(wwin, wapp, main_window);
>> +
>> + /* Now, paint the icon */
>> + paint_app_icon(wapp);
>> +
>> + /* Save the app_icon in a file */
>> + save_app_icon(wapp);
>> + }
>
> The icon is created, and saved, if no_appicon is not set. If is set,
> nothing happends. Try this:
>
> 1. Open an application, for example, xcalc.
> 2. Set the no_appicon in xcalc, apply, save, close the xcalc
> 3. rm ~/GNUstep/Library/WindowMaker/CachedPixmaps/*xcalc* (the xcalc
> pixmap)
> 4. Launch now xcalc again:
> 4a. You don't have icon.
> 4b. Hit alt+tab you can't see the xcalc icon
>
> If you do the same with my patch, in 4a and 4b you have the icon. Why?
> because I am creating the icon in save-app_icon().
>
>
>
> With the new code, the wArrangeIcon checks if wwapp->icon exists, If is
> null, then doesn't exists and for this reason the icon is not added in
> the icon list. But with my patch, always exists, therefore, wwapp->icon
> is always true and then the icon is added, but, the icon is not mapped
> in X11, then you cannot see it.
>
> I don't know if my explanation is enougth :-/
Now the current next is not correct. Now the save icon function is not
called always. If the no_appicon is set, the icon is not saved now.
Because the extractIcon function no longer exists, then the icon is not
saved. Probably you should revert most of my patches.
But IMHO revert these patches is not correct. The reason is not because
I did it. The reason is because now you have a problem with a hole in
the miniwindows, but we have a correct method to save the icons, we have
less code, we have the icons in the switchpanel and I am sure that my
patch solves the problem with the holes. I think that these new patches,
perhaps are not fine, but I think that are better than the old code.
Cheers
kix
--
To unsubscribe, send mail to [email protected].
