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

paint everything together with opengl #792

Closed
totaam opened this issue Jan 24, 2015 · 12 comments
Closed

paint everything together with opengl #792

totaam opened this issue Jan 24, 2015 · 12 comments

Comments

@totaam
Copy link
Collaborator

totaam commented Jan 24, 2015

Issue migrated from trac ticket # 792

component: client | priority: minor | resolution: fixed

2015-01-24 07:14:41: totaam created the issue


Instead of doing the video screen update, followed by the rgb paint for the 1 pixel bottom and right edges, we could either bundle them together (as an array of updates?), or at least pass a flag that implies "present" so that we update the back buffer and only when everything is updated we present it on screen with a single swap-buffers.

Somewhat related to other opengl improvements, see #679.

@totaam
Copy link
Collaborator Author

totaam commented Jan 24, 2015

2015-01-24 07:15:12: totaam changed status from new to assigned

@totaam
Copy link
Collaborator Author

totaam commented Jan 24, 2015

2015-01-24 07:15:12: totaam changed owner from antoine to totaam

@totaam
Copy link
Collaborator Author

totaam commented Jan 24, 2015

2015-01-24 07:15:12: totaam changed title from paint thing together with opengl to paint everything together with opengl

@totaam
Copy link
Collaborator Author

totaam commented Jul 3, 2015

2015-07-03 16:38:14: antoine commented


I want to get this in 0.16, at the very least for the "easy" case of 1 pixel edge windows. If we ever get vsync frames on the server (#386 - maybe as part of wayland #387), then this will take care of the client side.

We should be able to add an extra optional flag called "flush", which would default to True when not present (older servers, or for the normal path).
When we process multiple regions (1 pixel case, or as part of the same set of batched screen updates), we can set the flag on all but the last paint packet.
Client-side, we can just accumulate the rectangles in present_fbo and only process them when we flush.

@totaam
Copy link
Collaborator Author

totaam commented Jul 6, 2015

2015-07-06 11:59:30: antoine changed status from assigned to new

@totaam
Copy link
Collaborator Author

totaam commented Jul 6, 2015

2015-07-06 11:59:30: antoine changed owner from totaam to afarr

@totaam
Copy link
Collaborator Author

totaam commented Jul 6, 2015

2015-07-06 11:59:30: antoine commented


Done in r9846.

As per the commit message, this affects:

  • regions batched up together (happens regularly)
  • edges of video encoders (which are usually limited to even dimensions, so we send the edge separately using rgb)

We now send a flag to the client so it will delay updating the screen until all the screen updates have been received. (flush=0 or unset)

This is what it looks like with -d opengl:

  • a normal paint (without delayed flush)
present_fbo: adding (155, 756, 6, 13) to pending paint list, flush=0
do_present_fbo: painting [(155, 756, 6, 13)]
  • a group of delayed paints:
present_fbo: adding (17, 197, 90, 13) to pending paint list, flush=9
present_fbo: adding (107, 197, 24, 13) to pending paint list, flush=8
present_fbo: adding (155, 756, 6, 13) to pending paint list, flush=7
present_fbo: adding (17, 210, 240, 13) to pending paint list, flush=6
present_fbo: adding (131, 197, 852, 13) to pending paint list, flush=5
present_fbo: adding (17, 171, 966, 13) to pending paint list, flush=4
present_fbo: adding (17, 158, 966, 13) to pending paint list, flush=3
present_fbo: adding (17, 184, 48, 13) to pending paint list, flush=2
present_fbo: adding (65, 184, 918, 13) to pending paint list, flush=1
present_fbo: adding (509, 145, 474, 13) to pending paint list, flush=0
do_present_fbo: painting [(17, 197, 90, 13), (107, 197, 24, 13), (155, 756, 6, 13), (17, 210, 240, 13), (131, 197, 852, 13), (17, 171, 966, 13), (17, 158, 966, 13), (17, 184, 48, 13), (65, 184, 918, 13), (509, 145, 474, 13)]

Notable difference: on win32, we use double buffering and so the painting will just paint the whole window (a single region will be shown).

Hopefully, this will reduce CPU load on the client (I guess this could be done in #797), and give us smoother repaints, and maybe even a higher refresh rate.
@afarr: mostly a FYI, I can't think of anything specific to test. Maybe just verify the present_fbo debug output matches expectations.

@totaam
Copy link
Collaborator Author

totaam commented Sep 12, 2015

2015-09-12 02:44:05: afarr changed owner from afarr to antoine

@totaam
Copy link
Collaborator Author

totaam commented Sep 12, 2015

2015-09-12 02:44:05: afarr commented


Tested with 0.16.0 r10624 windows and osx clients against 0.16.0 r10624 fedora 21 client.

I'm seeing the present_fbo output as expected. It is nearly always doing normal paints (always in my short test on windows client), but I saw some of the delayed paints on OSX (with messages in between about gtk2 window backing stats and notifications of switching to RGB paint state in between a flush=2 and a flush=1 and a flush=0).

I'm assuming that is expected, and handing this back to you to close. If that sounds unexpected let me know and I'll collect some logs.

@totaam
Copy link
Collaborator Author

totaam commented Sep 12, 2015

2015-09-12 09:05:37: antoine changed status from new to closed

@totaam
Copy link
Collaborator Author

totaam commented Sep 12, 2015

2015-09-12 09:05:37: antoine set resolution to fixed

@totaam
Copy link
Collaborator Author

totaam commented Sep 12, 2015

2015-09-12 09:05:37: antoine commented


@afarr: you should be able to reproduce more easily on win32:

  • with an odd sized window and video, which is probably what you were seeing on osx: flush=2 will be a large video yuv paint, followed by flush=1 and flush=0 as rgb
  • with rgb encoding and lots of activity on screen
  • with XPRA_BATCH_MIN_DELAY=50 XPRA_BATCH_ALWAYS=1 with r10625 onwards, this caps the framerate at 20 fps and forces paint batching

Will follow up in #981, so closing here.

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

No branches or pull requests

1 participant