#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 )
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 useOpenGL
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 toPixmapBacking
when needed (as there is little GL specific code inGLClientWindow
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)
Change History (23)
comment:1 Changed 8 years ago by
Description: | modified (diff) |
---|
comment:2 Changed 8 years ago by
Description: | modified (diff) |
---|
comment:3 Changed 8 years ago by
Description: | modified (diff) |
---|
comment:4 Changed 8 years ago by
Description: | modified (diff) |
---|
comment:5 Changed 8 years ago by
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 8 years ago by
Description: | modified (diff) |
---|
comment:7 Changed 8 years ago by
Theoretically we can paint planar RGB. It requires a separate shader and some more support code, however.
comment:8 Changed 8 years ago by
comment:9 Changed 8 years ago by
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..
comment:10 Changed 8 years ago by
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 8 years ago by
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 8 years ago by
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 8 years ago by
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 8 years ago by
Milestone: | 0.11 → future |
---|---|
Owner: | changed from ahuillet to Antoine Martin |
Status: | new → assigned |
Nice-to-have but not essential, re-scheduling.
comment:15 Changed 7 years ago by
Changed 7 years ago by
Attachment: | opengl-transparency-v5.patch added |
---|
working patch for posix clients (win32 broken)
comment:16 Changed 7 years ago by
Summary: | opengl rendering improvements → opengl 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..
comment:17 Changed 7 years ago by
Owner: | changed from Antoine Martin to alas |
---|---|
Status: | assigned → new |
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/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).
comment:18 Changed 7 years ago by
Milestone: | future → 0.12 |
---|
This belongs in the 0.11 milestone, new ticket added for resizing flickering for 0.12: #478
comment:20 Changed 7 years ago by
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 7 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
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.
comment:22 Changed 3 months ago by
this ticket has been moved to: https://github.com/Xpra-org/xpra/issues/385
scaling completed in r3906