[CinCV TNG] [PATCH] Rewrite of BC_DisplayInfo::test_window

Johannes Sixt j6t at kdbg.org
Sat Apr 25 22:16:06 CEST 2015


Am 25.04.2015 um 19:48 schrieb Einar R√ľnkaru:
> 
> Hi, Hannes
> 
> On 04/25/2015 05:18 PM, Johannes Sixt wrote:
>> Am 24.04.2015 um 22:56 schrieb Johannes Sixt:
> 
>>>
>>> I tested this new patch, and it does not work *at all*. I use the KDE
>>> that comes with openSuSE 13.2. The window manager should therefore be
>>> KWM.
>>>
>>> The old code works flawlessly. The windows are never offset by a single
>>> pixel.
> 
> Old code assumes WM does not have bar on top or left. When there is bar, 
> the computed values are slightly wrong. Then as Yaroslav reported old 
> test hangs when WM for some reason decides to resisze the test window.
>>
>> It looks like most of the time the ConfigureNotify event is not
>> received. This leaves xm and ym at zero, minus TEST_X and TEST_Y at the
>> end of the function computes an offset of -100.
> 
> Ok. The check that the result offset is reasonable should be added.
>>
>> I say "most of the time" because test_window() is now called many times
>> since the test 'top_border < 0' in init_borders() keeps calling
>> test_window() many times. At one point, the ConfigureNotify is received,
>> but that happens so late in the game that all windows are already
>> shifted by 100 pixels.
>>
>> Perhaps the calls to XMoveResizeWindow() and XResizeWindow() that you
>> removed should remain?
> 
> Can you increment the usleep value? Or add sleep(5) after it. May be the 
> test_window just should give some more time to WM. The question is will 
> we wait too few or for some reason ConfigureNotify is not sent.

Increasing the wait time to 0.1sec helps indeed. But I dislike that
there is a dependency on the timing. Are we guaranteed to receive a
ConfigureNotify event? At least for me the following on top of your
patch helps:

diff --git a/guicast/bcdisplayinfo.C b/guicast/bcdisplayinfo.C
index 10400df..101ce48 100644
--- a/guicast/bcdisplayinfo.C
+++ b/guicast/bcdisplayinfo.C
@@ -101,23 +101,20 @@ void BC_DisplayInfo::test_window(int &x_out,
 
 	XMapWindow(display, win); 
 	XFlush(display);
 	XSync(display, 0);
-	// Wait until WM reacts
-	usleep(10000);
 
 	int xm = 0, ym = 0;
 	XEvent event;
 
-	while(XPending(display))
+	for (;;)
 	{
 		XNextEvent(display, &event);
 		if(event.type == ConfigureNotify && event.xconfigure.window == win)
 		{
-			if(xm < event.xconfigure.x)
-				xm = event.xconfigure.x;
-			if(ym < event.xconfigure.y)
-				ym = event.xconfigure.y;
+			xm = event.xconfigure.x;
+			ym = event.xconfigure.y;
+			break;
 		}
 	}
 
 	XDestroyWindow(display, win);

Should we handle more than one ConfigureNotify event?

-- Hannes



More information about the Cinelerra mailing list