Opened 11 years ago
Closed 10 years ago
#11503 closed defect (fixed)
glx.c in crOpenGL needs to open a second X Connection in order to process damage events -> fixed as of 21 Oct 2013, 4.2 and later
| Reported by: | smspillaz | Owned by: | |
|---|---|---|---|
| Component: | 3D support | Version: | VirtualBox 4.2.6 |
| Keywords: | opengl, linux | Cc: | |
| Guest type: | Linux | Host type: | Linux |
Description
This is more of a follow up to https://www.virtualbox.org/ticket/10894 .
glx.c in crOpenGL still needs to open up a second X connection in order to process damage events. Processing damage on pixmaps before re-copying them is fine, but using a second X connection in order to fetch the damage is risky because at any point in your library, you could be affected by a server grab in the client process. There is a an issue with just listening for damage on the first connection, because the client might interfere with that. (I've found there are places where compiz may still hang in VirtualBox 4.2.6 because of this). So instead, the best way to handle it is to fetch damage synchronously from the server using XDamageSubtract and XFixesFetchRegion. This doesn't have any impact on performance, because the previous operation was already synchronous as we had to call XSync before fetching damage evnets.
I've attached a patch to do that. I've checked that it compiles, however I'm not able to get VirtualBox to install correctly, so I can't verify that it works as expected. However, I've written similar code in the past, so I'm 90% sure it should work.
(At the moment we're just working around this in compiz itself, but that's ugly because we had to do things like detect drivers).
Attachments (4)
Change History (11)
by , 11 years ago
| Attachment: | 0001-Remove-the-second-damage-display-connection-and-inst.2.patch added |
|---|
by , 11 years ago
| Attachment: | 0001-Remove-the-second-damage-display-connection-and-inst.patch added |
|---|
Don't open a second X connection and query for damage directly
comment:1 by , 11 years ago
Ah, just ignore the first listed patch. I'm not sure how to remove it from here.
comment:2 by , 11 years ago
| Component: | guest additions → 3D support |
|---|
comment:3 by , 11 years ago
Sorry for being slow to respond to this. I applied your patch, adjusted for the current code base, but when Unity is loaded straight from LightDM on an Ubuntu 12.10 guest I only see the background. When I run it from a terminal I see output telling me it is using slow software rendering but it works. I will upload my re-factored version of your patch.
by , 11 years ago
| Attachment: | Remove-the-second-damage-display-connection-and-inst-adjusted-1.patch added |
|---|
Adjusted patch
by , 11 years ago
| Attachment: | unity-output added |
|---|
Output when Unity is started from the command line.
comment:4 by , 11 years ago
By the way I am semi-familiar with that part of our code but not fully, in case that is not clear from my comment!
comment:5 by , 11 years ago
| Summary: | glx.c in crOpenGL needs to open a second X Connection in order to process damage events → glx.c in crOpenGL needs to open a second X Connection in order to process damage events -> fixed as of 21 Oct 2013, 4.2 and later |
|---|
Sorry again that this has taken so long. I have applied your patch manually to take the code base changes into account and found it to work with Ubuntu 8.10, 10.04 and 13.10 guests, the first two of which were hanging on Compiz start-up without the patch. I still have to give above-mentioned Ubuntu 12.10 guest another try, but I think that given the success with the others I will keep the patch and treat that as a new issue if it is still causing problems.
Thank you very much for your analysis and the patch.
comment:6 by , 11 years ago
Ubuntu 12.10 works too. So either I made some silly mistake applying the patch last time (can't see it) or there was another relevant fix in the mean-time.
Don't open a second X connection and query for damage directly