VirtualBox

Ticket #11503 (closed defect: fixed)

Opened 14 months ago

Last modified 6 months ago

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:
Priority: minor 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

0001-Remove-the-second-damage-display-connection-and-inst.2.patch Download (439 bytes) - added by smspillaz 14 months ago.
Don't open a second X connection and query for damage directly
0001-Remove-the-second-damage-display-connection-and-inst.patch Download (9.1 KB) - added by smspillaz 14 months ago.
Don't open a second X connection and query for damage directly
Remove-the-second-damage-display-connection-and-inst-adjusted-1.patch Download (8.4 KB) - added by michael 13 months ago.
Adjusted patch
unity-output Download (29.5 KB) - added by michael 13 months ago.
Output when Unity is started from the command line.

Change History

Changed 14 months ago by smspillaz

Don't open a second X connection and query for damage directly

Changed 14 months ago by smspillaz

Don't open a second X connection and query for damage directly

comment:1 Changed 14 months ago by smspillaz

Ah, just ignore the first listed patch. I'm not sure how to remove it from here.

comment:2 Changed 14 months ago by misha

  • Component changed from guest additions to 3D support

comment:3 Changed 13 months ago by michael

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.

Changed 13 months ago by michael

Adjusted patch

Changed 13 months ago by michael

Output when Unity is started from the command line.

comment:4 Changed 13 months ago by michael

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 Changed 6 months ago by michael

  • Summary changed from glx.c in crOpenGL needs to open a second X Connection in order to process damage events to 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 Changed 6 months ago by michael

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.

comment:7 Changed 6 months ago by frank

  • Status changed from new to closed
  • Resolution set to fixed

Fixed in 4.3.2.

Note: See TracTickets for help on using tickets.

www.oracle.com
ContactPrivacy policyTerms of Use