Closed Bug 942358 Opened 6 years ago Closed 6 years ago

Support MacIOSurface textures in the basic compositor

Categories

(Core :: Graphics: Layers, defect)

All
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla28
Tracking Status
firefox28 --- fixed

People

(Reporter: dvander, Assigned: dvander)

References

Details

(Whiteboard: [qa-])

Attachments

(1 file, 2 obsolete files)

Plugin tests on Mac are failing with the basic compositor since there is no non-OGL MacIOSurfaceTextureHost.
Attached patch macsurface.patch (obsolete) — Splinter Review
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.
Assignee: nobody → dvander
Attachment #8337115 - Flags: review?(matt.woodrow)
Attached patch macsurface.patch (obsolete) — Splinter Review
w/ missing file added
Attachment #8337115 - Attachment is obsolete: true
Attachment #8337115 - Flags: review?(matt.woodrow)
Attachment #8337116 - Flags: review?(matt.woodrow)
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.
Attachment #8337116 - Flags: review?(matt.woodrow) → review+
Attached patch v2Splinter Review
Attachment #8337116 - Attachment is obsolete: true
Attachment #8338071 - Flags: review?(matt.woodrow)
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?
Attachment #8338071 - Flags: review?(matt.woodrow) → review+
(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
https://hg.mozilla.org/mozilla-central/rev/72698dcafb4b
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Flags: in-testsuite?
Whiteboard: [qa-]
Depends on: 1236359
Depends on: 1241665
You need to log in before you can comment on or make changes to this bug.