Last Comment Bug 752640 - Allow PCompositor to work with Basic Layers backend
: Allow PCompositor to work with Basic Layers backend
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Graphics: Layers (show other bugs)
: Trunk
: All Linux
: -- normal (vote)
: mozilla15
Assigned To: Oleg Romashin (:romaxa)
:
: Milan Sreckovic [:milan]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-07 12:45 PDT by Oleg Romashin (:romaxa)
Modified: 2012-05-09 03:45 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Make PCompositorParent work with BASIC_LAYERS manager (1.41 KB, patch)
2012-05-07 12:51 PDT, Oleg Romashin (:romaxa)
ajuma.bugzilla: review+
Details | Diff | Splinter Review

Description Oleg Romashin (:romaxa) 2012-05-07 12:45:16 PDT
Make compositor Parent deal with LAYERS_BASIC backend
Tested with OMTC embedding widget and it seems to work fine.
OMTC embedding has target surface and PCompositorParent working in the same threads, so for other threading model probably additional thread safety code required.
Comment 1 Oleg Romashin (:romaxa) 2012-05-07 12:51:11 PDT
Created attachment 621694 [details] [diff] [review]
Make PCompositorParent work with BASIC_LAYERS manager
Comment 2 Ali Juma [:ajuma] 2012-05-07 14:50:21 PDT
(In reply to Oleg Romashin (:romaxa) from comment #0)
> OMTC embedding has target surface and PCompositorParent working in the same
> threads, so for other threading model probably additional thread safety code
> required.

As I understand it, the thread safety issue comes from using Cairo on both the shadowable side and the shadow side, which would be the case whenever we're doing OMTC with BasicLayers.

I'm fine with taking this patch though, since it lets us starting experimenting with BasicLayers OMTC (which might help us decide whether we want to invest more time in it).
Comment 3 Ali Juma [:ajuma] 2012-05-07 14:52:15 PDT
Comment on attachment 621694 [details] [diff] [review]
Make PCompositorParent work with BASIC_LAYERS manager

Review of attachment 621694 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/layers/ipc/CompositorParent.cpp
@@ +501,5 @@
>      if (!slm) {
>        return NULL;
>      }
>      return new ShadowLayersParent(slm, this);
> +  } else if (backendType == LayerManager::LAYERS_BASIC) {

Add a comment here that using this means using Cairo on two threads, so in order to ship this we need to build Cairo thread-safe (which might cause performance regressions).
Comment 4 Benoit Girard (:BenWa) 2012-05-07 15:16:19 PDT
The other alternative would be a new basic backend that doesn't use cairo (uses pixman directly maybe?). Not sure if that's a good idea or not.
Comment 6 Ed Morley [:emorley] 2012-05-09 03:45:36 PDT
https://hg.mozilla.org/mozilla-central/rev/e9e72f732d8b

Note You need to log in before you can comment on or make changes to this bug.