Open Bug 715433 Opened 8 years ago Updated 7 years ago

Full shadow-layer support for d3d10

Categories

(Core :: Graphics, defect)

x86
Windows 7
defect
Not set

Tracking

()

People

(Reporter: cjones, Unassigned)

References

(Blocks 1 open bug)

Details

We weren't able to re-use the generic basic-layers --> GPU layers remoting code with d3d10 because we need a d3d10 layer manager to use d2d well (so I was told).  So the current shadow layers impl renders to a back-buffer texture the size of the visible region on the child side and then swaps that on the parent side.  We did this simpler impl in case NGFX was ready in time.

The simple setup will continue to work for omtc, except we won't be able to use direct plugin-->compositor updates out of the box.  The current setup however *won't* work for async css animations.

No matter what, we need to add support for all Shadow*LayerD3D10 types.  That will be relatively easy.

We also need to decide which layers backend to use on the child side.

Option 1: try to use the generic basic-layers --> d3d10 path.  I remember concerns about d2d here, but I don't remember what they were specifically.  We're going to need to synchronize across thread contexts anyway, so maybe we can pull this off without extra overhead.

Option 2: add full shadowable layer support to d3d10.  Azure isn't going to be ready for remoting before we want omtc and async animation, so this is more palatable today.  This wouldn't be particularly difficult, just extra code.  We should ensure it's worth the maintenance burden.
The issue with basic-layers --> d3d10 path across processes is that each process gets its own command queue, so we need to synchronize explicitly.  Bas tells me multiple threads will share the same command queue, so they're implicitly synchronized.  So this isn't an issue cross-thread.  (This sounds almost too good to be true but I can't think of any issues yet.)

So, Bas and I agreed on basically Option 1 above, but we also discussed a refactoring that should make all this code cleaner.  All the layer backends have two main jobs, drawing to layers and then compositing them.  The code to draw layers is only interesting for thebes layers, and in all backends it basically looks like
 - [mumble] flag, dirty region management, etc.
 - [mumble] optimized self-copies
 - [mumble] component alpha, in GPU backends

The interesting code that really is different across all backends is for compositing.

So, the proposal is to split LayerManagers across that boundary, into (names tbd) something like LayerDrawer and LayerCompositor (LayerRenderer?).  There would still be a LayerManager API shared by both, for transactions and tree edits etc.

LayerDrawer would be approximately what ShadowLayerForwarder is now, and LayerCompositor would be approximately ShadowLayerManager.  All *Shadow*Layer*s would go away, and we would return to only using shadowable layers in one backend, this time the LayerDrawer.  The concept of shadowable layers would go away; all drawn layers are shadowable.

There would only be one LayerDrawer, and it would be extracted from the current basic layers code.  We would need to ensure we have properly optimized self-copies for all platforms.  We'll need some work for mac, but we have to do that anyway for omtc.  We also need support for sharing white/black buffers to the compositor for optimized component alpha, when using GPU compositors.  That's easy.

There would be a LayerCompositor for thebes, GL, d3d9, and d3d10.

In addition to sharing more code among backends and cleaning up the current rather suboptimal state of shadow layers (yay death to MOZ_IPC), this might allow us to resolve a potential problem with omtc: thread-safe CPU compositing.  We could have the LayerDrawer use thebes, and have an azure LayerCompositor that depended on a small amount of backend support, much less than the full 2D drawing API.  But it still might be easier to make cairo thread safe.

The general drawing path would become
 - LayerDrawer transaction
 - if compositor on different thread: IPC sharing magic to LayerCompositor
 - LayerCompositor transaction

A potential issue with that new approach is that rendering becomes less friendly to CPU cache for same-thread, CPU-composited backends.  Maybe the effect isn't measurable, or we don't care enough about that configuration going forward.

Thoughts?  I think the Layers API changes would be pretty small, but I haven't thought them through yet.
This sounds to me like we take the current "main thread layer manager"/"shadow layer manager" setup, rename the main thread layer manager to LayerDrawer, rename the shadow layer manager to LayerCompositor, consolidate to a single LayerDrawer implementation, and specialize LayerDrawer and LayerCompositor so that LayerDrawer can't do any compositing and LayerCompositor can't do any drawing. Is that what you intended?

If so, it sounds reasonable, but I'm not sure you can make LayerDrawer 100% independent of the LayerCompositor backend.
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #2)
> This sounds to me like we take the current "main thread layer
> manager"/"shadow layer manager" setup, rename the main thread layer manager
> to LayerDrawer, rename the shadow layer manager to LayerCompositor,
> consolidate to a single LayerDrawer implementation, and specialize
> LayerDrawer and LayerCompositor so that LayerDrawer can't do any compositing
> and LayerCompositor can't do any drawing. Is that what you intended?
> 

Yes.

> If so, it sounds reasonable, but I'm not sure you can make LayerDrawer 100%
> independent of the LayerCompositor backend.

It's not currently anyway, in the shadowable/shadow layer setup.
> > If so, it sounds reasonable, but I'm not sure you can make LayerDrawer 100%
> > independent of the LayerCompositor backend.
> 
> It's not currently anyway, in the shadowable/shadow layer setup.

I know. Just saying that you might not reach all the way to your goal here. Sounds like it would be a win anyway.
Also, I assume in this model layout only talks to LayerDrawer?

If so, then I think we should leave LayerDrawer called LayerManager, because from layout's point of view it does everything. The fact that some of it delegates to LayerCompositor is a layers implementation detail.
The goal isn't to have LayerDrawer execute all the same instructions for every LayerCompositor (although that would be nice), but rather to have only one LayerDrawer implementation specialized as needed for compositors (use component alpha?  need double-buffering?  etc.).
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #5)
> Also, I assume in this model layout only talks to LayerDrawer?
> 

Yes and no.  Async shadow-tree manipulation still happens on the LayerCompositor's tree.  Whether or not we should call that layout is fuzzier.  FrameLayerBuilder only talks to LayerDrawer.
(In reply to Chris Jones [:cjones] [:warhammer] from comment #1)
> A potential issue with that new approach is that rendering becomes less
> friendly to CPU cache for same-thread, CPU-composited backends.  Maybe the
> effect isn't measurable, or we don't care enough about that configuration
> going forward.
> 

Another issue: we're going to break http://mxr.mozilla.org/mozilla-central/source/gfx/layers/basic/BasicLayers.cpp#674, since we'll always need to retain buffers.
Hmm. That will regress component alpha with non-GPU backends, unless we try to implement component-alpha compositing on the CPU.
That seems reasonable to me.
I'm not sure in which bug the layers refactoring is happening, but I'm going to pretend it's this one.
Duplicate of this bug: 725886
You need to log in before you can comment on or make changes to this bug.