neutrinolabs / xorgxrdp

Xorg drivers for xrdp
Other
428 stars 108 forks source link

GRX: no need to reset or map as xrdp takes care of it #282

Closed jsorg71 closed 5 months ago

Nexarian commented 5 months ago

While I believe this is fine, we still have a bug around the scenario I outlined earlier. It might only happen with 3+ monitors though:

I stared at this for a bit and couldn't figure out why. It seems the monitor configuration is fine and stored correctly. Xorgxrdp seems to be adding/removing/updating monitors properly, etc. I suspect something needs to be reset/cleared that isn't.

matt335672 commented 4 months ago

I can't reproduce @Nexarian's problem with two monitors.

1) Log in to xrdp using mstsc.exe with two monitors. This works OK:-

   $ xrandr
   Screen 0: minimum 256 x 256, current 3200 x 1200, maximum 16384 x 16384
   rdp0 connected primary 1920x1200+0+0 0mm x 0mm
      1920x1200     50.00* 
   rdp1 connected 1280x1024+1920+0 0mm x 0mm

2) Disconnect and log in using xfreerdp /gfx /v:<host> /u:<user> /d: /p: /dynamic-resolution. Screen is not redrawn correctly. If I refresh it though, everything looks OK. Then:-

   $ xrandr
   Screen 0: minimum 256 x 256, current 1024 x 768, maximum 16384 x 16384
   rdp0 connected primary 1024x768+0+0 0mm x 0mm

3) Disconnect and reconnect with mstsc.exe. I'm back to two screens.

I can reproduce something similar using just mstsc.exe. In my case, when I went back to two screens, the primary monitor was redrawn, but the secondary monitor wasn't - it stayed blank. However, xrandr showed two screens, and moving an application on to the second screen resulted in it being redrawn.

I'm going to try to get to the bottom of the corruption I'm seeing at the moment. @Nexarian - could this be what you are seeing?

Nexarian commented 4 months ago

This is very close to what I'm seeing. However, in my case if I move an application to the second screen the screen is not redrawn. The app is redrawn -- sorta, but not erased (similar to that memory effect that used to happen with Windows 95/98 when explorer would crash), and it's not always the right resolution either.

matt335672 commented 4 months ago

I've spent quite a few hours today looking at this, and while I've got a few observations, I'm not near a solution. I'd welcome any feedback on the following:-

First things. Is this your experience, or is yours different? 1) This is nothing to do with the resize state machine as we're talking about resize-on-connect which currently doesn't use that. 2) I don't think this is GFX-related. I've used bpp=24 to disable GFX and I'm seeing the same thing. 3) It also happens with the VNC backend, although (subjectively) less often.

Other things:- 1) The way I generally see it is to start with two screens, disconnect, then connect with a single screen of a different resolution. This generally gives me something like the following:- NoRedraw 2) Having done this, xrefresh doesn't cause a correct window redraw, even with backing store disabled on the X server. This seems very odd. 3) On the other hand, if I start another application, everything seems to work OK. 4) I've not managed to replicate this with XFCE so far - only GNOME.

Nexarian commented 4 months ago

@matt335672 I may have sent you on a wild goose chase. I can't seem to reproduce the problematic bug anymore. What you mention above are bugs that have been around in XRDP for some time. We should investigate, but I don't think this blocks merging.

@jsorg71 @metalefty I think we're probably good to merge this if you are.

matt335672 commented 4 months ago

I agree with @Nexarian.

There's more that needs to be done on resizing. Off the top of my head:-

On the other hand, what we've got is I think robust enough for a first release.

I'll get on with the following, unless anyone else has got something more useful for me to do. This shouldn't hold up any release activities:-

matt335672 commented 4 months ago

One more question, possibly for @jsorg71

When we move to the 1 monitor scenario from the 3 monitor scenario (say), we delete the outputs rather than just marking them disconnected, which is what VNC does.

VNC appears to handle this better than xorgxrdp (although this is completely subjective). Is there a good why we delete the outputs rather than simply disconnecting them? Is it worth looking into this approach?

metalefty commented 4 months ago

@Nexarian @jsorg71 @matt335672

Thank you guys. I have tested the latest changeset I also think it's good enough to merge. Let's get back to the master issue.

jsorg71 commented 4 months ago

@matt335672 You are right about leaving the outputs disconnected. Originally it was always deleting the outputs and then adding the outputs needed. This was crashing some randr clients. Now it changes existing outputs, then removing extras. It would probably be even reliable to never delete them.

matt335672 commented 4 months ago

@jsorg71 - thanks.

One other observation I can add to this is that running xev -root -event randr against xorgxrdp can fail with BadRROutput when an output get deleted. It's possible to patch xev to cope with this, but a better approach might be for it not to fail in the first place.

Suggest we leave the GFX stream as it is for now for v0.10 and I'll ready a patch for devel when it resumes. We can always backport it to v0.10 if necessary.