942358 - Support MacIOSurface textures in the basic compositor

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[David Anderson [:dvander] - inactive, e-mail if emergency]

Plugin tests on Mac are failing with the basic compositor since there is no non-OGL MacIOSurfaceTextureHost.

Comment 1

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: macsurface.patch

With this patch I can visit youtube and watch stuff with Basic OMTC (non-e10s). I have no idea if it's the right approach though.

Comment 2

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: macsurface.patch

w/ missing file added

Comment 3

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8337116 [details] [diff] [review]
macsurface.patch

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

::: gfx/layers/basic/MacIOSurfaceTextureHostBasic.cpp
@@ +71,5 @@
> +
> +void
> +MacIOSurfaceTextureHostBasic::SetCompositor(Compositor* aCompositor)
> +{
> +  BasicCompositor* glCompositor = static_cast<BasicCompositor*>(aCompositor);

'glCompositor' is a poor name. And obviously stolen.

::: gfx/layers/composite/TextureHost.cpp
@@ +144,5 @@
> +        aDesc.get_SurfaceDescriptorMacIOSurface();
> +      result = new MacIOSurfaceTextureHostBasic(aID, aFlags, desc);
> +      break;
> +    }
> +#endif

These changes aren't backend independent, so don't really fit here.

Create a 'CreateTextureHostBasic' function (similar to CreateTextureHostOGL) in BasicCompositor.cpp, and have it handle this surface descriptor type. Then fallback to this function for other surface descriptor types.

Comment 4

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: v2

Comment 5

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8338071 [details] [diff] [review]
v2

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

::: gfx/layers/basic/TextureHostBasic.cpp
@@ +22,5 @@
> +#ifdef XP_MACOSX
> +    case SurfaceDescriptor::TSurfaceDescriptorMacIOSurface: {
> +      const SurfaceDescriptorMacIOSurface& desc =
> +        aDesc.get_SurfaceDescriptorMacIOSurface();
> +      result = new MacIOSurfaceTextureHostBasic(aID, aFlags, desc);

What includes MacIOSurfaceTextureHostBasic.h for this to compile?

::: gfx/layers/basic/TextureHostBasic.h
@@ +16,5 @@
> +#include "mozilla/layers/CompositorTypes.h"  // for TextureFlags
> +#include "mozilla/layers/LayersSurfaces.h"  // for SurfaceDescriptor
> +#include "mozilla/layers/TextureHost.h"  // for DeprecatedTextureHost, etc
> +#include "mozilla/mozalloc.h"           // for operator delete, etc
> +#include "mozilla/gfx/2D.h"

Do we really need all these includes?

Comment 6

[David Anderson [:dvander] - inactive, e-mail if emergency]

(In reply to Matt Woodrow (:mattwoodrow) from comment #5)
> ::: gfx/layers/basic/TextureHostBasic.h
> @@ +16,5 @@
> > +#include "mozilla/layers/CompositorTypes.h"  // for TextureFlags
> > +#include "mozilla/layers/LayersSurfaces.h"  // for SurfaceDescriptor
> > +#include "mozilla/layers/TextureHost.h"  // for DeprecatedTextureHost, etc
> > +#include "mozilla/mozalloc.h"           // for operator delete, etc
> > +#include "mozilla/gfx/2D.h"
> 
> Do we really need all these includes?

Computer says no.

https://hg.mozilla.org/integration/mozilla-inbound/rev/72698dcafb4b

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/72698dcafb4b