public inbox for cygwin@cygwin.com
 help / color / mirror / Atom feed
From: "S.J. Luo" <sjaluo@gmail.com>
To: cygwin@cygwin.com
Subject: Re: Window flickering problem in XWin multiwindow mode
Date: Sat, 7 May 2022 10:01:32 +0800	[thread overview]
Message-ID: <CAMoHPCaD+wx98n3QmaehE+xdiuOe6OC1kQsEPLm156HS9r4xzA@mail.gmail.com> (raw)
In-Reply-To: <CAMoHPCZ18Q_G=JTwOwu2ij3feJ6cd9CHxddONuProrL3kwgBKw@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 5423 bytes --]

Hi,

> > > I have some EDA tools running on a Linux machine and display on my Windows
> > > PC using xorg-server-21.1.3 XWin multiwindow mode
> > > Sometimes the application window flickers forever for an unknown reason.
> > > The problem became more severe after my PC upgrade to Windows10.
> > >
> > > Knowing the root cause, I am now able to demonstrate the issue with a small
> > > test case as well as a patch that works for me. Both are attached below.
> >
> > Thanks very much for taking the time to look into this, and writing a patch.
> > But I'd like to add a bit more commentary, which perhaps you can supply,
> > about the analysis you did to determine:
> >
> > 1. how the code is misbehaving
> > 2. how this change remedies that
> > 3. how this change doesn't effect anything else
> >
> > What your fix does is examine the queue of pending WM messages to
> > determine if there are any more pending for the window receiving a
> > WM_WM_CHANGE_STATE message, and if there are, ignore it.
> >
> > It seems to me that is error prone, in a couple of ways:
> >
> > 1. it should check the message type as well, as e.g. a pending message
> > of a different type could cause a WM_WM_CHANGE_STATE message to be ignored.
> >
> > 2. even then, assuming that successive WM_WM_CHANGE_STATE messages
> > cancel out each other isn't always true e.g. consider a sequence of
> > _NET_WM_STATE_ADD _NET_WM_STATE_MAXIMIZED_VERT followed by
> > _NET_WM_STATE_ADD _NET_WM_STATE_MAXIMIZED_HORZ.
> >

> Here I have some details on the problem:
> In following, WWCS means WM_WM_CHANGE_STATE
> There exists a message self-generating loop
> I.e., a WWCS message may generate another WWCS.
>
> winSendMessageToWM(WWCS) --> MultiWindowWMProc(WWCS)
> --> UpdateState() --> ShowWindow() --> MultiWindowProc(WM_SIZE)
> --> winAdjustXWindowState() --> winSendMessageToWM(WWCS)
>
> In normal case, there is a conditional code inside UpdateState() that
> could break the loop.
>
>             if (current_state == state)
>                 return;
>
> Normally after maximizing the window, the WWCS just
> to be self-generated once. On processing the 2nd WWCS,
> the current window state matches WWCS target state
> and ShowWindow() won't be called and the loop could end.
>
> Initial
>     Window state normal
>     queue : | WWCS(max1) |
> --> process WWCS(max1) :
>     Winow state (norm) != target state (max2)
>     Set Window state to max and generate WWCS(max2)
>     queue : | WWCS(max2) |
> --> Process WWCS(max2) :
>     Winow state (max) == target state (max)
>     break
>
> where WWCS(max1) means WWCS msg with target state max
> and 1 means first generated/inserted WWCS in queue.
>
> However, there is case triggering this loop running forever.
> Assuming intitially there are two WWCS message exists
> where one is to maximize window and one is to normal window
> The condition of state compare in UpdateState() would never match.
> And WWCS(max) and WWCS(norm) is going to self-generate in turn.
>
> Initial
>     Window state normal
>     queue : | WWCS(max1) | WWCS(norm1) |
> --> process WWCS(max1)
>     Winow state (norm) != target state (max)
>     Set window state to max and generate WWCS(max2)
>     queue : | WWCS(norm1) | WWCS(max2) |
> --> Process WWCS(norm1)
>     Winow state (max) != target state (norm)
>     Set window state to norm and generate WWCS(norm2)
>     queue : | WWCS(max2) | WWCS(norm2) |
> --> Process WWCS(max2)
>     Winow state (norm) != target state (max)
>     Set window state to max and generate WWCS(max3)
>     queue : | WWCS(norm2) | WWCS(max3) |
> --> Process WWCS(norm2)
>     Winow state (max) != target state (norm)
>     Set window state to norm and generate WWCS(norm3)
>     queue : | WWCS(max3) | WWCS(norm3) |
> :
> (loop forever)
>

I just did some more study on the code of multiwindow window manager and
X11 Window Properties as well as the questions you brought up.

The first question :

> it should check the message type as well, as e.g.
> a different type mesage could cause WM_WM_CHANGE_STYLE to be ignored.

Not sure what the "type" of a message means.
Do you mean window manager message WM_xxxxx that is processed by
winMultiWindowWMProc() where it is alreadychecked in HaveMessage(),
or do you mean X message XCB_xxxxxx that is processed by
winMultiWindowXMsgProc()?
Maybe you can provide an example of another message type?

The second question:

> Successive WM_WM_CHANGE_STATE messages cancel out each other
> isn't always true ex. _NET_WM_STATE_ADD _NET_WM_STATE_MAXIMIZED_VERT
> followed by _NET_WM_STATE_ADD _NET_WM_STATE_MAXIMIZED_HORZ.

Inspecting the code that deal with XCB_CLIENT_MESSAGE, it seems already
true before the patch: The window goes maximized only if X client do one
_WM_STATE_ADD action with _NET_WM_STATE_MAXIMIZED_VERT and
_NET_WM_STATE_MAXIMIZED_HORZ simultaneously specified. Otherwise,
the WM_WM_CHANGE would not be sent to the queue.
Further, in UpdateWindowState(), the state and related X properties are
either all-overwritten or all-skipped.
There seems to be no state property half-update case if we skip
UpdateWindowState().

In anyway, I just came out somewhat different patch attached.
Where it just skips the Win32 ShowWindow() call instead of
skipping processing WM_WM_CHANGE_STATE. It might be better because
the X properties are still always updated in each in WM_WM_CHANGE_STATE.
Maybe you could provide further comment.

SL

[-- Attachment #2: flicker2b.patch --]
[-- Type: application/x-patch, Size: 4634 bytes --]

  parent reply	other threads:[~2022-05-07  2:01 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-26  3:53 S.J. Luo
2022-04-30  1:40 ` S.J. Luo
2022-04-30  2:06   ` Mark Geisert
2022-04-30  3:58     ` Mark Geisert
2022-04-30 15:25 ` Jon Turney
     [not found]   ` <CAMoHPCZ18Q_G=JTwOwu2ij3feJ6cd9CHxddONuProrL3kwgBKw@mail.gmail.com>
2022-05-07  2:01     ` S.J. Luo [this message]
2022-08-14 11:08       ` Jon Turney

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAMoHPCaD+wx98n3QmaehE+xdiuOe6OC1kQsEPLm156HS9r4xzA@mail.gmail.com \
    --to=sjaluo@gmail.com \
    --cc=cygwin@cygwin.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).