-
Notifications
You must be signed in to change notification settings - Fork 1k
Do not request screen updates while the window is not visible #1940
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
base: master
Are you sure you want to change the base?
Conversation
9ab40bd
to
1c4fc47
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems reasonable. Is this an issue you've observed in practice?
writer()->writeFramebufferUpdateRequest({0, 0, | ||
server.width(), | ||
server.height()}, | ||
!forceNonincremental); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if there already is a pending update request?
writer()->writeFramebufferUpdateRequest({0, 0, | ||
server.width(), | ||
server.height()}, | ||
!forceNonincremental); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this needed if we're using continuous updates?
writer()->writeFramebufferUpdateRequest({0, 0, | ||
server.width(), | ||
server.height()}, | ||
!forceNonincremental); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you're going to look at forceNonincremental
, then you need to reset it as well.
vncviewer/parameters.cxx
Outdated
core::BoolParameter | ||
updateOnlyWhenVisible("UpdateOnlyWhenVisible", | ||
"Do not request screen updates while the window is " | ||
"not visible, e.g. minimized.", | ||
false); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really need to add another setting for this? Why can't it be constantly be enabled?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do all VNC servers tolerate not sending updates for extended periods of time? If yes and this can be always enabled, then I fully agree that this option should be removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would hope, since they don't know how fast a client might be able to respond. All servers I know of track the coordinates of things that are changed, rather than the changed pixels themselves. So there shouldn't be any issue of things overbuffering.
I'm running several instances of the TigerVNC viewer, and each of them uses 10%-15% CPU when there are a lot of updates on the screen. |
New option: UpdateOnlyWhenVisible
1c4fc47
to
9d1329c
Compare
I'm a bit surprised that you're getting Any idea if we can get some hint for the compositing desktops as well? E.g. GNOME, which is the default desktop on most Linux systems, doesn't really do minimize. |
KDE 5 on X11.
Blocking compositing was historically a big issue outside KDE. Only KDE allowed to block it automatically for fullscreen applications and provided the shortcut for manually disabling it. I don't really know what the current state of GNOME and possibly Wayland is, and if they even want this or care. |
Compositing is fundamental to modern desktops, so I don't think there's any option of blocking that. I was rather hoping that the desktop would be kind enough to send us some other signal to indicate "you're currently not visible, so don't waste time rendering stuff". |
Do not request screen updates while the window is not visible, e.g. minimized.
This saves a lot of bandwidth and CPU.
New option: UpdateOnlyWhenVisible