redraw windows while resizing/moving windows in multiwindow mode

redraw windows while resizing/moving windows in multiwindow mode

[Oliver Schmidt]

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

Hi,

the following patch is a quick & dirty hack to enable redrawing of 
windows while the user moves or resizes the window.

This patch should be seen as experimental proof that this can be done 
with only small changes in the source code.

The main problem with window resizing/moving under Windows is, that in 
the function invocation stack dix/Dispatch -> os/WaitForSomething -> 
WakeupHandler -> hw/xwin/winWakeupHandler -> Windows/DispatchMessage -> 
hw/xwin/winTopLevelWindowProc the Windows function DispatchMessage does 
not return while the user resizes/moves a window. This function only 
returns after the user releases the mouse button. However the 
dix/Dispatch functions must be run to allow the clients to process the 
queued redraw/resizing events.

The enclosed hack simply moves the invocation of DispatchMessage in 
winWakeupHandler outside WaitForSomething into Dispatch and breaks up 
the Dispatch function into a loop and inner dispatch handling that can 
be called from the winTopLevelWindowProc while WM_SIZE/WM_MOVE events 
are processed. This could further be improved by setting a windows timer 
while resizing moving to process clients messages even if the mouse is 
not moved while resizing (and therefore WM_SIZE/WM_MOVE events are not 
send).

What do you think about this idea? One problem here is, that the dix 
package is also affected. Of course some work must be done to cleanly 
integrate this into the existing dix/hw architecture.

Best regards,
Oliver

[-- Attachment #2: patch05.txt --]
[-- Type: text/plain, Size: 4475 bytes --]

diff --git a/cygwin/dix/dispatch.c b/cygwin/dix/dispatch.c
index 44f8087..18e9697 100644
--- a/cygwin/dix/dispatch.c
+++ b/cygwin/dix/dispatch.c
@@ -343,33 +343,27 @@ DisableLimitedSchedulingLatency(void)
 
 #define MAJOROP ((xReq *)client->requestBuffer)->reqType
 
-void
-Dispatch(void)
-{
-    int        *clientReady;     /* array of request ready clients */
-    int	result;
-    ClientPtr	client;
-    int	nready;
-    HWEventQueuePtr* icheck = checkForInput;
-    long			start_tick;
+static     int        *clientReady;     /* array of request ready clients */
+static     int	result;
+static     ClientPtr	client;
+static     int	nready;
+static     HWEventQueuePtr* icheck = checkForInput;
+static     long			start_tick;
 
-    nextFreeClientID = 1;
-    nClients = 0;
-
-    clientReady = malloc(sizeof(int) * MaxClients);
-    if (!clientReady)
-	return;
-
-    SmartScheduleSlice = SmartScheduleInterval;
-    while (!dispatchException)
-    {
+int DispatchOneStep(Bool handleWindowMessage)
+{
+    int rslt = 0;
+    
         if (*icheck[0] != *icheck[1])
 	{
 	    ProcessInputEvents();
 	    FlushIfCriticalOutputPending();
 	}
-
 	nready = WaitForSomething(clientReady);
+	rslt = nready;
+
+        if (handleWindowMessage)
+            handleNextWindowMessage();
 
 	if (nready && !SmartScheduleDisable)
 	{
@@ -460,6 +454,24 @@ Dispatch(void)
 		client->smart_stop_tick = SmartScheduleTime;
 	}
 	dispatchException &= ~DE_PRIORITYCHANGE;
+
+    return rslt;
+}
+
+void
+Dispatch(void)
+{
+    nextFreeClientID = 1;
+    nClients = 0;
+
+    clientReady = malloc(sizeof(int) * MaxClients);
+    if (!clientReady)
+	return;
+
+    SmartScheduleSlice = SmartScheduleInterval;
+    while (!dispatchException)
+    {
+        DispatchOneStep(TRUE);
     }
 
     if (ddxHooks.ddxBeforeReset)
diff --git a/cygwin/hw/xwin/winmultiwindowwndproc.c b/cygwin/hw/xwin/winmultiwindowwndproc.c
index bd84c05..265fdcc 100644
--- a/cygwin/hw/xwin/winmultiwindowwndproc.c
+++ b/cygwin/hw/xwin/winmultiwindowwndproc.c
@@ -321,6 +321,7 @@ winTopLevelWindowProc (HWND hwnd, UINT message,
   static Bool		s_fTracking = FALSE;
   Bool			needRestack = FALSE;
   LRESULT		ret;
+  static Bool           hasEnteredSizeMove = FALSE;
 
 #if CYGDEBUG
   winDebugWin32Message("winTopLevelWindowProc", hwnd, message, wParam, lParam);
@@ -871,7 +872,15 @@ winTopLevelWindowProc (HWND hwnd, UINT message,
 
     case WM_MOVE:
       /* Adjust the X Window to the moved Windows window */
-      winAdjustXWindow (pWin, hwnd);
+      if (!hasEnteredSizeMove) 
+        {
+          winAdjustXWindow (pWin, hwnd);
+        }
+      else
+        {
+          winAdjustXWindow (pWin, hwnd);
+          while (DispatchOneStep(FALSE) > 0) {}
+        }
       return 0;
 
     case WM_SHOWWINDOW:
@@ -1012,6 +1021,16 @@ winTopLevelWindowProc (HWND hwnd, UINT message,
       */
       break; 
 
+    case WM_ENTERSIZEMOVE:
+      hasEnteredSizeMove = TRUE;
+      return 0;
+
+    case WM_EXITSIZEMOVE:
+      /* Adjust the X Window to the moved Windows window */
+      hasEnteredSizeMove = FALSE;
+      winAdjustXWindow (pWin, hwnd);
+      return 0;
+
     case WM_SIZE:
       /* see dix/window.c */
 #if CYGWINDOWING_DEBUG
@@ -1036,9 +1055,17 @@ winTopLevelWindowProc (HWND hwnd, UINT message,
 		(int)(GetTickCount ()));
       }
 #endif
-      /* Adjust the X Window to the moved Windows window */
-      winAdjustXWindow (pWin, hwnd);
-      if (wParam == SIZE_MINIMIZED) winReorderWindowsMultiWindow();
+      if (!hasEnteredSizeMove)
+        {
+          /* Adjust the X Window to the moved Windows window */
+          winAdjustXWindow (pWin, hwnd);
+          if (wParam == SIZE_MINIMIZED) winReorderWindowsMultiWindow();
+        }
+      else
+        {
+          winAdjustXWindow (pWin, hwnd);
+          while (DispatchOneStep(FALSE) > 0) {}
+        }
       return 0; /* end of WM_SIZE handler */
 
     case WM_STYLECHANGING:
diff --git a/cygwin/hw/xwin/winwakeup.c b/cygwin/hw/xwin/winwakeup.c
index 031a510..6eab76a 100644
--- a/cygwin/hw/xwin/winwakeup.c
+++ b/cygwin/hw/xwin/winwakeup.c
@@ -43,6 +43,14 @@ winWakeupHandler (int nScreen,
 		  unsigned long ulResult,
 		  pointer pReadmask)
 {
+    /* was: handleNextWindowMessage, but
+            this will block in WaitForSomething when
+            moving resizing windows in multiwindow 
+            mode. */
+}
+
+void handleNextWindowMessage(void)
+{
   MSG			msg;
 
   /* Process one message from our queue */


[-- Attachment #3: Type: text/plain, Size: 223 bytes --]

--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Problem reports:       http://cygwin.com/problems.html
Documentation:         http://x.cygwin.com/docs/
FAQ:                   http://x.cygwin.com/docs/faq/

Re: redraw windows while resizing/moving windows in multiwindow mode

[Jon TURNEY]

On 20/08/2011 23:41, Oliver Schmidt wrote:
> the following patch is a quick & dirty hack to enable redrawing of windows
> while the user moves or resizes the window.
>
> This patch should be seen as experimental proof that this can be done with
> only small changes in the source code.

This is fine as a proof of concept, and it would be nice to handle this better 
and do X window updates during the Windows modal resizing loop, but I don't 
think I can apply this patch.

> The main problem with window resizing/moving under Windows is, that in the
> function invocation stack dix/Dispatch -> os/WaitForSomething -> WakeupHandler
> -> hw/xwin/winWakeupHandler -> Windows/DispatchMessage ->
> hw/xwin/winTopLevelWindowProc the Windows function DispatchMessage does not
> return while the user resizes/moves a window. This function only returns after
> the user releases the mouse button. However the dix/Dispatch functions must be
> run to allow the clients to process the queued redraw/resizing events.

Well, in fact, no X events are processed while in the modal resizing loop, so 
for e.g. if you have ico running in another X window, it stops updating while 
an xterm window is being resized.

Note that there are other modal loops in Windows message handling, I think 
moving the scrollbar also involves one (which can happen in windowed mode with 
the -scrollbar option)

> The enclosed hack simply moves the invocation of DispatchMessage in
> winWakeupHandler outside WaitForSomething into Dispatch and breaks up the
> Dispatch function into a loop and inner dispatch handling that can be called
> from the winTopLevelWindowProc while WM_SIZE/WM_MOVE events are processed.
> This could further be improved by setting a windows timer while resizing
> moving to process clients messages even if the mouse is not moved while
> resizing (and therefore WM_SIZE/WM_MOVE events are not send).
>
> What do you think about this idea? One problem here is, that the dix package
> is also affected. Of course some work must be done to cleanly integrate this
> into the existing dix/hw architecture.

I'm not sure how to structure the change to Dispatch() in a way which would be 
acceptable upstream.

An additional point to consider is that you may have introduced the 
possibility of re-entrancy into either the X window message dispatcher, or the 
Windows message dispatcher, which may not be safe. (e.g. winTopLevelProc -> 
DispatchOneStep -> (some X event processing calls a DDX function which calls 
SendMessage) -> winTopLevelProc ...)

An alternative approach would be to move the Windows message pump into a 
separate thread from the X server message dispatch.  Unfortunately, this would 
probably involve rather major changes, and careful examination of all the 
places where the window drawing code accesses internal server state to see if 
locking was needed. (I think Xquartz is structured more like that)

Alternatively, it might be worth investigating to see if it is possible to use 
a message filter set with SetWindowsHookEx(WH_MSGFILTER) to run the X window 
dispatch while Windows is in a modal loop?

-- 
Jon TURNEY
Volunteer Cygwin/X X Server maintainer

--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Problem reports:       http://cygwin.com/problems.html
Documentation:         http://x.cygwin.com/docs/
FAQ:                   http://x.cygwin.com/docs/faq/

Re: redraw windows while resizing/moving windows in multiwindow mode

[Oliver Schmidt]

Hi Jon,

On 9/7/2011 5:05 PM, Jon TURNEY wrote:
> This is fine as a proof of concept, and it would be nice to handle this

did you try the patch? It looks & feels very smooth if you resize a
xlock and the xclock and all x11 background windows are redrawn while
resizing ;-)

> better and do X window updates during the Windows modal resizing loop,
> but I don't think I can apply this patch.

but I hope this patch can be used as a starting point.

> Well, in fact, no X events are processed while in the modal resizing
> loop, so for e.g. if you have ico running in another X window, it stops
> updating while an xterm window is being resized.

with the patch X events are processed. With the patch, ico redraws also
while windows are moved or resized, as long as the mouse is moved. For
display updating without moving the mouse while modal resizing/moving is
in progress, I already mentioned the timer event that is possible to
improve the patch.

Thanks for mentioning "ico", I didn't know this program, it is an
interesting experimental tool: it shows that the patch is too
aggressive, i.e. the ui interface is not responsive, perhaps due to my
"critical" code fragment:

	while (DispatchOneStep(FALSE) > 0) {}

So I will try now to improve the patch for better responsiveness.

> Note that there are other modal loops in Windows message handling, I
> think moving the scrollbar also involves one (which can happen in
> windowed mode with the -scrollbar option)

One could introduce a similar patch there too ;-) However a patch for
scrollbar option in windowed mode is not as reasonable as in multiwindow
mode, because the static redrawing of the x server makes sense in
windowed mode. Only in multiwindow mode the redrawing is strange, e.g.
if you applied my patch "minimize redraw events after resizing/moving
windows in multiwindow mode", you will see other X11 background windows
while "resizing" a x11 foreground window in the window that is resized,
because actually the x11 window is not resized due to missing x11 event
processing, but the xserver simply redraws all x11 windows in the
current size. In windowed mode, no x11 window is resized.

> I'm not sure how to structure the change to Dispatch() in a way which
> would be acceptable upstream.

I hoped, you had an idea. What are the criteria to be accepted upstream?
At least the patch introduces only a "bypass", i.e. existing code/usage
is not affected. It would be discouraging if no upstream changes are
possible to improve the cygwin x server's multi window mode, since this
is the mode that allows the highest integration of x11 applications with
native windows programs. If no upstream changes are possible one
fallback could be to have a local patch (or #ifdef switch) for the
cygwin x server.


> An additional point to consider is that you may have introduced the
> possibility of re-entrancy into either the X window message dispatcher,
> or the Windows message dispatcher, which may not be safe. (e.g.
> winTopLevelProc -> DispatchOneStep -> (some X event processing calls a
> DDX function which calls SendMessage) -> winTopLevelProc ...)

Could you explaind this more explicitly? How can this be tested?

As I understood the code, the function "Dispatch" is only called once
per x server invocation. And the new function "DispatchOneStep" is meant
to be processed re-entrant. This is why the boolean parameter
handleWindowMessage is introduced and why I had to remove the invocation
of DispatchMessage out of the winWakeupHandler which is called in
WaitForSomething.

> An alternative approach would be to move the Windows message pump into a
> separate thread from the X server message dispatch.  Unfortunately, this
> would probably involve rather major changes, and careful examination of

I agree that this would cause a lot of more work than the approach of my
patch. I'm not sure if moving the Windows message handling into a
different thread will solve the problem totally: there still is the
problem, that in the modal windows event loop the x11 event processing
must be invoked. At least one has to totally decouple the x11 and
Windows event processing, but then still in the modal event loop the now
decoupled x11 processing must be triggered. So it seems to me, that
decoupling the x11 and Windows processing does only prevent upstream
changes but does not solve the problem, that in the modal Windows event
loop small progressing parts for X11 (coupled or decoupled) have to be done.

> Alternatively, it might be worth investigating to see if it is possible
> to use a message filter set with SetWindowsHookEx(WH_MSGFILTER) to run
> the X window dispatch while Windows is in a modal loop?

I'm not sure if I'm understanding this approach correctly, but I think
even with SetWindowsHookEx we still have the problem, that the main loop
in Dispatch has to be broken into smaller parts that can be invoked from
inside the modal Windows event loop (or hook).

Best regards,
Oliver


--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Problem reports:       http://cygwin.com/problems.html
Documentation:         http://x.cygwin.com/docs/
FAQ:                   http://x.cygwin.com/docs/faq/

Re: redraw windows while resizing/moving windows in multiwindow mode

[Jon TURNEY]

On 08/09/2011 14:09, Oliver Schmidt wrote:
> On 9/7/2011 5:05 PM, Jon TURNEY wrote:
>> This is fine as a proof of concept, and it would be nice to handle this
>
> did you try the patch? It looks&  feels very smooth if you resize a
> xlock and the xclock and all x11 background windows are redrawn while
> resizing ;-)
>
>> better and do X window updates during the Windows modal resizing loop,
>> but I don't think I can apply this patch.
>
> but I hope this patch can be used as a starting point.
>
>> Well, in fact, no X events are processed while in the modal resizing
>> loop, so for e.g. if you have ico running in another X window, it stops
>> updating while an xterm window is being resized.
>
> with the patch X events are processed. With the patch, ico redraws also
> while windows are moved or resized, as long as the mouse is moved. For
> display updating without moving the mouse while modal resizing/moving is
> in progress, I already mentioned the timer event that is possible to
> improve the patch.
>
> Thanks for mentioning "ico", I didn't know this program, it is an
> interesting experimental tool: it shows that the patch is too
> aggressive, i.e. the ui interface is not responsive, perhaps due to my
> "critical" code fragment:
>
> 	while (DispatchOneStep(FALSE)>  0) {}
>
> So I will try now to improve the patch for better responsiveness.
>
>> Note that there are other modal loops in Windows message handling, I
>> think moving the scrollbar also involves one (which can happen in
>> windowed mode with the -scrollbar option)
>
> One could introduce a similar patch there too ;-) However a patch for
> scrollbar option in windowed mode is not as reasonable as in multiwindow
> mode, because the static redrawing of the x server makes sense in
> windowed mode.

I'm not sure I follow your reasoning here.  If we have 'ico' running in 
windowed mode, why should it stop updating while the scrollbars are being moved?

> Only in multiwindow mode the redrawing is strange, e.g.
> if you applied my patch "minimize redraw events after resizing/moving
> windows in multiwindow mode", you will see other X11 background windows
> while "resizing" a x11 foreground window in the window that is resized,
> because actually the x11 window is not resized due to missing x11 event
> processing, but the xserver simply redraws all x11 windows in the
> current size. In windowed mode, no x11 window is resized.
>
>> I'm not sure how to structure the change to Dispatch() in a way which
>> would be acceptable upstream.
>
> I hoped, you had an idea. What are the criteria to be accepted upstream?
> At least the patch introduces only a "bypass", i.e. existing code/usage
> is not affected. It would be discouraging if no upstream changes are
> possible to improve the cygwin x server's multi window mode, since this
> is the mode that allows the highest integration of x11 applications with
> native windows programs. If no upstream changes are possible one
> fallback could be to have a local patch (or #ifdef switch) for the
> cygwin x server.
>
>
>> An additional point to consider is that you may have introduced the
>> possibility of re-entrancy into either the X window message dispatcher,
>> or the Windows message dispatcher, which may not be safe. (e.g.
>> winTopLevelProc ->  DispatchOneStep ->  (some X event processing calls a
>> DDX function which calls SendMessage) ->  winTopLevelProc ...)
>
> Could you explaind this more explicitly? How can this be tested?

> As I understood the code, the function "Dispatch" is only called once
> per x server invocation. And the new function "DispatchOneStep" is meant
> to be processed re-entrant.

I don't see how you can guarantee that all the X server code which 
DispatchOneStep() might call is re-entrant?

I cannot rid myself the suspicion this may happen (since there are now code 
paths winTopLevelProc->DispatchOneStep and DispatchOneStep->winTopLevelProc)

> This is why the boolean parameter
> handleWindowMessage is introduced and why I had to remove the invocation
> of DispatchMessage out of the winWakeupHandler which is called in
> WaitForSomething.

>> An alternative approach would be to move the Windows message pump into a
>> separate thread from the X server message dispatch.  Unfortunately, this
>> would probably involve rather major changes, and careful examination of
>
> I agree that this would cause a lot of more work than the approach of my
> patch. I'm not sure if moving the Windows message handling into a
> different thread will solve the problem totally: there still is the
> problem, that in the modal windows event loop the x11 event processing
> must be invoked. At least one has to totally decouple the x11 and
> Windows event processing, but then still in the modal event loop the now
> decoupled x11 processing must be triggered. So it seems to me, that
> decoupling the x11 and Windows processing does only prevent upstream
> changes but does not solve the problem, that in the modal Windows event
> loop small progressing parts for X11 (coupled or decoupled) have to be done.
>
>> Alternatively, it might be worth investigating to see if it is possible
>> to use a message filter set with SetWindowsHookEx(WH_MSGFILTER) to run
>> the X window dispatch while Windows is in a modal loop?
>
> I'm not sure if I'm understanding this approach correctly, but I think
> even with SetWindowsHookEx we still have the problem, that the main loop
> in Dispatch has to be broken into smaller parts that can be invoked from
> inside the modal Windows event loop (or hook).

Yes, I think you are correct.  A hook has the advantage that X event dispatch 
will continue to occur during all the modal event loops that Windows has, 
rather than just the ones we have noticed :-)

-- 
Jon TURNEY
Volunteer Cygwin/X X Server maintainer

--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Problem reports:       http://cygwin.com/problems.html
Documentation:         http://x.cygwin.com/docs/
FAQ:                   http://x.cygwin.com/docs/faq/