opengl rendering improvements: handle plain RGB, scaling, transparency

[totaam]

Issue migrated from trac ticket # 385

component: client | priority: major | resolution: fixed | keywords: opengl

2013-07-18 06:40:55: antoine created the issue

---

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

[[BR]]

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 32-bit visuals and transparent windows #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)

[[BR]]

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.

[totaam]

2013-07-18 06:43:50: antoine edited the issue description

[totaam]

2013-07-18 06:47:02: antoine edited the issue description

[totaam]

2013-07-21 06:22:33: antoine edited the issue description

[totaam]

2013-07-21 06:22:33: antoine commented

---

scaling completed in r3906

[totaam]

2013-07-21 06:25:52: antoine edited the issue description

[totaam]

2013-07-21 07:17:20: antoine commented

---

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.

[[BR]]

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?

[[BR]]

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)

[totaam]

2013-07-21 07:18:20: antoine edited the issue description

[totaam]

2013-07-21 09:04:35: ahuillet commented

---

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

[totaam]

2013-07-21 14:37:38: antoine commented

---

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

[totaam]

2013-07-28 06:20:21: antoine commented

---

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

[totaam]

2013-07-28 07:04:42: antoine commented

---

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:

[totaam]

2013-07-28 12:54:33: antoine commented

---

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.

[totaam]

2013-07-30 10:44:28: antoine commented

---

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

[totaam]

2013-08-05 06:53:15: antoine commented

---

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

[totaam]

2013-10-17 08:42:06: totaam changed status from new to assigned

[totaam]

2013-10-17 08:42:06: totaam changed owner from ahuillet to totaam

[totaam]

2013-10-17 08:42:06: totaam commented

---

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

[totaam]

2013-11-06 11:55:22: totaam commented

---

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

[totaam]

2013-12-08 10:14:52: totaam uploaded file opengl-transparency-v5.patch (11.2 KiB)

working patch for posix clients (win32 broken)

[totaam]

2013-12-08 12:24:17: totaam changed title from opengl rendering improvements to opengl rendering improvements: handle plain RGB, scaling, transparency

[totaam]

2013-12-08 12:24:17: totaam commented

---

r4880 + r4881 + r4883 adds transparency support for GL windows, r4879 (server side), backported in 4894 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)

[[BR]]

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.

[[BR]]

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

[[BR]]

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

[totaam]

2013-12-08 14:48:05: totaam changed status from assigned to new

[totaam]

2013-12-08 14:48:05: totaam changed owner from totaam to afarr

[totaam]

2013-12-08 14:48:05: totaam commented

---

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).
[[BR]]
One remaining niggle is the fact that I cannot get OpenGL to use the premultiplied alpha despite following the blending docs... (see r4902)

[[BR]]

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 ([/browser/xpra/trunk/src/tests/xpra/test_apps/transparent_colors.py xpra.test_apps.transparent_colors] can be used to test transparency easily) on OpenGL windows (verify window type is GL in session info).

[totaam]

2013-12-18 08:37:09: totaam commented

---

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

[totaam]

2013-12-18 08:44:17: totaam commented

---

Oooops, wrong milestone update

[totaam]

2014-01-09 01:15:16: afarr commented

---

Testing with win client 0.11.0 5144 (fedora 19 0.11.0 5144): 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 5144 produces the same results: rgb and png work as expected and webp doesn't support the transparencies.

[totaam]

2014-01-09 06:52:46: totaam changed status from new to closed

[totaam]

2014-01-09 06:52:46: totaam changed resolution from ** to fixed

[totaam]

2014-01-09 06:52:46: totaam commented

---

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.