Closed Bug 538266 Opened 11 years ago Closed 11 years ago

Add affine transformations to Layers

Categories

(Core :: Graphics, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: bas.schouten, Assigned: roc)

References

Details

Attachments

(4 files, 4 obsolete files)

Attached patch Basic 3D Matrix (obsolete) — Splinter Review
Layers should be able to have 3D transformations applied to them.

I've included a diff for a basic 3D matrix structure to use. It includes a conversion method from a gfxMatrix and allows easy creation of some of the most basic transformation matrices.
+ * This class represents a 3D affine transformation. The matrix is layed

"laid"

Not just affine transformations, right?

I'm going to add some stuff to this patch.
Another nit is that instead of \return 3D matrix we use "@return" etc.

+  static inline gfx3DMatrix Translate(float aX, float aY, float aZ);

Let's call this "Translation" (a noun instead of a verb).

+  static inline gfx3DMatrix FromMatrix(const gfxMatrix &aMatrix);

I think this should be From2D.

I'm going to create Is2D and Get2D methods.
Also I don't think we need comments like "\return 3D matrix" which don't tell us more than what's in the signature.
Also,

  union {
    struct {
      float _11, _12, _13, _14;
      float _21, _22, _23, _24;
      float _31, _32, _33, _34;
      float _41, _42, _43, _44;
    };
    float m[4][4];
  };

Do we really need this union? Isn't m[0][0] just as readable as _11?
Also, the default constructed matrix should be the identity matrix.

I'm going to add IsIdentity().
Attached patch Part 1: gfx3DMatrix (obsolete) — Splinter Review
This is Bas's patch with a few changes.
Assignee: nobody → roc
Attached patch Part 2: add transforms (obsolete) — Splinter Review
Add transforms to Layer and implement them in BasicLayers.
We really need some kind of software support for perspective transforms here, for BasicLayers to use.

To support proper 3D we'll also need to add a mode switch to choose between
a) flattening each child of a container independently and compositing them in layer order
b) compositing all the children together with z-buffering

To implement b) we'll need a software implementation of z-buffering, for BasicLayers.

It seems to me that if every layer represents a flat plane with z=0, we could simplify the transform matrix. Does that make sense? Do we actually need to allow layers to be a full 3D scene graph, or is it OK to flatten each layer into a plane?
No longer blocks: layers
Depends on: layers
What does it mean for a matrix to be 2D?  DirectX and OpenGL have different default concepts of "up" (GL is Y-up, DX is Z-up), so at the very least there should be a comment explaining which depth dimension is being checked.  I would prefer that we stick with the GL conventions.

Also, I agree with roc that m[0][0] is better than _11 -- mainly because with _11, I wonder whether it's 0-based or 1-based indexing.  Do we store the matrix as row-major because cairo's matrices are row-major? (I can't even remember if they were...)  That's going to be annoying for OpenGL, but we can deal; we should make sure we do the same as the current gfxMatrix class, though.


> To support proper 3D we'll also need to add a mode switch to choose between
> a) flattening each child of a container independently and compositing them in
> layer order
> b) compositing all the children together with z-buffering

This sounds like two different layer group types, or at least a mode switch on the layer group with the default being.. hm, probably a?  Is that what you're thinking by 'mode switch'?

> It seems to me that if every layer represents a flat plane with z=0, we could
> simplify the transform matrix. Does that make sense? Do we actually need to
> allow layers to be a full 3D scene graph, or is it OK to flatten each layer
> into a plane?

Let's not remove the generality unless we really need to... we have to deal with full 16-element matrices for the 3D APIs anyway.
(In reply to comment #9)
> What does it mean for a matrix to be 2D?  DirectX and OpenGL have different
> default concepts of "up" (GL is Y-up, DX is Z-up), so at the very least there
> should be a comment explaining which depth dimension is being checked.  I
> would prefer that we stick with the GL conventions.

I'm not sure. I'm just following the code that Bas wrote for From2D.

> > To support proper 3D we'll also need to add a mode switch to choose between
> > a) flattening each child of a container independently and compositing them in
> > layer order
> > b) compositing all the children together with z-buffering
> 
> This sounds like two different layer group types, or at least a mode switch on
> the layer group with the default being.. hm, probably a?  Is that what you're
> thinking by 'mode switch'?

I'm thinking of a flag on the container layer, defaulting to a) (since (a) is what we need for everything except certain kinds of CSS 3D transforms, and it's more efficient).
(In reply to comment #9)
> What does it mean for a matrix to be 2D?  DirectX and OpenGL have different
> default concepts of "up" (GL is Y-up, DX is Z-up), so at the very least there
> should be a comment explaining which depth dimension is being checked.  I would
> prefer that we stick with the GL conventions.
> 
> Also, I agree with roc that m[0][0] is better than _11 -- mainly because with
> _11, I wonder whether it's 0-based or 1-based indexing.  Do we store the matrix
> as row-major because cairo's matrices are row-major? (I can't even remember if
> they were...)  That's going to be annoying for OpenGL, but we can deal; we
> should make sure we do the same as the current gfxMatrix class, though.
> 
> 
> > To support proper 3D we'll also need to add a mode switch to choose between
> > a) flattening each child of a container independently and compositing them in
> > layer order
> > b) compositing all the children together with z-buffering
> 
> This sounds like two different layer group types, or at least a mode switch on
> the layer group with the default being.. hm, probably a?  Is that what you're
> thinking by 'mode switch'?
> 
> > It seems to me that if every layer represents a flat plane with z=0, we could
> > simplify the transform matrix. Does that make sense? Do we actually need to
> > allow layers to be a full 3D scene graph, or is it OK to flatten each layer
> > into a plane?
> 
> Let's not remove the generality unless we really need to... we have to deal
> with full 16-element matrices for the 3D APIs anyway.
The 3D Matrix is row-major. So is our 2D Matrix. And indeed, I also think we should not remove the generality, first of all because this means the Matrices can go straight into the 3D APIs, as Vlad says. And second because it means we can use these internally to do projections.

Projecting 3D transformations and getting the right Z-value on the final vector in viewport space for Z-buffering require the full matrix for a bunch of reasons. If you would just 'shear' the layers as 2D planes, you're right, but we need to know which point of a layer is behind another, as Roc correctly stated.

Finally, the union, I'm fine with removing that, it's just something most 3D libraries do. And the other reason is I preferred m[i][j] while 2D Matrix does xx, xy, etc. So I figured I should support both.
(In reply to comment #9)
> What does it mean for a matrix to be 2D?  DirectX and OpenGL have different
> default concepts of "up" (GL is Y-up, DX is Z-up), so at the very least there
> should be a comment explaining which depth dimension is being checked.  I would
> prefer that we stick with the GL conventions.

DX doesn't specify any concept of 'up' really, as far as I know, it just depends on how you lay out your projection matrix. Although yes, if you use the D3DX utility functions most default projection matrices will consider Z-up. On the other hand 'Z-buffering' which DX also calls it and the way its transformation pipeline is setup will work based on Z, so that suggests the Z axis runs into the screen.

I wrote XY to be the 2D plane and Z to be depth. The main reason for this is I don't want to create an axis flip when going from 2D to 3D, I think it would make things horribly confusing in the long run. Therefor, I suggest Y is up as it is now.
(In reply to comment #9)
> _11, I wonder whether it's 0-based or 1-based indexing.  Do we store the matrix
> as row-major because cairo's matrices are row-major? (I can't even remember if
> they were...)  That's going to be annoying for OpenGL, but we can deal; we
> should make sure we do the same as the current gfxMatrix class, though.

Right, one more thing I forgot to reply to! Sorry for the spam :-).

It's actually not going to be that annoying for OpenGL, as OpenGL seems to store its columns in the matrix in a different direction (i.e. m[j][i]) I suspect the reason for this is that otherwise the memory layout wouldn't allow using a SIMD dot-product function. (i.e. if you need to multiply a vector with a column you need that column to be tightly packed in memory). Therefor the matrix transposition would happen automatically on letting OGL read the matrix from memory I think.
(In reply to comment #13)
> store its columns in the matrix in a different direction (i.e. m[j][i]) I
> suspect the reason for this is that otherwise the memory layout wouldn't allow
> using a SIMD dot-product function. (i.e. if you need to multiply a vector with
> a column you need that column to be tightly packed in memory). Therefor the
> matrix transposition would happen automatically on letting OGL read the matrix
> from memory I think.

Actually, now that I think about it, that's not at all true, since D3D uses row vectors, and OGL using column vectors. Since column vectors take the dot-product with the row in the matrix, and row vectors take the dot-product with the column in the matrix, actually neither OGL or D3Ds align nicely for SIMD dot-products. In other words I actually have no idea why OGL is using column-major. But OGL also considers vectors as column vectors, so the double transposition still means we shouldn't have any trouble.
(In reply to comment #10)
> (In reply to comment #9)
> > > To support proper 3D we'll also need to add a mode switch to choose between
> > > a) flattening each child of a container independently and compositing them in
> > > layer order
> > > b) compositing all the children together with z-buffering
> > 
> > This sounds like two different layer group types, or at least a mode switch on
> > the layer group with the default being.. hm, probably a?  Is that what you're
> > thinking by 'mode switch'?
> 
> I'm thinking of a flag on the container layer, defaulting to a) (since (a) is
> what we need for everything except certain kinds of CSS 3D transforms, and it's
> more efficient).
Why are there fundamentally different? I'm not sure I understand, if we apply any transformation where the third column (assuming we use row vectors :-)) is equal to 0 0 1 0, we would never transform the Z-component of the layer. And we wouldn't need to do anything specific to flatten it.

Or we thinking about the optimization of avoiding Z-buffering? In which case I think analyzing the transformation matrix for anything affecting the Z-component works as well as a flag on the layer?
If you do the naive hardware thing, are you guaranteed that wherever two layers have the same z-value after transformation, the rendering is in layer order?

Analyzing the transformation matrix might be sufficient instead of a flag, but a flag might be simpler.
(In reply to comment #16)
> If you do the naive hardware thing, are you guaranteed that wherever two layers
> have the same z-value after transformation, the rendering is in layer order?
> 
> Analyzing the transformation matrix might be sufficient instead of a flag, but
> a flag might be simpler.
Yeah. I'm fine with either. As for the hardware thing, with most z-tests it should draw with an equal z value I think. If we disable using the z-buffer for those layers only 2D transformed they will always be drawn in order.
Linux GCC didn't like the anonymous struct, so I just removed the 'm' stuff, since we weren't really using it.
Attachment #420480 - Attachment is obsolete: true
Attachment #425095 - Flags: review?(jmuizelaar)
Attachment #420481 - Attachment is obsolete: true
Attachment #425096 - Flags: superreview?(dbaron)
Attachment #425096 - Flags: review?(jmuizelaar)
It seems the Matrix members accidentally became protected. I don't think they should be, it's probably best to just have them public.
Yes, I did that without thinking enough. I'll make them public.
Comment on attachment 425095 [details] [diff] [review]
Part 1: gfx3DMatrix

>Bug 538266. Create gfx3DMatrix. r=jrmuizel
>
>diff --git a/gfx/thebes/public/Makefile.in b/gfx/thebes/public/Makefile.in
>--- a/gfx/thebes/public/Makefile.in
>+++ b/gfx/thebes/public/Makefile.in
>@@ -4,17 +4,18 @@ topsrcdir	= @top_srcdir@
> srcdir		= @srcdir@
> VPATH		= @srcdir@
> 
> include $(DEPTH)/config/autoconf.mk
> 
> MODULE		= thebes
> 
> 
>-EXPORTS		= 	gfxASurface.h \
>+EXPORTS		= 	gfx3DMatrix.h \
>+			gfxASurface.h \
> 			gfxAlphaRecovery.h \
> 			gfxBlur.h \
> 			gfxColor.h \
> 			gfxContext.h \
> 			gfxFont.h \
> 			gfxFontConstants.h \
> 			gfxFontUtils.h \
> 			gfxImageSurface.h \
>diff --git a/gfx/thebes/public/gfx3DMatrix.h b/gfx/thebes/public/gfx3DMatrix.h
>new file mode 100644
>--- /dev/null
>+++ b/gfx/thebes/public/gfx3DMatrix.h
>@@ -0,0 +1,195 @@
>+/* -*- Mode: C++; tab-width: 20; indent-tabs-mode: nil; c-basic-offset: 4 -*-
>+ * ***** BEGIN LICENSE BLOCK *****
>+ * Version: MPL 1.1/GPL 2.0/LGPL 2.1
>+ *
>+ * The contents of this file are subject to the Mozilla Public License Version
>+ * 1.1 (the "License"); you may not use this file except in compliance with
>+ * the License. You may obtain a copy of the License at
>+ * http://www.mozilla.org/MPL/
>+ *
>+ * Software distributed under the License is distributed on an "AS IS" basis,
>+ * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
>+ * for the specific language governing rights and limitations under the
>+ * License.
>+ *
>+ * The Original Code is Mozilla Corporation code.
>+ *
>+ * The Initial Developer of the Original Code is Oracle Corporation.
>+ * Portions created by the Initial Developer are Copyright (C) 2005
>+ * the Initial Developer. All Rights Reserved.
>+ *
>+ * Contributor(s):
>+ *   Bas Schouten <bschouten@mozilla.com
>+ *
>+ * Alternatively, the contents of this file may be used under the terms of
>+ * either the GNU General Public License Version 2 or later (the "GPL"), or
>+ * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
>+ * in which case the provisions of the GPL or the LGPL are applicable instead
>+ * of those above. If you wish to allow use of your version of this file only
>+ * under the terms of either the GPL or the LGPL, and not to allow others to
>+ * use your version of this file under the terms of the MPL, indicate your
>+ * decision by deleting the provisions above and replace them with the notice
>+ * and other provisions required by the GPL or the LGPL. If you do not delete
>+ * the provisions above, a recipient may use your version of this file under
>+ * the terms of any one of the MPL, the GPL or the LGPL.
>+ *
>+ * ***** END LICENSE BLOCK ***** */
>+
>+#ifndef GFX_3DMATRIX_H
>+#define GFX_3DMATRIX_H
>+
>+#include <gfxTypes.h>
>+#include <gfxMatrix.h>
>+#include <math.h>
>+
>+/**
>+ * This class represents a 3D transformation. The matrix is laid
>+ * out as follows:
>+ *
>+ * _11 _12 _13 _14
>+ * _21 _22 _23 _24
>+ * _31 _32 _33 _34
>+ * _41 _42 _43 _44
>+ *
>+ * This matrix is treated as row-major.

It would be nice to include some rationale for the decision to use row-major.

>+ */
>+class THEBES_API gfx3DMatrix
>+{

[snip]

>+};
>+
>+inline
>+gfx3DMatrix::gfx3DMatrix(void)
>+{
>+  _11 = _22 = _33 = _44 = 1.0f;
>+  _12 = _13 = _14 = 0.0f;
>+  _21 = _23 = _24 = 0.0f;
>+  _31 = _32 = _34 = 0.0f;
>+  _41 = _42 = _43 = 0.0f;
>+}

There's some inconsistency in the type/style of constants use for initialization in this file. Doesn't bother me much though.

>+
>+inline gfx3DMatrix
>+gfx3DMatrix::operator*(const gfx3DMatrix &aMatrix)
>+{
>+  gfx3DMatrix matrix;
>+
>+  matrix._11 = _11 * aMatrix._11 + _12 * aMatrix._21 + _13 * aMatrix._31 + _14 * aMatrix._41;
>+  matrix._21 = _21 * aMatrix._11 + _22 * aMatrix._21 + _23 * aMatrix._31 + _24 * aMatrix._41;
>+  matrix._31 = _31 * aMatrix._11 + _32 * aMatrix._21 + _33 * aMatrix._31 + _34 * aMatrix._41;
>+  matrix._41 = _41 * aMatrix._11 + _42 * aMatrix._21 + _43 * aMatrix._31 + _44 * aMatrix._41;
>+  matrix._12 = _11 * aMatrix._12 + _12 * aMatrix._22 + _13 * aMatrix._32 + _14 * aMatrix._42;
>+  matrix._22 = _21 * aMatrix._12 + _22 * aMatrix._22 + _23 * aMatrix._32 + _24 * aMatrix._42;
>+  matrix._32 = _31 * aMatrix._12 + _32 * aMatrix._22 + _33 * aMatrix._32 + _34 * aMatrix._42;
>+  matrix._42 = _41 * aMatrix._12 + _42 * aMatrix._22 + _43 * aMatrix._32 + _44 * aMatrix._42;
>+  matrix._13 = _11 * aMatrix._13 + _12 * aMatrix._23 + _13 * aMatrix._33 + _14 * aMatrix._43;
>+  matrix._23 = _21 * aMatrix._13 + _22 * aMatrix._23 + _23 * aMatrix._33 + _24 * aMatrix._43;
>+  matrix._33 = _31 * aMatrix._13 + _32 * aMatrix._23 + _33 * aMatrix._33 + _34 * aMatrix._43;
>+  matrix._43 = _41 * aMatrix._13 + _42 * aMatrix._23 + _43 * aMatrix._33 + _44 * aMatrix._43;
>+  matrix._14 = _11 * aMatrix._14 + _12 * aMatrix._24 + _13 * aMatrix._34 + _14 * aMatrix._44;
>+  matrix._24 = _21 * aMatrix._14 + _22 * aMatrix._24 + _23 * aMatrix._34 + _24 * aMatrix._44;
>+  matrix._34 = _31 * aMatrix._14 + _32 * aMatrix._24 + _33 * aMatrix._34 + _34 * aMatrix._44;
>+  matrix._44 = _41 * aMatrix._14 + _42 * aMatrix._24 + _43 * aMatrix._34 + _44 * aMatrix._44;
>+
>+  return matrix;
>+}

I wonder if gcc vectorizes this function...

>+
>+inline gfx3DMatrix
>+gfx3DMatrix::From2D(const gfxMatrix &aMatrix)
>+{
>+  gfx3DMatrix matrix;
>+  matrix._11 = (float)aMatrix.xx;
>+  matrix._12 = (float)aMatrix.yx;
>+  matrix._21 = (float)aMatrix.xy;
>+  matrix._22 = (float)aMatrix.yy;
>+  matrix._41 = (float)aMatrix.x0;
>+  matrix._42 = (float)aMatrix.y0;
>+  matrix._33 = matrix._44 = 1;

why reinitialize _33 and _44 but not any of the other unset members? 

>+  return matrix;
>+}
>+
>+inline PRBool
>+gfx3DMatrix::IsIdentity() const
>+{
>+  return _11 == 1.0f && _12 == 0.0f && _13 == 0.0f && _14 == 0.0f &&
>+         _21 == 0.0f && _22 == 1.0f && _23 == 0.0f && _24 == 0.0f &&
>+         _31 == 0.0f && _32 == 0.0f && _33 == 1.0f && _34 == 0.0f &&
>+         _41 == 0.0f && _42 == 0.0f && _43 == 0.0f && _44 == 1.0f;
>+}

What's the intended use of this function? Should the floating point comparisons be less exact?

>+
>+inline gfx3DMatrix
>+gfx3DMatrix::Translation(float aX, float aY, float aZ)
>+{
>+  gfx3DMatrix matrix;
>+
>+  matrix._11 = 1;
>+  matrix._22 = 1;
>+  matrix._33 = 1;
>+  matrix._41 = aX;
>+  matrix._42 = aY;
>+  matrix._43 = aZ;
>+  matrix._44 = 1;
>+  return matrix;
>+}
>+
>+inline gfx3DMatrix
>+gfx3DMatrix::Scale(float aFactor)
>+{
>+  gfx3DMatrix matrix;
>+
>+  matrix._11 = matrix._22 = matrix._33 = aFactor;
>+  matrix._44 = 1;
>+  return matrix;
>+}

These two functions have inconsistent initialization again...
Attachment #425095 - Flags: review?(jmuizelaar) → review+
(In reply to comment #24)
> It would be nice to include some rationale for the decision to use row-major.

Sure. Uh Bas, what is the reason? :-)

> >+};
> >+
> >+inline
> >+gfx3DMatrix::gfx3DMatrix(void)
> >+{
> >+  _11 = _22 = _33 = _44 = 1.0f;
> >+  _12 = _13 = _14 = 0.0f;
> >+  _21 = _23 = _24 = 0.0f;
> >+  _31 = _32 = _34 = 0.0f;
> >+  _41 = _42 = _43 = 0.0f;
> >+}
> 
> There's some inconsistency in the type/style of constants use for
> initialization in this file. Doesn't bother me much though.

I'll fix them all to use f.

> >+inline gfx3DMatrix
> >+gfx3DMatrix::From2D(const gfxMatrix &aMatrix)
> >+{
> >+  gfx3DMatrix matrix;
> >+  matrix._11 = (float)aMatrix.xx;
> >+  matrix._12 = (float)aMatrix.yx;
> >+  matrix._21 = (float)aMatrix.xy;
> >+  matrix._22 = (float)aMatrix.yy;
> >+  matrix._41 = (float)aMatrix.x0;
> >+  matrix._42 = (float)aMatrix.y0;
> >+  matrix._33 = matrix._44 = 1;
> 
> why reinitialize _33 and _44 but not any of the other unset members?

I'll remove the last line.

> >+inline PRBool
> >+gfx3DMatrix::IsIdentity() const
> >+{
> >+  return _11 == 1.0f && _12 == 0.0f && _13 == 0.0f && _14 == 0.0f &&
> >+         _21 == 0.0f && _22 == 1.0f && _23 == 0.0f && _24 == 0.0f &&
> >+         _31 == 0.0f && _32 == 0.0f && _33 == 1.0f && _34 == 0.0f &&
> >+         _41 == 0.0f && _42 == 0.0f && _43 == 0.0f && _44 == 1.0f;
> >+}
> 
> What's the intended use of this function? Should the floating point comparisons
> be less exact?

They're OK as-is. I use this in part 2 as follows:

 // Returns true if we need to save the state of the gfxContext when
 // we start painting aLayer (and restore the state when we've finished
 // painting aLayer)
 static PRBool
 NeedsState(Layer* aLayer)
 {
-  return aLayer->GetClipRect() != nsnull;
+  return aLayer->GetClipRect() != nsnull ||
+         !aLayer->GetTransform().IsIdentity();
 }

The only important property right now is that gfx3DMatrix().IsIdentity() returns true, so that if you don't set the transform or clip rect on a layer, we don't need to save the gfxContext state.
(In reply to comment #24)
> (From update of attachment 425095 [details] [diff] [review])
> >+ * This matrix is treated as row-major.
> 
> It would be nice to include some rationale for the decision to use row-major.
> 

Assuming we consider our vectors row vectors, this matrix type will be identical in memory to the OpenGL and D3D matrices. OpenGL matrices are column-major, however OpenGL also treats vectors as column vectors, the double transposition makes everything work out nicely.

I also believe it to be a more intuitive memory layout. If you looked at this as an array from a C++ point of view (say matrix[4][4]), generally one would expect matrix[i][j] i to be the row and j to be the column. Common mathematical understanding would assume i to be the row. Which in this case it would be. That might very well be personal though :-).
(In reply to comment #25)
> (In reply to comment #24)
> > What's the intended use of this function? Should the floating point comparisons
> > be less exact?
> 
> They're OK as-is. I use this in part 2 as follows:
> 
>  // Returns true if we need to save the state of the gfxContext when
>  // we start painting aLayer (and restore the state when we've finished
>  // painting aLayer)
>  static PRBool
>  NeedsState(Layer* aLayer)
>  {
> -  return aLayer->GetClipRect() != nsnull;
> +  return aLayer->GetClipRect() != nsnull ||
> +         !aLayer->GetTransform().IsIdentity();
>  }
> 
> The only important property right now is that gfx3DMatrix().IsIdentity()
> returns true, so that if you don't set the transform or clip rect on a layer,
> we don't need to save the gfxContext state.

Putting something like this in a comment would be wonderful.
Attachment #425096 - Flags: review?(jmuizelaar) → review+
Comment on attachment 425097 [details] [diff] [review]
Part 3: move GfxRectToIntRect to nsLayoutUtils

r=mats
Attachment #425097 - Flags: review?(matspal) → review+
Updated patch so that a layer's cliprect is applied after its transform.
Attachment #425096 - Attachment is obsolete: true
Attachment #427054 - Flags: superreview?(dbaron)
Attachment #425096 - Flags: superreview?(dbaron)
Blocks: 546508
Comment on attachment 427054 [details] [diff] [review]
Part 2: add transforms to layer API (v2)

sr=dbaron
Attachment #427054 - Flags: superreview?(dbaron) → superreview+
Comment on attachment 425099 [details] [diff] [review]
Part 4: fix visible-region calculation to account for transforms

r=dbaron
Attachment #425099 - Flags: review?(dbaron) → review+
Whiteboard: [needs landing]
Layers patches are not compiling anymore after landing 503989

error: nsISupports is an ambiguous base of nsHtmlMediaElement....
You need to log in before you can comment on or make changes to this bug.