Subject: Re: [X2Go-Dev] Turn xinerama.conf file related code inside X2Go Client into legacy code (for NX agents << 3.6)

From: Mike Gabriel <[email-masked]>
Date: Mon, 18 May 2015 08:40:55 +0000

Hi Alex,
(re-adding #872 to Cc:, dropping x2go-dev instead)

(my answers in-line at end of mail text)

On  So 17 Mai 2015 08:45:57 CEST, Oleksandr Shneyder wrote:

> Am 15.05.2015 um 13:59 schrieb Mike Gabriel:
>> Package: x2goclient
>> Severity: important
>> Version: 4.0.4.0
>>
>> Hi Uli, hi Alex,
>>
>> I turn this discussion into a bug report against X2Go Client.
>>
>> Short summary for Alex: Ulrich has completely revisited [2] Xinerama in
>> nx-libs recently for the 3.6.x branch of nx-libs [1].
>>
>> The complete Xinerama code has been rewritten and moved mainly into the
>> hw/nxagent/Screen.c file, so that we can soon drop your patched
>> libNX_Xinerama.so library and all that LD_LIBRARY_PATH stuff from
>> x2goruncommand.
>>
>> With Ulrichs work, the hacky part in X2Go Client of writing and
>> continuously updating a xinerama.conf file server-side from the
>> client-side becomes obsolete and should only be kepts as legacy code for
>> cases where X2Go Client talks to nxagent servers << 3.6.
>>
>> I'd also appreciate it if you could give feedback on the below question.
>>
>> Thanks,
>> Mike
>>
>> [1] https://github.com/ArcticaProject/nx-libs
>> [2] https://github.com/ArcticaProject/nx-libs/pull/23
>>
>> On  Do 14 Mai 2015 23:32:03 CEST, Ulrich Sibiller wrote:
>>
>>> while testing my xinerama re-implementation I noticed that x2goclient
>>> reports a wrong width sometimes when simply moving the window without
>>> changing the size. The reported size differs by one pixel  (too much
>>> or too little). When running x2goclient --debug you can see this:
>>>
>>> x2go-DEBUG-../src/onmainwindow.cpp:10160> "New proxy geometry: (x:
>>> 329, y: 214, w: 1440, h: 900"
>>> x2go-DEBUG-../src/onmainwindow.cpp:10160> "New proxy geometry: (x:
>>> 365, y: 213, w: 1440, h: 900"
>>> x2go-DEBUG-../src/onmainwindow.cpp:10160> "New proxy geometry: (x:
>>> 825, y: 190, w: 1440, h: 900"
>>> x2go-DEBUG-../src/sshprocess.cpp:189> executing remote command via
>>> SshProcess object (24): "export DISPLAY=:50;printf '\''1095 0 345
>>> 900\n0 0 1095 900'\'' >
>>> $HOME/.x2go/C-uli-50-1431636769_stDKDE_dp24/xinerama.conf"
>>>
>>> -> here the 345 should have been a 344.
>>>
>>> Interestingly my patched x2goagent (that completely ignores
>>> xinerama.conf) sees that value. So I conclude that x2goclient is
>>> manipulating the nxproxy window size by one pixel itself. Disabling
>>> xinerama in the session's configuration fixes the problem.
>>>
>>> That off-by-one behaviour is caused by this code in onmainwindow.cpp:
>>>
>>> void ONMainWindow::slotXineramaConfigured()
>>> {
>>>     if (resumingSession.fullscreen)
>>>         return;
>>>     if (xinSizeInc == -1)
>>>         xinSizeInc=1;
>>>     else
>>>         xinSizeInc=-1;
>>> #ifdef Q_OS_LINUX
>>>     lastDisplayGeometry.setWidth(lastDisplayGeometry.width()+xinSizeInc);
>>>     XSync(QX11Info::display(),false);
>>>     XResizeWindow(QX11Info::display(), proxyWinId,
>>>
>>> lastDisplayGeometry.width(),lastDisplayGeometry.height());
>>>     XSync(QX11Info::display(),false);
>>> #endif
>>> #ifdef Q_OS_WIN
>>>     QRect geom;
>>>     wapiWindowRect ( (HWND) proxyWinId, geom );
>>>     wapiMoveWindow( (HWND) proxyWinId, geom.x(), geom.y(),
>>> geom.width()+xinSizeInc, geom.height(),true);
>>>     lastDisplayGeometry=proxyWinGeometry();
>>> #endif
>>>     xineramaTimer->start(500);
>>> }
>>>
>>> If I set xinSizeInc to zero in all cases the off-by-one behaviour is
>>> completely gone. Why does xinSizeInc exist at all?

> It's a great news. Do we need to make a changes in the X2Go Client for

X2Go Client needs to test the remote x2goagent version and check  
whether it is << 3.5.99.0 or >= 3.5.99.0. If it is >= 3.5.99.0, the  
new Xinerama support will be present (once we have released 3.5.99.0,  
at the moment the code has not been merged). If the new Xinerama code  
is present, then there is no need for the xinerama.conf server-side  
writing/updating anymore.

> this? I think it should be sufficient to make changes in x2goruncommand.

I think that too, see X2GoBTS #873 [1].

> Should user still have possibility to turn Xinerama off? I can still

This is a good question. I don't think that we should include the  
xinerama.conf part in nxagent anymore, but a way for switching off the  
Xinerama extension should be possible, indeed.

The current plan is to make the cmdline options +xinerama / -xinerama  
(or -extension Xinerama) functional in nxagent. With all previous  
versions of nxagent, this did not have any effect, so far. Ulrich is  
working on that.

> imagine the use case when user have an x2goagent window in the middle of
> two physical monitors but want to see agent window as one display.

Indeed.

> regards
> Alex

Mike

[1] https://bugs.x2go.org/cgi-bin/bugreport.cgi?bug=873
-- 

DAS-NETZWERKTEAM
mike gabriel, herweg 7, 24357 fleckeby
fon: +49 (1520) 1976 148

GnuPG Key ID 0x25771B31
mail: [email-masked], http://das-netzwerkteam.de

freeBusy:
https://mail.das-netzwerkteam.de/freebusy/m.gabriel%40das-netzwerkteam.de.xfb