[X2Go-Dev] improve Xinerama

[Mike Gabriel]

Hi Ulrich,

On Fri, May 01, 2015 at 01:13:01PM +0200, Ulrich Sibiller wrote:
> On Thu, Apr 30, 2015 at 5:18 AM, Mike Gabriel
> <mike.gabriel at das-netzwerkteam.de> wrote:
> >> However, I am still unsure about the correct solution here. Some
> >> questions that arise:
> >> - do we need a switch to disable that lookup? Why is xinerama disabled
> >> by default when configuring new x2go sessions?
> >
> > In nxagent, there is +xinerama and -xinerama as cmdline option (or {-|+}
> > extension Xinerama). IMHO these typical XServer options should be usable for
> > nxagent. Is that possible?
> 
> I had already tried them, but they seem to be broken currently. This
> is probably because the XINERAMA extension is provided via the randr
> extension in the current code. If you run nxagent -extension RANDR
> then XINERAMA will be disabled, too.

Ok... interesting...

> > The reasons for Xinerama being off by default in X2Go Client:
> >
> >   o when new features get added, the default is the previous situation
> >     until the new feature is tested well enough
> 
> Good strategy.
> 
> >   o old servers may not have Xinerama support, yet (at the time of
> >     releasing the feature in X2Go Client)
> 
> In this case the we take that as xinerama configuration with a single
> large screen. The nxagent will always know about xinerama.

If Xinerama support in nx-libs 3.6.x can be handled without any config
option, at all, then we should enable it always and by default.

Maybe it is possible to make +xinerama / -xinerama functional again, so
that nxagent has the option to disable xinerama (via xrandr).

> >   o Xinerama in X2Go has been buggy again and again (due to packaging
> >     issues in the various distros that nx-libs is packaged for; due to
> >     bugs in X2Go Client when creating xinerama.conf)
> 
> Yes, I have noticed that. And I doubt that my changes will be perfect
> on the first shot...

Oh, don't worry about that!
 
> > IMHO, Xinerama enabled should be the default for nx-libs 3.6.x. Esp. if it
> > does not require a xinerama.conf file anymore (i.e., an interaction by the
> > remote session client).
> 
> When X11 did the transition from 6.9 to 7.0 they did not add a single
> feature. Maybe the even did omit bugfixes. The only difference between
> those two is the source code structure and the build process. So I
> think all those cleanup things you are doing should not be mixed with
> code changes, do you?

We are actuall mixing code cleanup work, CVE hunting, bug fixes, build
fixes, etc. at the moment.

I try to avoid bringing in new features, but maybe I will even start working
on getting XComposite available and fixed in nx-libs, soon.

> >> - do we need a notification from the nxproxy when the xinerama setup
> >> changes during connection?
> >
> > Yes. In general, the geometry of an X2Go desktop session window (and thus
> > the "Xinerama borders" inside) can change.
> 
> I have seen x2goclient already has that feature when I was doing the
> test you describe below. And those tests reveal that I need to
> intersect the client geometry with the xinerama information. For this
> to work I need to know the position of the nxproxy window (not the
> size, I think, RANDR has that information) . My code is currently
> located in the nxagent. How can I find out the position of the nxproxy
> window from the server side?

Phew... good question. The way it is done is this:

  o X2Go Client detects the X window ID of the nxproxy instance
  o X2Go Client queries the client-side Xserver for the window position
    of nxproxy (by using the exact X window ID)
  o X2Go Client does the calculation internally (intersecting nxproxy
    position and size with client-side Xserver's Xinerama borders

I'd say this requires a patch in nxproxy, so that it knows its position
all by itself. Or maybe even let nxproxy do the intersecting???
 
> Apart from that I think the client side code for updating that
> information should move from x2goclient to nxproxy. This way all
> products using NX will benefit.

/me nods on that! Great!!!

> > Use cases that should work:
> >
> >   o rootless session mode (single applications, running in nxagent,
> >     being nxproxied to the client should be aware of the client-side
> >     Xserver's Xinerama settings), the nxagent session should be notified
> >     by geometry changes of the client-side Xserver
> 
> I think in rootless mode the nxagent should always cover the whole
> desktop. I am not sure if this is valid currently, but it really
> should.

I think, it does this.

> If that is the case this test should succeed. I will check
> again.

Ok.

> >     Question: should application windows be de-maximized on geometry
> >     changes? Or re-maximized? Or ...?
> 
> This is something the window manager within the session must handle.
> All window managers supporting randr should already be aware of
> geometry changes. In rootless mode the client side window manager will
> take care of that.

Ok. For my i3 tiling window manager I can say that it works with rootless
NX/X2Go sessions. I guess, it will work with other WMs, too.

> >   o fullscreen desktop session mode (the Xinerama settings of the
> >     client-side Xserver should be available inside the fullscreen
> >     desktop session running in nxagent), the nxagent session should
> >     be notified by geometry changes of the client-side Xserver
> 
> Fullscreen working currently, since I fetch that information directly
> from the client-side X server on every request .

Ok. Cool.

> >     Note: as you don't have access to the client-side Xserver controls
> >     (except from an outside shell with xrandr cmdline tool), geometry
> >     changes in this use case are probably really rare.
> 
> On my Thinkpad I can press Fn+F7 to cycle through several screen
> configurations which will change the number of screen and therefore
> will influence the xinerama setup. So this is quite normal.

Ah, right. Ok...

> >   o windowed desktop session mode (tricky, see below)
> >
> > The most tricky part may be nxagent's windowed desktop sessions. This little
> 
> Yes, that case is tricky as it needs intersection. And here I have the
> mentioned problem: I need to know the position of the nxproxy window
> on the client-side X server

See above.

> >> - xinerama is integrated into xrandr (in
> >> programs/Xserver/randr/rrxinerama.c; the file I mentioned yesterday is
> >> compiled and included, but not used apparently). Does hooking in there
> >> produce any side effects? Currently clients asking nxagent for
> >> xinerama info get the values from the real server but there are
> >> internal structures that are not updated.
> >
> > Not sure about this without looking into the code any deeper. Basically,
> > NoMachine also patched libXrandr heavily in an NX. Partially for backporting
> > xrandr 1.2 proto version (IIRC), but possible also for some NX specific
> 
> BTW: I think we really need at least xrandr 1.3. to be able to add
> resolutions on the fly.

Ok. I can take a look at that.

> > stuff (haven't looked any close so far). So maybe the xrandr implementation
> > in programs/Xserver/randr also depends on the patched libNX_Xrandr.
> 
> Hmm, I haven't looked at the patched lib yet. The patched randr server
> code mainly does one think: replace dixLookupWindow() by
> SecurityLookupWindow().

Ok...

> But in general I would expect the libraries are NOT providing server
> functions put are provided for clients to use them. Isn't that the
> case? My local X server here on ubuntu 14.04 opens some X libraries
> but not libXinerama or libXrandr:
> 
> /usr/lib/x86_64-linux-gnu/libX11.so.6.3.0
> /usr/lib/x86_64-linux-gnu/libX11-xcb.so.1.0.0
> /usr/lib/x86_64-linux-gnu/libXau.so.6.0.0
> /usr/lib/x86_64-linux-gnu/libXdamage.so.1.1.0
> /usr/lib/x86_64-linux-gnu/libXdmcp.so.6.0.0
> /usr/lib/x86_64-linux-gnu/libXext.so.6.4.0
> /usr/lib/x86_64-linux-gnu/libXfixes.so.3.1.0
> /usr/lib/x86_64-linux-gnu/libXfont.so.1.4.1
> /usr/lib/x86_64-linux-gnu/libXxf86vm.so.1.0.0

At build time, dpkg-shlibdeps (or what's-its-name) indeed reports about
unnecessarily linked libraries for nxagent. IIRC, libNX_Xinerama and
libNX_Xrandr were among them.

Let me check that later. It would mean that we can drop libNX_Xrandr
immediately and that we only have to update the Xserver code itself.

> >> - is there a need to keep the current solution with LD_LIBRARY_PATH ?
> >> (I'd say no)
> >
> > No, it was only used for enabling Xinerama support in X2Go sessions. If
> > nxagent can be compiled against libXinerama from X.Org and the Xinerama
> > magic resided in programs/Xserver/**, then setting LD_LIBRARY_PATH can be
> > dropped completely from X2Go/NX.
> 
> OK.
> 
> I think until that problem with the client window position is solved I
> will implement the halfway solution that still refers to
> $NX_XINERAMA_CONFIG but will work without the LD_LIBRARY_PATH hack.

Maybe it would be good to keep both approaches in place for legacy support.

If a xinerama.conf file is present use that. If not, try out your new code.

Greets,
Mike


-- 

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

GnuPG Key ID 0x25771B31
mail: mike.gabriel at das-netzwerkteam.de, http://das-netzwerkteam.de

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.x2go.org/pipermail/x2go-dev/attachments/20150501/29ef4e2b/attachment.pgp>