xpra icon
Bug tracker and wiki

Opened 6 years ago

Closed 6 years ago

#385 closed enhancement (fixed)

opengl rendering improvements: handle plain RGB, scaling, transparency

Reported by: Antoine Martin Owned by: alas
Priority: major Milestone: 0.11
Component: client Version:
Keywords: opengl Cc:

Description (last modified by Antoine Martin)

See wiki/ClientRendering and #350 (which is probably needed first)


In order to bring feature parity with the regular gtk2 pixmap backing, we would need:

  • handing plain RGB modes - maybe the fbo code does most of that already? (then we just need to enable it?)
  • handle scaling
  • handle transparency: see #279 and OpenGL Common Mistakes: no alpha in the framebuffer
  • ensure we don't try to create textures bigger than GL_MAX_RECTANGLE_TEXTURE_SIZE_ARB as this won't work.. problem is that we may resize at any time, so maybe we should just not use OpenGL when there is any risk that the windows will be too big: whenever the total display size is bigger than the texture size. Another solution would be to dynamically switch to PixmapBacking when needed (as there is little GL specific code in GLClientWindow anyway)


This will make the code more manageable (fewer special cases), should make the overall experience better (ie: we can then use RGB modes with mmap and gl, x264 can encode to/from rgb without doing csc server side) and will give the opengl more of a chance to be used.

Attachments (1)

opengl-transparency-v5.patch (11.2 KB) - added by Antoine Martin 6 years ago.
working patch for posix clients (win32 broken)

Download all attachments as: .zip

Change History (22)

comment:1 Changed 6 years ago by Antoine Martin

Description: modified (diff)

comment:2 Changed 6 years ago by Antoine Martin

Description: modified (diff)

comment:3 Changed 6 years ago by Antoine Martin

Description: modified (diff)

scaling completed in r3906

comment:4 Changed 6 years ago by Antoine Martin

Description: modified (diff)

comment:5 Changed 6 years ago by Antoine Martin

The problem with video RGB painting is that the data coming out of x264 is in planar mode ("GBRP") so we cannot re-use _do_paint_rgb24 as is.


Can we update the fbo one plane at a time? Maybe using a new texture for each plane?
Or is our only option to first convert to plain RGB and re-use the _do_paint_rgb24 code?


The client-side conversion is probably almost as costly as doing YUV444 to RGB csc server-side before compression, but since we are CPU bound server-side, we are much better off shifting this burden to the client. (effectively lowering the overall latency)

comment:6 Changed 6 years ago by Antoine Martin

Description: modified (diff)

comment:7 Changed 6 years ago by ahuillet

Theoretically we can paint planar RGB. It requires a separate shader and some more support code, however.

comment:8 Changed 6 years ago by Antoine Martin

Planar RGB done in r3928 + r3927 + r3926 + r3925 + r3923 + r3922 + r3921 + r3920

comment:9 Changed 6 years ago by Antoine Martin

Looks to me like the new buffer management code (#350) is making PyOpenGL unhappy (100% reproducible):

OpenGL paint error: ("No array-type handler for type \
    <type 'buffer'> (value: <read-only buffer ptr 0x7f394011b910, size 171904 ) registered", \
    <OpenGL.GL.images.ImageInputConverter object at 0x2277390>)
Traceback (most recent call last):
  File "xpra/client/gl/gl_window_backing.py", line 376, in gl_paint_planar
    self.update_planar_textures(x, y, enc_width, enc_height, img, pixel_format, scaling=(enc_width!=width or enc_height!=height))
  File "xpra/client/gl/gl_window_backing.py", line 431, in update_planar_textures
    glTexSubImage2D(GL_TEXTURE_RECTANGLE_ARB, 0, x, y, width/div_w, height/div_h, GL_LUMINANCE, GL_UNSIGNED_BYTE, pixel_data)
  File "OpenGL/latebind.py", line 45, in __call__
    return self._finalCall( *args, **named )
  File "OpenGL/wrapper.py", line 781, in wrapperCall
    pyArgs = tuple( calculate_pyArgs( args ))
  File "OpenGL/wrapper.py", line 354, in calculate_pyArgs
    yield converter(args[index], self, args)
  File "OpenGL/GL/images.py", line 433, in __call__
    return arrayType.asArray( arg )
  File "OpenGL/arrays/arraydatatype.py", line 141, in asArray
    return cls.getHandler(value).asArray( value, typeCode or cls.typeConstant )
  File "OpenGL/arrays/arraydatatype.py", line 52, in __call__
    typ, repr(value)[:50]
TypeError: ("No array-type handler for type <type 'buffer'> \
    (value: <read-only buffer ptr 0x7f394011b910, size 171904 ) registered", \
    <OpenGL.GL.images.ImageInputConverter object at 0x2277390>)

(This particular trace occurred with GBRP when using quality=100)
Solutions (most of which assume that the problem is to do with read-only memory - TBC):

  • make the buffer read-write? We would need to check with avcodec which has functions we must call to check if the buffer is indeed (still?) writeable - and if not... not sure what we can do, since the read/write attributes must be set on construction via PyBuffer_FromMemory
  • maybe we can find an image input converter that does handle read-only buffers?
  • just copy the buffer to read-write memory: doing it from avcodec would be wasteful (other client backings do not require it), doing it from the gl backing is harder as we would need to know that the buffer is read-only in the first place..
Last edited 6 years ago by Antoine Martin (previous) (diff)

comment:10 Changed 6 years ago by Antoine Martin

Damn, read-write makes no difference, so maybe PyOpenGL requires a new style buffer. PITA

This works around the issue by copying to a new string object before passing the data to gl:

--- src/xpra/client/gl/gl_window_backing.py	(revision 3977)
+++ src/xpra/client/gl/gl_window_backing.py	(working copy)
@@ -426,7 +426,7 @@
             glActiveTexture(texture)
             glBindTexture(GL_TEXTURE_RECTANGLE_ARB, self.textures[index])
             glPixelStorei(GL_UNPACK_ROW_LENGTH, rowstrides[index])
-            pixel_data = img_data[index]
+            pixel_data = img_data[index][:]
             debug("texture %s: div=%s, rowstride=%s, %sx%s, data=%s bytes", index, divs[index], rowstrides[index], width/div_w, height/div_h, len(pixel_data))
             glTexSubImage2D(GL_TEXTURE_RECTANGLE_ARB, 0, x, y, width/div_w, height/div_h, GL_LUMINANCE, GL_UNSIGNED_BYTE, pixel_data)
             if index == 1:

comment:11 Changed 6 years ago by Antoine Martin

r3985 applies the change from comment:10, just so we can move on and use the code, but this will need a better solution eventually.

comment:12 Changed 6 years ago by Antoine Martin

r4017 uses a string wrapper rather than a buffer wrapper, which makes pyopengl happy and allows us to revert the extra buffer copy added in r3985.

I've left swscale alone, since we don't use it with opengl, but it may need the same treatment if we ever do csc with opengl (for whatever reason).


Items left to do:

  • GL_MAX_RECTANGLE_TEXTURE_SIZE_ARB should be exposed to the client class so it can avoid gl windows if the screen size is too big (>4k is not uncommon these days)
  • transparency

comment:13 Changed 6 years ago by Antoine Martin

r4017 was "wrong": the string wrapper makes a copy... which is what we tried to avoid, so r4048 reverts that and just makes the buffer copy from the decoding thread instead of the UI thread (and only for opengl)... so we still need to find a better way to make the buffer work with zero copy and pyopengl... maybe The new-style Py_buffer struct

comment:14 Changed 6 years ago by Antoine Martin

Milestone: 0.11future
Owner: changed from ahuillet to Antoine Martin
Status: newassigned

Nice-to-have but not essential, re-scheduling.

comment:15 Changed 6 years ago by Antoine Martin

r4480 made the pixel buffer read/write, so we no longer have to copy the pixels before uploading them with glTexSubImage2D (see comment:9), removed copying in r4702

Changed 6 years ago by Antoine Martin

working patch for posix clients (win32 broken)

comment:16 Changed 6 years ago by Antoine Martin

Summary: opengl rendering improvementsopengl rendering improvements: handle plain RGB, scaling, transparency

r4880 + r4881 + r4883 adds transparency support for GL windows, r4879 (server side), backported in r4894 so that older servers don't end up sending BGRA pixels without telling the client the data is in BGRA and not RGBA... (as is normally the case when we don't specify)


This change is already very useful for mmap/rgb modes where we now handle 32-bit pixel data in native BGR(A) format, so the server no longer needs to do any byteswapping and/or losing the alpha channel, it will also be very useful with vp9/h265 which both support alpha channels.


Buffer improvements task is now in its own ticket: #465


Last remaining item for this ticket is the maximum GL texture size, which is made more difficult by #263: we cannot just set a maximum window size as this also changes the behaviour of the UI..

Last edited 6 years ago by Antoine Martin (previous) (diff)

comment:17 Changed 6 years ago by Antoine Martin

Owner: changed from Antoine Martin to alas
Status: assignednew

r4882 + r4883 improves texture size checking and ensures we have a fallback in case GL window setup fails. We already had a check to disable GL if the total screen space is bigger than the maximum texture supported. Ideally, we would also deal with failures during window resize, as one can resize a window and make it bigger than the screen size... but this will do for now (modern graphics card support 16k x 16k texture sizes which is more than we need).

One remaining niggle is the fact that I cannot get OpenGL to use the premultiplied alpha despite following the blending docs... (see r4902)



afarr: please test combinations I do not have access to (as of r4977, one needs to use XPRA_ALPHA=1 xpra attach ... to try to enable alpha channel on osx and win32 where it is normally disabled by default):

  • OSX clients (which may support transparency - TBC)
  • win32 clients (no transparency in gtk2)

To make sure that there are no regressions - the performance improvements are too small to be measurable under normal usage.

The video encodings (vpx and h264) should be unchanged, rgb, png and webp should support window transparency (xpra.test_apps.transparent_colors can be used to test transparency easily) on OpenGL windows (verify window type is GL in session info).

Last edited 6 years ago by Antoine Martin (previous) (diff)

comment:18 Changed 6 years ago by Antoine Martin

Milestone: future0.12

This belongs in the 0.11 milestone, new ticket added for resizing flickering for 0.12: #478

comment:19 Changed 6 years ago by Antoine Martin

Milestone: 0.120.11

Oooops, wrong milestone update

Last edited 6 years ago by Antoine Martin (previous) (diff)

comment:20 Changed 6 years ago by alas

Testing with win client 0.11.0 r5144 (fedora 19 0.11.0 r5144): with rgb and png the transparent colors look lovely. With webp the squares are the right colors but streaked with black (not lovely).

Testing with osx client 0.11.0 r5144 produces the same results: rgb and png work as expected and webp doesn't support the transparencies.

comment:21 Changed 6 years ago by Antoine Martin

Resolution: fixed
Status: newclosed

The webp issue is not GL related (which is what this ticket is about), moved to #487

  • transparency works (bar webp issue above)
  • scaling - I had tested
  • window size limits - I had tested

Closing.

Note: See TracTickets for help on using tickets.