Re: FvwmIconMan still sometimes triggers Hint Warnings
On Wed, Mar 1, 2017 at 9:24 AM, Dominik Vogt wrote: > On Wed, Mar 01, 2017 at 07:18:11AM -0700, Jaimos Skriletz wrote: >> On Wed, Mar 1, 2017 at 5:30 AM, Dominik Vogt wrote: >> > On Tue, Feb 28, 2017 at 10:22:33PM -0700, Jaimos Skriletz wrote: >> >> On Mon, Feb 27, 2017 at 11:10 PM, Jaimos Skriletz >> >> wrote: >> >> > Hello, >> >> > >> >> > FvwmIconMan still reports Hint warnings even with the patchs applied. >> >> >> >> I wrote this small patch that has FvwmIconMan wait until the window >> >> has resized to set the window hints. This is in the lines of Dominik >> >> Vogt's patch, but waits until FvwmIconMan has fully resized to run >> >> fix_manager_size() to set the window hints. My tests here no longer >> >> seem to tiger the warnings like it still sometimes did. >> >> >> >> As an additional thought, talking to Thomas Adam about the patch in >> >> #fvwm, he mentioned that the issue is more FVWM being overly cautious >> >> and the fix should be in how fvwm handles size hint warnings over >> >> working with FvwmIconMan to avoid triggering them. >> >> >> > >> > So, if it's not possible to completely fix FvwmIconMan in a decent >> > way, what do we do with the warning? I'd rather not disable it, >> > but the number of false positives is too high. One could make a >> > style that disables the warning for a certain class of windows, >> > but that would probably be used as "style * ...", disabling the >> > warning for everything. :-/ >> > >> >> FvwmIconMan isn't the only application that triggers these warnings, >> but using it in a situation where it grows/shrinks is a very regular >> way to cause the warnings and it fills up .xsession-error with mostly >> warnings. Other applications I use only trigger these warnings rarely, >> and in each case the application appears to work fine so yet more >> false positives. But FvwmIconMan is the only one that regularly >> resizes itself triggering the warning a lot. > > Of course. Flooding the log was not the intention of the patch. > >> I had two ideas on this, one is use a BugOpts option that can turn on >> these warnings for a user who wants to debug things, but they are off >> by default. This is basically your style idea and the affect will be >> almost everywhere these warnings will not appear and thus may miss out >> on programs that actually need to be reported. > > Yes, but defaulting to "off" makes the warning effectively useless. > Yea, hence the same line as a style and then turning it off. Though if a window is misbehaving it could be turned on to see if it is triggering these warnings. But one would have to know a window is misbehaving and not able to just look in the logs to see that one is. >> Another is maybe don't have fvwm jump to conclusions that there is a >> warning. > > Let me rephrase that: Fvwm informs the user about something that > might not be possible to clean up. In such a situation the user > would see that the window did not get resized as intended and has > no clue why. At least, fvwm has told her that something strange > was going on. > >> What if there was some timer that fvwm gave the application >> to fix itself before reporting a warning, and then the warning is not >> just that the window had this inconstant state which seems to give >> false positives, but it has been inconsistent for a set amount of time >> without fixing itself. > > Too complex stuff for such a simple situation. Maybe one could > peek the event queue for an event that fixes the windows's size > and not complain if such an event is already pending? Another > option is to generate only a limited number of these warning for > each window. > > Does the attached patch improve the situation? > I have applied the patch and it doesn't seem to change the situation, FvwmIconMan is still triggering the hint size warnings every time a window is added or removed. jaimos
Re: FvwmIconMan still sometimes triggers Hint Warnings
On Wed, Mar 01, 2017 at 07:18:11AM -0700, Jaimos Skriletz wrote:
> On Wed, Mar 1, 2017 at 5:30 AM, Dominik Vogt wrote:
> > On Tue, Feb 28, 2017 at 10:22:33PM -0700, Jaimos Skriletz wrote:
> >> On Mon, Feb 27, 2017 at 11:10 PM, Jaimos Skriletz
> >> wrote:
> >> > Hello,
> >> >
> >> > FvwmIconMan still reports Hint warnings even with the patchs applied.
> >>
> >> I wrote this small patch that has FvwmIconMan wait until the window
> >> has resized to set the window hints. This is in the lines of Dominik
> >> Vogt's patch, but waits until FvwmIconMan has fully resized to run
> >> fix_manager_size() to set the window hints. My tests here no longer
> >> seem to tiger the warnings like it still sometimes did.
> >>
> >> As an additional thought, talking to Thomas Adam about the patch in
> >> #fvwm, he mentioned that the issue is more FVWM being overly cautious
> >> and the fix should be in how fvwm handles size hint warnings over
> >> working with FvwmIconMan to avoid triggering them.
> >>
> >
> > So, if it's not possible to completely fix FvwmIconMan in a decent
> > way, what do we do with the warning? I'd rather not disable it,
> > but the number of false positives is too high. One could make a
> > style that disables the warning for a certain class of windows,
> > but that would probably be used as "style * ...", disabling the
> > warning for everything. :-/
> >
>
> FvwmIconMan isn't the only application that triggers these warnings,
> but using it in a situation where it grows/shrinks is a very regular
> way to cause the warnings and it fills up .xsession-error with mostly
> warnings. Other applications I use only trigger these warnings rarely,
> and in each case the application appears to work fine so yet more
> false positives. But FvwmIconMan is the only one that regularly
> resizes itself triggering the warning a lot.
Of course. Flooding the log was not the intention of the patch.
> I had two ideas on this, one is use a BugOpts option that can turn on
> these warnings for a user who wants to debug things, but they are off
> by default. This is basically your style idea and the affect will be
> almost everywhere these warnings will not appear and thus may miss out
> on programs that actually need to be reported.
Yes, but defaulting to "off" makes the warning effectively useless.
> Another is maybe don't have fvwm jump to conclusions that there is a
> warning.
Let me rephrase that: Fvwm informs the user about something that
might not be possible to clean up. In such a situation the user
would see that the window did not get resized as intended and has
no clue why. At least, fvwm has told her that something strange
was going on.
> What if there was some timer that fvwm gave the application
> to fix itself before reporting a warning, and then the warning is not
> just that the window had this inconstant state which seems to give
> false positives, but it has been inconsistent for a set amount of time
> without fixing itself.
Too complex stuff for such a simple situation. Maybe one could
peek the event queue for an event that fixes the windows's size
and not complain if such an event is already pending? Another
option is to generate only a limited number of these warning for
each window.
Does the attached patch improve the situation?
> Anyways, for those of us who use FvwmIconMan as a growing/shrinking
> standalone module, this patch drastically reduces the amount of
> warnings, but I agree it really doesn't seem like the way to deal with
> this issue to make applications have to add these waits to deal with
> resizing both the window and the size hints.
Ciao
Dominik ^_^ ^_^
--
Dominik Vogt
>From 8f74a2e6f1f0e059e9d1e073bace15a33dd2c016 Mon Sep 17 00:00:00 2001
From: Dominik Vogt
Date: Wed, 1 Mar 2017 17:24:13 +0100
Subject: [PATCH] Try to fix FvwmIconMan warnings.
---
fvwm/events.c |9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/fvwm/events.c b/fvwm/events.c
index 980ccab..a7d41d0 100644
--- a/fvwm/events.c
+++ b/fvwm/events.c
@@ -3627,6 +3627,10 @@ ICON_DBG((stderr, "hpn: icon changed '%s'\n", fw->name.name));
break;
}
case XA_WM_NORMAL_HINTS:
+ {
+ int do_check;
+ XEvent dummy;
+
/* just mark wm normal hints as changed and look them up when
* the next ConfigureRequest w/ x, y, width or height set
* arrives. */
@@ -3641,8 +3645,11 @@ ICON_DBG((stderr, "hpn: icon changed '%s'\n", fw->name.name));
* Note that SET_HAS_NEW_WM_NORMAL_HINTS being set here to
* true is still valid.
*/
- GetWindowSizeHintsWithCheck(fw, 1);
+ do_check = !FCheckTypedWindowEvent(
+ dpy, FW_W(fw), ConfigureRequest, &dummy);
+ GetWindowSizeHintsWithCheck(fw, do_check);
break;
+ }
default:
if (te->xproperty.atom == _XA_WM_PROTOCOLS)
{
--
1.7.10.4
Re: FvwmIconMan still sometimes triggers Hint Warnings
On Wed, Mar 1, 2017 at 5:30 AM, Dominik Vogt wrote: > On Tue, Feb 28, 2017 at 10:22:33PM -0700, Jaimos Skriletz wrote: >> On Mon, Feb 27, 2017 at 11:10 PM, Jaimos Skriletz >> wrote: >> > Hello, >> > >> > FvwmIconMan still reports Hint warnings even with the patchs applied. >> >> I wrote this small patch that has FvwmIconMan wait until the window >> has resized to set the window hints. This is in the lines of Dominik >> Vogt's patch, but waits until FvwmIconMan has fully resized to run >> fix_manager_size() to set the window hints. My tests here no longer >> seem to tiger the warnings like it still sometimes did. >> >> As an additional thought, talking to Thomas Adam about the patch in >> #fvwm, he mentioned that the issue is more FVWM being overly cautious >> and the fix should be in how fvwm handles size hint warnings over >> working with FvwmIconMan to avoid triggering them. >> > > So, if it's not possible to completely fix FvwmIconMan in a decent > way, what do we do with the warning? I'd rather not disable it, > but the number of false positives is too high. One could make a > style that disables the warning for a certain class of windows, > but that would probably be used as "style * ...", disabling the > warning for everything. :-/ > FvwmIconMan isn't the only application that triggers these warnings, but using it in a situation where it grows/shrinks is a very regular way to cause the warnings and it fills up .xsession-error with mostly warnings. Other applications I use only trigger these warnings rarely, and in each case the application appears to work fine so yet more false positives. But FvwmIconMan is the only one that regularly resizes itself triggering the warning a lot. I had two ideas on this, one is use a BugOpts option that can turn on these warnings for a user who wants to debug things, but they are off by default. This is basically your style idea and the affect will be almost everywhere these warnings will not appear and thus may miss out on programs that actually need to be reported. Another is maybe don't have fvwm jump to conclusions that there is a warning. What if there was some timer that fvwm gave the application to fix itself before reporting a warning, and then the warning is not just that the window had this inconstant state which seems to give false positives, but it has been inconsistent for a set amount of time without fixing itself. Anyways, for those of us who use FvwmIconMan as a growing/shrinking standalone module, this patch drastically reduces the amount of warnings, but I agree it really doesn't seem like the way to deal with this issue to make applications have to add these waits to deal with resizing both the window and the size hints. jaimos
Re: FvwmIconMan still sometimes triggers Hint Warnings
On Tue, Feb 28, 2017 at 10:22:33PM -0700, Jaimos Skriletz wrote:
> On Mon, Feb 27, 2017 at 11:10 PM, Jaimos Skriletz
> wrote:
> > Hello,
> >
> > FvwmIconMan still reports Hint warnings even with the patchs applied.
>
> I wrote this small patch that has FvwmIconMan wait until the window
> has resized to set the window hints. This is in the lines of Dominik
> Vogt's patch, but waits until FvwmIconMan has fully resized to run
> fix_manager_size() to set the window hints. My tests here no longer
> seem to tiger the warnings like it still sometimes did.
>
> As an additional thought, talking to Thomas Adam about the patch in
> #fvwm, he mentioned that the issue is more FVWM being overly cautious
> and the fix should be in how fvwm handles size hint warnings over
> working with FvwmIconMan to avoid triggering them.
>
> I don't know enough about the issue to say which is more appropriate,
> but here is a patch that stops the warnings from being triggered in my
> tests.
Changing window geometry and hints has inherent race conditions by
design of the X protocol, unless every change is synchronized with
the server, and even then it's impossible to avoid some
inconsistencies. For example, there is no way to change the
window size and the requested min/max size in one atomic action.
Even more, there are lots of applications that set invalid hints
or in an ambigouos way. The window manager has to guess the
apllication's intention in such cases. A while ago I've started
adding more warnings to Fvwm in order to better identify such
situations, but this specific one has too many false positives.
Patching the application to wait for something happening is
definitely the wrong approach, not just because it is inefficient
but because there is no guarantee that the window manager ever
honours such requests.
So, if it's not possible to completely fix FvwmIconMan in a decent
way, what do we do with the warning? I'd rather not disable it,
but the number of false positives is too high. One could make a
style that disables the warning for a certain class of windows,
but that would probably be used as "style * ...", disabling the
warning for everything. :-/
> From 227d7ea2597ec3fec304c53934fcc41773ab7e89 Mon Sep 17 00:00:00 2001
> From: Jaimos Skriletz
> Date: Tue, 28 Feb 2017 17:43:12 -0700
> Subject: [PATCH 1/1] Wait until FvwmIconMan is resized to set window HINTS
>
> ---
> modules/FvwmIconMan/xmanager.c | 10 ++
> 1 file changed, 10 insertions(+)
>
> diff --git a/modules/FvwmIconMan/xmanager.c b/modules/FvwmIconMan/xmanager.c
> index 58eaaedc..b4efe890 100644
> --- a/modules/FvwmIconMan/xmanager.c
> +++ b/modules/FvwmIconMan/xmanager.c
> @@ -439,6 +439,16 @@ static void resize_window(WinManager *man)
> }
> MyXUngrabServer(theDisplay);
>}
> +
> + // Wait until the window has resised to fix the HINTS.
> + // counter is used to break an infinte loop.
> + XWindowAttributes attribs;
> + int counter = 2;
> + while ( counter && (attribs.width != man->geometry.width ||
> + attribs.height != man->geometry.height)) {
> +XGetWindowAttributes(theDisplay, man->theWindow, &attribs);
> +counter--;
> + }
>fix_manager_size(man, man->geometry.width, man->geometry.height);
> }
Ciao
Dominik ^_^ ^_^
--
Dominik Vogt
Re: FvwmIconMan still sometimes triggers Hint Warnings
On Mon, Feb 27, 2017 at 11:10 PM, Jaimos Skriletz
wrote:
> Hello,
>
> FvwmIconMan still reports Hint warnings even with the patchs applied.
>
I wrote this small patch that has FvwmIconMan wait until the window
has resized to set the window hints. This is in the lines of Dominik
Vogt's patch, but waits until FvwmIconMan has fully resized to run
fix_manager_size() to set the window hints. My tests here no longer
seem to tiger the warnings like it still sometimes did.
As an additional thought, talking to Thomas Adam about the patch in
#fvwm, he mentioned that the issue is more FVWM being overly cautious
and the fix should be in how fvwm handles size hint warnings over
working with FvwmIconMan to avoid triggering them.
I don't know enough about the issue to say which is more appropriate,
but here is a patch that stops the warnings from being triggered in my
tests.
jaimos
From 227d7ea2597ec3fec304c53934fcc41773ab7e89 Mon Sep 17 00:00:00 2001
From: Jaimos Skriletz
Date: Tue, 28 Feb 2017 17:43:12 -0700
Subject: [PATCH 1/1] Wait until FvwmIconMan is resized to set window HINTS
---
modules/FvwmIconMan/xmanager.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/modules/FvwmIconMan/xmanager.c b/modules/FvwmIconMan/xmanager.c
index 58eaaedc..b4efe890 100644
--- a/modules/FvwmIconMan/xmanager.c
+++ b/modules/FvwmIconMan/xmanager.c
@@ -439,6 +439,16 @@ static void resize_window(WinManager *man)
}
MyXUngrabServer(theDisplay);
}
+
+ // Wait until the window has resised to fix the HINTS.
+ // counter is used to break an infinte loop.
+ XWindowAttributes attribs;
+ int counter = 2;
+ while ( counter && (attribs.width != man->geometry.width ||
+ attribs.height != man->geometry.height)) {
+XGetWindowAttributes(theDisplay, man->theWindow, &attribs);
+counter--;
+ }
fix_manager_size(man, man->geometry.width, man->geometry.height);
}
--
2.11.0
