We need to support component-alpha thebeslayers in Azure.

This creates an efficient method of filling the child textures. It's independent of the drawing method used and does both surfaces from a single drawing call, most likely improving performance a little in the progress.

Comment on attachment 610887 [details] [diff] [review] Part 1: Add DrawTargetDual for drawing to two drawtargets Review of attachment 610887 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/2d/DrawTargetDual.cpp @@ +61,5 @@ > + SourceSurface *mB; > +}; > + > + > +class DualPattern Add a comment someplace about why we only need to have separate patterns for Surface patterns ::: gfx/2d/DrawTargetDual.h @@ +51,5 @@ > +#define FORWARD_FUNCTION(funcName) \ > + virtual void funcName() { mA->funcName(); mB->funcName(); } > +#define FORWARD_FUNCTION1(funcName, var1Type, var1Name) \ > + virtual void funcName(var1Type var1Name) { mA->funcName(var1Name); mB->funcName(var1Name); } > + It's probably worth adding a description of what the semantics of this draw target are, especially any tricky cases.

Comment on attachment 610889 [details] [diff] [review] Part 2: Generalize filltextures This is useful non-azure as well correct?

Comment on attachment 610889 [details] [diff] [review] Part 2: Generalize filltextures Review of attachment 610889 [details] [diff] [review]: ----------------------------------------------------------------- Nothing objectionable jumps out at me. ::: gfx/layers/d3d10/ThebesLayerD3D10.cpp @@ +394,5 @@ > + projection._22 = -2.0f / desc.Height; > + projection._33 = 0.0f; > + projection._41 = -1.0f; > + projection._42 = 1.0f; > + projection._44 = 1.0f; It would be nice if this setup code could be made a bit briefer. I don't know if there's anything we can share with code elsewhere. Alternatively, if it's not too much pain you could break it out into a subfunction. ::: gfx/layers/d3d10/ThebesLayerD3D10.h @@ +97,5 @@ > /* Create a new texture */ > void CreateNewTextures(const gfxIntSize &aSize, SurfaceMode aMode); > > + // Fill textures with opaque black and white in the specified region. > + void FillTextures(const nsIntRegion& aRegion, const nsIntPoint& aOffset); I think this needs a better name than FillTextures that makes it more obvious that it's doing black and white.

Comment on attachment 610890 [details] [diff] [review] Part 3: Use component alpha rendering when drawing content with azure Review of attachment 610890 [details] [diff] [review]: ----------------------------------------------------------------- It feels a bit like these changes are pretty intertwined and not that well separated out. I don't know if there's much we can do to fix that for now. ::: gfx/layers/d3d10/ThebesLayerD3D10.cpp @@ +482,5 @@ > context->NewPath(); > const nsIntRect *iterRect; > while ((iterRect = iter.Next())) { > context->Rectangle(gfxRect(iterRect->x, iterRect->y, iterRect->width, iterRect->height)); > + if (mDrawTarget && aMode == SURFACE_SINGLE_CHANNEL_ALPHA) { I don't understand why this change is here. @@ +570,5 @@ > + mDrawTarget = nsnull; > + } > + } > + > + if (gfxPlatform::UseAzureContentDrawing() && !mDrawTarget) { Shouldn't this be an else if on the condition above?

(In reply to Jeff Muizelaar [:jrmuizel] from comment #7) > Comment on attachment 610890 [details] [diff] [review] > Part 3: Use component alpha rendering when drawing content with azure > > Review of attachment 610890 [details] [diff] [review]: > ----------------------------------------------------------------- > > It feels a bit like these changes are pretty intertwined and not that well > separated out. I don't know if there's much we can do to fix that for now. > > ::: gfx/layers/d3d10/ThebesLayerD3D10.cpp > @@ +482,5 @@ > > context->NewPath(); > > const nsIntRect *iterRect; > > while ((iterRect = iter.Next())) { > > context->Rectangle(gfxRect(iterRect->x, iterRect->y, iterRect->width, iterRect->height)); > > + if (mDrawTarget && aMode == SURFACE_SINGLE_CHANNEL_ALPHA) { > > I don't understand why this change is here. This prevents needless clearing and prevents us clearing out the white we just used to fill the DrawTarget. It's an improvement and it creates consistency with non-Azure behavior lower down.

(In reply to Jeff Muizelaar [:jrmuizel] from comment #7) > Comment on attachment 610890 [details] [diff] [review] > Part 3: Use component alpha rendering when drawing content with azure > > Review of attachment 610890 [details] [diff] [review]: > ----------------------------------------------------------------- > @@ +570,5 @@ > > + mDrawTarget = nsnull; > > + } > > + } > > + > > + if (gfxPlatform::UseAzureContentDrawing() && !mDrawTarget) { > > Shouldn't this be an else if on the condition above? No, it shouldn't, the idea here is in the Azure world we don't keep track of our DrawTargets separately so it always creates its drawtarget at the end of the function when both textures are prepared and ready. This is, sadly, different from the cairo situation.

https://hg.mozilla.org/integration/mozilla-inbound/rev/3dd886d95c55 https://hg.mozilla.org/integration/mozilla-inbound/rev/e36fabc31211 https://hg.mozilla.org/integration/mozilla-inbound/rev/9fa58c6060c5

https://hg.mozilla.org/mozilla-central/rev/3dd886d95c55 https://hg.mozilla.org/mozilla-central/rev/e36fabc31211 https://hg.mozilla.org/mozilla-central/rev/9fa58c6060c5