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

Downscaled shadow monitor window contains "background garbage" #2903

Closed
totaam opened this issue Oct 16, 2020 · 19 comments
Closed

Downscaled shadow monitor window contains "background garbage" #2903

totaam opened this issue Oct 16, 2020 · 19 comments
Assignees
Milestone

Comments

@totaam
Copy link
Collaborator

totaam commented Oct 16, 2020

Issue migrated from trac ticket # 2903

component: client | priority: minor

2020-10-16 10:00:45: stdedos created the issue


27676

"Xpra-Python3-x86_64_4.1-27676\xpra_cmd" shadow ssh://user@ip/0 --ssh="plink -ssh -agent" -d ssh --env=XPRA_SHADOW_REFRESH_DELAY=200 --title="@title@ on @@/@server-display@" --opengl=no --bandwidth-limit=6Mbps --desktop-scaling=0.75 --webcam=no --speaker=off --microphone=off --pulseaudio=no

args=--ssh="plink -ssh -agent" -d ssh --env=XPRA_SHADOW_REFRESH_DELAY=200 --title="@title@ on @@/@server-display@" --opengl=no --bandwidth-limit=6Mbps --desktop-scaling=0.75 --webcam=no --speaker=off --microphone=off --pulseaudio=no
XPRA_CUSTOM_TITLE_BAR=0
XPRA_EXECUTABLE=Xpra-Python3-x86_64_4.1-27676
XPRA_NETWORK_ADAPTER_TYPE=wifi
XPRA_SCROLL_ENCODING=0

2020-10-16 11:56:31,540 parse_ssh_string(plink -ssh -agent)
2020-10-16 11:56:32,526 Xpra GTK3 client version 4.1-27676 64-bit
2020-10-16 11:56:32,528  running on Microsoft Windows 10
2020-10-16 11:56:33,534 GStreamer version 1.18.0 for Python 3.8.6 64-bit
2020-10-16 11:56:33,869 created named pipe 'Xpra\11276'
2020-10-16 11:56:34,182 keyboard layout code 0x409
2020-10-16 11:56:34,183 identified as 'United States - English' : us
2020-10-16 11:56:34,368 Warning: libqrencode not found
2020-10-16 11:56:34,463  keyboard settings: layout=us
2020-10-16 11:56:34,466  desktop size is 4160x1440 with 1 screen:
2020-10-16 11:56:34,467   Default (1100x381 mm - DPI: 96x96) workarea: 4160x1400
2020-10-16 11:56:34,468     Generic PnP Monitor 1600x900 at 0x534 (309x174 mm - DPI: 131x131) workarea: 1600x860 at 0x534
2020-10-16 11:56:34,468     C32JG5x 2560x1440 at 1600x0 (697x392 mm - DPI: 93x93) workarea: 2560x1400 at 1600x0
2020-10-16 11:56:34,469  downscaled to 75%, virtual screen size: 5547x1920
2020-10-16 11:56:34,470   Default (1100x381 mm - DPI: 128x128) workarea: 5547x1867
2020-10-16 11:56:34,470     Generic PnP Monitor 2133x1200 at 0x712 (309x174 mm - DPI: 175x175) workarea: 2133x1147 at 0x712
2020-10-16 11:56:34,471     C32JG5x 3413x1920 at 2133x0 (697x392 mm - DPI: 124x124) workarea: 3413x1867 at 2133x0
2020-10-16 11:56:55,444 enabled remote logging
2020-10-16 11:56:55,451 Xpra GTK3 shadow server version 3.0.12-[r27620](../commit/7eac9a9d283d837589e80430ee4d1c2f49470a3f) 64-bit
2020-10-16 11:56:55,456  running on Linux Ubuntu 16.04 xenial
2020-10-16 11:56:55,459  remote desktop size is 6400x1440

(xpra_cmd:11276): Pango-WARNING **: 11:56:55.937: couldn't load font "Bitstream Vera Sans Not-Rotated 14.662109375", falling back to "Sans Not-Rotated 14.662109375", expect ugly output.
2020-10-16 11:56:56,969 UI thread is now blocked
2020-10-16 11:56:56,987 UI thread is running again, resuming
2020-10-16 11:57:16,752 server is not responding, drawing spinners over the windows
2020-10-16 11:57:17,342 server is OK again

@totaam
Copy link
Collaborator Author

totaam commented Oct 16, 2020

2020-10-16 12:39:38: stdedos uploaded file Xpra_cmd_2020-10-16_11-57-34.png (1979.3 KiB)

Xpra_cmd_2020-10-16_11-57-34.png

@totaam totaam added this to the 4.2 milestone Jan 23, 2021
@stdedos
Copy link
Collaborator

stdedos commented Feb 19, 2021

More sights for sore eyes:

Xpra_cmd_2021-01-25_10-56-20

Apart from cherry-picking the (2) windows to be shown, and the whitening of Evolution, the rest is as-seen.

@stdedos
Copy link
Collaborator

stdedos commented May 4, 2021

SHOCKING NEWS 😱

I found a solution to hiding that tasteless gray area that works like a broken xerox.
It is known, that shadow windows cannot be resized except when maximized (PLS don't "fix" that behavior, #3007).

If I use the scaleup shortcut, the scaled down window will now fill the whole area (original viewport + gray area).
Then, if I use the scaledown shortcut, all of a sudden, the viewport (and window) scales down nicely!

There may be an initialization error (or race condition?) on the code that controls the viewport and the shadow window size.

I would also be interested on "what on earth" is that gray area, and why does it "work" like that (i.e. capturing the last pixels that were projected on it).

With scaling started at 75%, it goes to 100% and then down to 67%.
Geometry: 1279x719 with base-size: 1920x1080

@totaam
Copy link
Collaborator Author

totaam commented May 4, 2021

There may be an initialization error (or race condition?) on the code that controls the viewport and the shadow window size.

-d scaling should tell us what it is happening.

I would also be interested on "what on earth" is that gray area, and why does it "work" like that (i.e. capturing the last pixels that were projected on it).

The padding area is meant to be re-painted with either white or black, depending on whether the window is transparent or not.
You may get different results by toggling opengl on or off.

@stdedos
Copy link
Collaborator

stdedos commented May 4, 2021

OpenGL is always off because Intel 4000 HD (can't find the exact name, no open xpra window mentions it anywhere)

@stdedos
Copy link
Collaborator

stdedos commented May 4, 2021

@totaam
Copy link
Collaborator Author

totaam commented May 4, 2021

OpenGL is always off because Intel 4000 HD

FYI: --opengl=on or --opengl=force can override that. Behaviour for the "grey" area may reveal something.
The full driver info can be seen with xpra opengl.


First thing of interest in the log, lots of:

Warning: failed to setup video pipeline (59, (2, 3), (2, 3), 1920, 1342, swscale(BGRX to NV12), 'NV12', (1, 1), 1280, 894, ffmpeg(NV12 to h264))
(..)
Exception: could not open h264 encoder context: Not yet implemented in FFmpeg, patches welcome

This means that xpra tried to use a video encoder, failed and then used a picture codec instead. This is a waste of time and makes the server re-evaluate all the possible video pipelines for every single screen update! (only to fail because the best one is the same.. the failing one)

So the version of ffmpeg in Ubuntu Focal has libva support, but not one we can actually use for acceleration.. oh joy.
They ship ffmpeg 4.2.4, which is not very old.
From now on, d18b0f1 + e5f9f52 will require 4.4 or later to try to enable libva hardware acceleration, because I am too lazy to figure out which exact versions actually can work and this latest one definitely does.


Then I noticed the screen updates are getting downscaled, but not by the same amount used on the client side:

calculate_scaling(1920, 1080, 4096, 4096)=(1, 2) (q=98, s=56, scaling_control=None)

a437a9e fixes that - it will now be a multiple of whatever scaling value the client is using.


Is this to force authentication again when an xpra client connects?

bash -c xpra id "${DISPLAY}" | grep session-type= | cut -d = -f 2 | grep -qP "shadow" || exit 0 ; \
    env DBUS_SESSION_BUS_ADDRESS="$(cut -f 2- -d= "${XDG_RUNTIME_DIR}/dbus-session")" \
    gnome-screensaver -l

I assume that this a --start-on-connect?
Neat, we should make this a bit easier to use: #3110


Finally, wrt to desktop-scaling proper, from your log:

  • the initial value is 0.75
  • then it switches to 2/3
  • then it switches to 1 (no scaling)
  • then to 2/3 again

Are you saying that going through these specific values does something that --desktop-scaling=2/3 does not do?

@stdedos
Copy link
Collaborator

stdedos commented May 4, 2021

The padding area is meant to be re-painted with either white or black, depending on whether the window is transparent or not.

Yes but, at that state, it's neither. Bug or feaure? 😕
It's also somewhat annoying on gnome-terminal: Nowadays, it cannot scale perfectly on the maximized window, because of size-hint constraints (because of the CSS that Gnome is using nowadays i.e. Focal; not because of XPRA).

Can you forcefully limit the application at the constraints it requests, and then explicitly fill the rest with a color? black? use the majority of the color that touches the borders (i.e. for gnome-terminal and Ubuntu: purple)?


FYI: --opengl=on or --opengl=force can override that. Behaviour for the "grey" area may reveal something.
The full driver info can be seen with xpra opengl.

I have it --opengl=off by default, exactly because I have noticed xpra crashes on Windows that seem to be mitigated by that (at least on seamless mode). If you want me to probe something specifically on this mode, let me know.

So the version of ffmpeg in Ubuntu Focal has libva support, but not one we can actually use for acceleration.. oh joy.

I'll try to get a ffmpeg ppa installed with >=4.3, and see what happens.

Are you saying that going through these specific values does something that --desktop-scaling=2/3 does not do?

I didn't try to start with 2/3, because I wanted 75% 😛
Using 75% does not scale the window nicely (I will get the size hints tomorrow); switching to 2/3 does.
I don't know if the problem is caused by the non-standard scaling percent or if it is fixed by the scaleup/scaledown.

@totaam
Copy link
Collaborator Author

totaam commented May 5, 2021

Yes but, at that state, it's neither. Bug or feaure? confused

Bug.

Can you forcefully limit the application at the constraints it requests,

Window geometry is hard.
Maybe we should lie to the application about what size was actually allocated to it and pretend that we did honour the size-constraints it had requested.
Then we would avoid all sorts of resizing issues where the application is trying to force the window size to match what it wants. But this is bound to have unwanted side effects..

and then explicitly fill the rest with a color? black?

That is exactly what is meant to happen already.

use the majority of the color that touches the borders (i.e. for gnome-terminal and Ubuntu: purple)?

It is doable with OpenGL and GL_CLAMP_TO_EDGE: https://open.gl/textures
Though it will look weird when the edge is not a single solid colour.
And I don't think it can be done with the cairo backend without stretching all the edges manually.

I'll try to get a ffmpeg ppa installed with >=4.3, and see what happens.

Don't. That may break things further. (reminder: 4.4 is required, 4.3 won't work)

@stdedos
Copy link
Collaborator

stdedos commented May 12, 2021

Don't. That may break things further. (reminder: 4.4 is required, 4.3 won't work)

Solid advice as, the only available PPA (https://launchpad.net/~savoury1/+archive/ubuntu/ffmpeg4) managed to block vp[89] codecs from being used

     24 vpx: vp8 encoding failed: failed to instantiate vp8 encoder with ABI version 23: Invalid parameter
     25 vpx: vp9 encoding failed: failed to instantiate vp9 encoder with ABI version 23: Invalid parameter
     26 vpx: all the codecs have failed! (vp8, vp9)

and managed to silently crash the xpra server of my main session (no logs, no cores, no nothing) continuously.

Luckily, not the one of my side session.

@stdedos
Copy link
Collaborator

stdedos commented May 14, 2021

Yes but, at that state, it's neither. Bug or feaure? confused

Bug.

It seems to me that only the down / right borders create "erroneous situations".
If the top / left borders are padded, black appears correctly:
Xpra_cmd_2021-05-14_12-23-52

@totaam
Copy link
Collaborator Author

totaam commented May 14, 2021

I think I have seen the problem.
I believe that upscaling an xterm can lead to a similar situation, where the pixel backing area covers the whole window instead of using padding.

windows.1.geometry=(0, 19, 1915, 1057)
windows.1.client-geometry=(0, 19, 1920, 1062)
2021-05-14 22:13:32,662 source image surface: (cairo.Format.RGB24, 1915, 1057, 7660, cairo.Content.COLOR)
2021-05-14 22:13:32,662 cairo_paint_surface(<function CairoBackingBase.cairo_paint_surface.<locals>.set_source_surface at 0x7f43fc8a9a60>, <cairo.ImageSurface object at 0x7f43fe2e8a10>, 0, 0, 1915, 1057, 1915, 1057, {b'rgb_format': 'BGRX', b'window-size': (1915, 1057), 'encoding': 'mmap'}) backing=<cairo.ImageSurface object at 0x7f43cc0243f0>, paint box line width=0
2021-05-14 22:13:32,666 paint_backing_offset_border() size=(width=3840, height=2123), offsets=(0, 0, 0, 0)
2021-05-14 22:13:32,666 clip_to_backing(gtk3.CairoBacking(ImageSurface(1920, 1062) : size=(1920, 1062), render_size=(3840, 2123)), <cairo.Context object at 0x7f43fe2e8a10>) rectangle=(0, 0, 3840, 2123)
2021-05-14 22:13:32,666 cairo_draw: backing=<cairo.ImageSurface object at 0x7f43cc0243f0>, size=(1920, 1062), render-size=(3840, 2123), offsets=(0, 0, 0, 0), pointer_overlay=None
2021-05-14 22:13:32,666 render-size=(3840, 2123), size=(1920, 1062)

So the "padding" ends up being pixels in the backing we never populate since they do not exist on the server side.
Normally, this isn't a problem because we initialize the backing with zeroes (cairo's OPERATOR_CLEAR), but when we change the scaling then we may initialize it with the previous contents, which may have pixels there. And they will stay there, forever.

@totaam totaam self-assigned this May 14, 2021
@stdedos
Copy link
Collaborator

stdedos commented May 14, 2021

Sounds like memzero is what it should be done before/instead memcpy 😕

totaam added a commit that referenced this issue May 15, 2021
totaam added a commit that referenced this issue May 15, 2021
totaam added a commit that referenced this issue May 15, 2021
@totaam
Copy link
Collaborator Author

totaam commented May 15, 2021

Sounds like memzero is what it should be done before/instead memcpy

That was already the case, as I said:

Normally, this isn't a problem because we initialize the backing with zeroes (cairo's OPERATOR_CLEAR)

The problem actually came from how cairo copies the old backing to the new resized one. The commit above fixes that.
The other commit makes it easier to see what the backing copy does, by allowing us to skip it with XPRA_CAIRO_COPY_OLD_BACKING=0.

I'm still not 100% happy with how GTK sends us so many window resize events for just a single maximization... but whatever.

@stdedos
Copy link
Collaborator

stdedos commented May 15, 2021

That was already the case, as I said:

Sorry about that 😕 I was thinking pure C rather than an API above it.

I'm still not 100% happy with how GTK sends us so many window resize events for just a single maximization... but whatever.

I don't know how that translates in-code; however, I think I literally see it live - and it's so annoying.
I also don't know if it was always like that, or some library changed it, my OS upgrade causes this, or both.

@totaam totaam closed this as completed May 17, 2021
@stdedos
Copy link
Collaborator

stdedos commented May 27, 2021

The garbage has disappeared 😀

@stdedos
Copy link
Collaborator

stdedos commented Jul 28, 2021

Are you saying that going through these specific values does something that --desktop-scaling=2/3 does not do?

It seems that, after all, the problem I was focusing with this seems to be with initialization: whether scaling is at 2/3 or 0.75, the screen window is initialized by xpra as if there's no scaling at all.

Tinkering with scaling (and/or passing through a non-scaling state) seems to alleviate the aforementioned problem.

@totaam
Copy link
Collaborator Author

totaam commented Nov 1, 2021

This fix may (finally) explain this bug: 28cd217.

@stdedos
Copy link
Collaborator

stdedos commented Nov 1, 2021

I hope so. We will never know 🙂

I think (part of) this was #3140 - which is now resolved 😀

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

No branches or pull requests

2 participants