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

transparency support is broken with pixmap backing #961

Closed
totaam opened this issue Aug 23, 2015 · 7 comments
Closed

transparency support is broken with pixmap backing #961

totaam opened this issue Aug 23, 2015 · 7 comments

Comments

@totaam
Copy link
Collaborator

totaam commented Aug 23, 2015

Issue migrated from trac ticket # 961

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

2015-08-23 10:54:35: antoine created the issue


Tested with opengl=no and various encodings as part of the testing for #937 and associated changes (in particular the new test code in r10409), the client still claims to support alpha and tells the server (with added debug):

HAS_ALPHA=True, properties={'encoding.transparency': False, 'encodings.rgb_formats': ('RGB', 'RGBX', 'RGBA'), \
    'encoding.full_csc_modes': {
        'h264': ('ARGB', 'BGRA', 'BGRX', 'GBRP', 'RGB', 'XRGB', 'YUV420P'), 'h265': ('GBRP', 'RGB', 'YUV420P'), \
        'vp9': ('YUV420P',), \
        'vp8': ('YUV420P',)}, 'encoding.csc_modes': ('BGRX', 'GBRP', 'RGB', 'YUV420P', 'BGRA', 'ARGB', 'XRGB') \
    }, supports_transparency=True

But the standard test scripts come up without transparency..

@totaam
Copy link
Collaborator Author

totaam commented Aug 23, 2015

2015-08-23 11:08:06: antoine changed status from new to assigned

@totaam
Copy link
Collaborator Author

totaam commented Aug 23, 2015

2015-08-23 11:08:06: antoine edited the issue description

@totaam
Copy link
Collaborator Author

totaam commented Aug 23, 2015

2015-08-23 11:08:06: antoine commented


Bisected down:

So, the problem is from r8551 which synchronizes _NET_WM_BYPASS_COMPOSITOR.
It looks innocuous... but it does call realize().

init_window(metadata) comes before setup_window(), and so it may realize the window before we have had a chance to set the colormap.

@totaam
Copy link
Collaborator Author

totaam commented Aug 23, 2015

2015-08-23 13:02:22: antoine commented


In theory, something like this should fix it (delaying metadata init until after setup_window):

--- xpra/client/client_window_base.py	(revision 10395)
+++ xpra/client/client_window_base.py	(working copy)
@@ -49,6 +49,7 @@
 
         self.init_window(metadata)
         self.setup_window()
+        self.update_metadata(metadata)
 
     def __repr__(self):
         return "ClientWindow(%s)" % self._id
@@ -68,7 +69,6 @@
         self._window_workspace = WORKSPACE_UNSET        #will get set in set_metadata if present
         self._desktop_workspace = self.get_desktop_workspace()
         workspacelog("init_window(..) workspace=%s, current workspace=%s", self._window_workspace, self._desktop_workspace)
-        self.update_metadata(metadata)
 
 
     def get_desktop_workspace(self):

But it doesn't... no idea why.
Neither does turning off "bypass-compositor" syncing.. so another change must have the same effect.
I even tried to turn off most of the handlers that call realize(), still no luck!

@totaam
Copy link
Collaborator Author

totaam commented Aug 23, 2015

2015-08-23 15:33:18: antoine changed status from assigned to closed

@totaam
Copy link
Collaborator Author

totaam commented Aug 23, 2015

2015-08-23 15:33:18: antoine set resolution to fixed

@totaam
Copy link
Collaborator Author

totaam commented Aug 23, 2015

2015-08-23 15:33:18: antoine commented


Bisecting again, but connecting to a 0.14.x server, so that the "bypass-compositor" code doesn't fire and we can find the second transparency bug:

So the second bug is caused by r9977: which is now partially reverted in r10419. That fixes transparency when connecting to older servers.
We now also skip unpremultiply if there is no alpha (which would do absolutely nothing useful), as is the case with csc modules giving us RGBX or with mmap giving us raw BGRX.

For newer servers, you also need r10420 which applies the patch from comment:2. The potential for regressions from this one is high...

And since I was re-ordering initialization code, I took it a step further and cleaned up the mess that was the realize() override and delayed properties (many attributes are in fact plain X11 properties that can only be set once the window is realized): see r10421.
Again, the potential for regressions is higher than normal.

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