Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Conversation

jsorg71
Copy link
Contributor

@jsorg71 jsorg71 commented Feb 4, 2024

No description provided.

@Nexarian
Copy link
Contributor

Nexarian commented Feb 5, 2024

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:

  • Log in with multi-mon
  • Disconnect
  • Log in with single monitor and optionally resize it.
  • Reconnect with multi-mon
  • Multi-mon is now broken. It only remembers the single monitor from the last session. The rest are blank.

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.

@Nexarian Nexarian merged commit e17da50 into neutrinolabs:gfx_mainline_merge_work Feb 5, 2024
8 checks passed
@jsorg71 jsorg71 deleted the gfx_mainline_merge_no_reset branch February 5, 2024 06:36
@matt335672
Copy link
Member

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
Copy link
Contributor

Nexarian commented Feb 5, 2024

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
Copy link
Member

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
Copy link
Contributor

Nexarian commented Feb 6, 2024

@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
Copy link
Member

I agree with @Nexarian.

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

  • We can't move to the more streamlined GFX resizing without also changing the way that we perform server-requested resizes (i.e. using xrandr to change the monitor size). This needs the resize state machine and the server rest logic to be re-engineered.
  • I don't think we handle all server-side cases correctly. For example, rejecting a single-monitor resize on a multi-monitor setup.

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:-

  • Knocking up (or finding) a simple RandR monitor so we can check the X server is generating the correct events. I suspect it is, and the issues I've seen above are down to GNOME not handling multiple simultaneous monitor changes.
  • Writing up the state of resize so we have a place to document user issues like the above.

@matt335672
Copy link
Member

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
Copy link
Member

@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
Copy link
Contributor Author

jsorg71 commented Feb 6, 2024

@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
Copy link
Member

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants