Finish the OMTC implementation on OS X

RESOLVED FIXED in mozilla27

Status

()

Core
Graphics: Layers
RESOLVED FIXED
5 years ago
a year ago

People

(Reporter: Joe Drew (not getting mail), Assigned: mattwoodrow)

Tracking

(Blocks: 1 bug)

Trunk
mozilla27
x86
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(relnote-firefox 24+)

Details

Attachments

(12 attachments, 2 obsolete attachments)

2.47 KB, patch
bas
: review+
Details | Diff | Splinter Review
8.11 KB, patch
BenWa
: review+
Details | Diff | Splinter Review
3.03 KB, patch
jrmuizel
: review+
Details | Diff | Splinter Review
1.82 KB, patch
bas
: review+
Details | Diff | Splinter Review
1.16 KB, patch
roc
: review+
Details | Diff | Splinter Review
1.92 KB, patch
roc
: review+
Details | Diff | Splinter Review
1.49 KB, patch
roc
: review+
Details | Diff | Splinter Review
9.13 KB, patch
roc
: review+
Details | Diff | Splinter Review
3.34 KB, patch
bas
: review+
Details | Diff | Splinter Review
7.92 KB, patch
mstange
: review+
Details | Diff | Splinter Review
5.46 KB, patch
roc
: review+
Details | Diff | Splinter Review
10.81 KB, patch
roc
: review+
Details | Diff | Splinter Review
(Reporter)

Description

5 years ago
Benoit Girard and Ali Juma did a lot of the initial work in implementing OMTC on OS X, but it's not done yet. This bug will track the work required to finish it.
(Reporter)

Updated

5 years ago
Blocks: 598873
Just a quick overview of what I think is needed before we can ship it without regressing:

1) Draw with cairo+quartz instead of cairo+pixman.
2) Texture sharing (bug 728524)
3) Either direct plugin (best), or add proper paths to shadow iosurfaces (easy).
Depends on: 728524
No longer blocks: 598873
Blocks: 598873

Updated

5 years ago
Depends on: 796182
(Assignee)

Updated

5 years ago
Depends on: 808469
(Assignee)

Updated

5 years ago
Depends on: 809273
(Assignee)

Updated

5 years ago
Depends on: 811439
(Assignee)

Updated

4 years ago
Depends on: 860615

Updated

4 years ago
Depends on: 861490

Updated

4 years ago
Depends on: 862556
(Assignee)

Updated

4 years ago
Depends on: 867474
(Assignee)

Updated

4 years ago
Depends on: 868919
(Assignee)

Comment 2

4 years ago
Created attachment 745793 [details] [diff] [review]
Use the right texture target when allocating textures
Attachment #745793 - Flags: review?
(Assignee)

Updated

4 years ago
Attachment #745793 - Flags: review? → review?(bas)
(Assignee)

Comment 3

4 years ago
Created attachment 745794 [details] [diff] [review]
Use TextureImageCGL again, and used TiledTexturedImage for large textures

Looks like the refactoring actually broke our usage of TextureImageCGL, and we've been using BasicTextureImage instead. Bit sad that talos didn't pick this up.
Attachment #745794 - Flags: review?(bgirard)
(Assignee)

Comment 4

4 years ago
CC'ing Jeff and Joe for the sad in comment 3.
(Assignee)

Comment 5

4 years ago
Created attachment 745797 [details] [diff] [review]
Setup our OpenGL surface before compositing.

This fixes a bunch of errors we were getting with MOZ_GL_DEBUG
Attachment #745797 - Flags: review?(bgirard)
(Assignee)

Comment 6

4 years ago
Created attachment 745798 [details] [diff] [review]
Record memory usage for gfxQuartzSurface

I forget exactly why I needed to add these, but it fixed a warning of some sort.
Attachment #745798 - Flags: review?(jmuizelaar)
(Assignee)

Comment 7

4 years ago
Created attachment 745799 [details] [diff] [review]
Round texture coords to the nearest texel

Oh floating point...
Attachment #745799 - Flags: review?(bas)
(Assignee)

Comment 8

4 years ago
Created attachment 745800 [details] [diff] [review]
Use CreateThebesSurfaceAliasForDrawTarget_hack in CopyableCanvasLayer

This just matches what we have to do in the other impl's.
Attachment #745800 - Flags: review?(roc)
(Assignee)

Comment 9

4 years ago
Created attachment 745801 [details] [diff] [review]
Setup PaintWillResample correctly

Nothing was actually calling this function. This should match what the client side set.
Attachment #745801 - Flags: review?(roc)
(Assignee)

Comment 10

4 years ago
Created attachment 745802 [details] [diff] [review]
Call PaintWindow so we get a DidPaint

Adds this call back so that we get a DidPaintWindow call. The actual paint attempt will be a no-op.

It might be nice to stop using the widget events entirely and instead fire WillPaint/DidPaint around the refresh driver painting, and fire MozAfterPaint when the compositor has completed. Might leave that for now though.
Attachment #745802 - Flags: review?(roc)
(Assignee)

Comment 11

4 years ago
Created attachment 745803 [details] [diff] [review]
Send the values of mScaleToSize/mScaleMode across IPC
Attachment #745803 - Flags: review?(roc)
(Assignee)

Comment 12

4 years ago
Created attachment 745804 [details] [diff] [review]
Various fixes to small bugs
Attachment #745804 - Flags: review?(bas)
(Assignee)

Comment 13

4 years ago
With these patches the only failures left are a 10.6-only crash inside the OpenGL driver, and bug 868919.

With the former, we might need to delay enabling OMTC on that platform until we figure out.

Updated

4 years ago
Summary: [Meta] Finish the OMTC implementation on OS X → Finish the OMTC implementation on OS X
Comment on attachment 745793 [details] [diff] [review]
Use the right texture target when allocating textures

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

I seem to distinctly recall this wasn't what we wanted for one of the Gralloc things.. But hopefully I'm mistaking, testing should prove.
Attachment #745793 - Flags: review?(bas) → review+
Attachment #745799 - Flags: review?(bas) → review+
Attachment #745804 - Flags: review?(bas) → review+
Attachment #745798 - Flags: review?(jmuizelaar) → review+
Attachment #745800 - Flags: review?(roc) → review+
Comment on attachment 745801 [details] [diff] [review]
Setup PaintWillResample correctly

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

I'm confused. Where's the implementation of SetPaintWillResample that actually does something?
Comment on attachment 745802 [details] [diff] [review]
Call PaintWindow so we get a DidPaint

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

What ensures that PaintWindow doesn't actually paint anything?
Comment on attachment 745803 [details] [diff] [review]
Send the values of mScaleToSize/mScaleMode across IPC

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

::: gfx/layers/ImageLayers.h
@@ +56,5 @@
>    void SetScaleToSize(const gfxIntSize &aSize, ScaleMode aMode)
>    {
>      mScaleToSize = aSize;
>      mScaleMode = aMode;
> +    Mutated();

Please make this "Mutated()" conditional on size/mode changing.
Attachment #745803 - Flags: review?(roc) → review+
(Assignee)

Comment 18

4 years ago
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #15)
> Comment on attachment 745801 [details] [diff] [review]
> Setup PaintWillResample correctly
> 
> Review of attachment 745801 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I'm confused. Where's the implementation of SetPaintWillResample that
> actually does something?

http://mxr.mozilla.org/mozilla-central/source/gfx/layers/composite/ContentHost.h#102

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #16)
> Comment on attachment 745802 [details] [diff] [review]
> Call PaintWindow so we get a DidPaint
> 
> Review of attachment 745802 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> What ensures that PaintWindow doesn't actually paint anything?

http://mxr.mozilla.org/mozilla-central/source/layout/base/nsPresShell.cpp#5504
(Assignee)

Comment 19

4 years ago
(In reply to Bas Schouten (:bas.schouten) from comment #14)
> Comment on attachment 745793 [details] [diff] [review]
> Use the right texture target when allocating textures
> 
> Review of attachment 745793 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I seem to distinctly recall this wasn't what we wanted for one of the
> Gralloc things.. But hopefully I'm mistaking, testing should prove.

Indeed, you wrote the patch that fixed b2g, which is why I asked you to review :)

The GL_TEXTURE_EXTERNAL branch inside MakeTextureIfNeeded should preserve the existing behaviour for b2g, while fixing it for mac.
Comment on attachment 745801 [details] [diff] [review]
Setup PaintWillResample correctly

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

Mark ContentHostBase::SetPaintWillResample explicitly virtual.
Attachment #745801 - Flags: review?(roc) → review+
Attachment #745802 - Flags: review?(roc) → review+
(Assignee)

Updated

4 years ago
Attachment #745797 - Flags: review?(bgirard) → review?(mstange)
Comment on attachment 745797 [details] [diff] [review]
Setup our OpenGL surface before compositing.

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

::: gfx/layers/composite/LayerManagerComposite.cpp
@@ +295,5 @@
>    // Allow widget to render a custom background.
> +  mCompositor->GetWidget()->DrawWindowUnderlay(this, nsIntRect(bounds.x,
> +                                                               bounds.y,
> +                                                               bounds.width,
> +                                                               bounds.height));

Why?

::: widget/cocoa/nsChildView.mm
@@ +1873,5 @@
> +nsChildView::PreRender(LayerManager* aManager)
> +{
> +  nsAutoPtr<GLManager> manager(GLManager::CreateGLManager(aManager));
> +  NSOpenGLContext *glContext = (NSOpenGLContext *)manager->gl()->GetNativeData(GLContext::NativeGLContext);
> +  [(ChildView*)mView preRender: glContext];

Local style is no space after colon.

::: widget/nsIWidget.h
@@ +1169,5 @@
>       */
>      virtual void CleanupWindowEffects() = 0;
>  
> +    virtual void PreRender(LayerManager* aManager) = 0;
> +

needs IID change
(In reply to Markus Stange from comment #21)
> Comment on attachment 745797 [details] [diff] [review]
> Setup our OpenGL surface before compositing.
> 
> Review of attachment 745797 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: gfx/layers/composite/LayerManagerComposite.cpp
> @@ +295,5 @@
> >    // Allow widget to render a custom background.
> > +  mCompositor->GetWidget()->DrawWindowUnderlay(this, nsIntRect(bounds.x,
> > +                                                               bounds.y,
> > +                                                               bounds.width,
> > +                                                               bounds.height));
> 
> Why?

If the reason for the actualBounds -> bounds change is the fact that actualBounds is currently in screen coordinates (i.e. has a non-zero origin), then I think bug 861332 has a better fix.
Actually, in what cases can actualBounds differ from bounds/mRenderBounds?
(Assignee)

Comment 23

4 years ago
(In reply to Markus Stange from comment #22)
> (In reply to Markus Stange from comment #21)
> > Comment on attachment 745797 [details] [diff] [review]
> > Setup our OpenGL surface before compositing.
> > 
> > Review of attachment 745797 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > ::: gfx/layers/composite/LayerManagerComposite.cpp
> > @@ +295,5 @@
> > >    // Allow widget to render a custom background.
> > > +  mCompositor->GetWidget()->DrawWindowUnderlay(this, nsIntRect(bounds.x,
> > > +                                                               bounds.y,
> > > +                                                               bounds.width,
> > > +                                                               bounds.height));
> > 
> > Why?
> 
> If the reason for the actualBounds -> bounds change is the fact that
> actualBounds is currently in screen coordinates (i.e. has a non-zero
> origin), then I think bug 861332 has a better fix.
> Actually, in what cases can actualBounds differ from bounds/mRenderBounds?

No, it's just a change I made earlier when tinkering with things and forgot to revert :)
Will fix it before landing.
(Assignee)

Comment 24

4 years ago
Created attachment 747207 [details] [diff] [review]
Setup our OpenGL surface before compositing. v2
Attachment #745797 - Attachment is obsolete: true
Attachment #745797 - Flags: review?(mstange)
Attachment #747207 - Flags: review?(mstange)
(Assignee)

Comment 25

4 years ago
Created attachment 747210 [details] [diff] [review]
Fix bugs with component alpha
Attachment #747210 - Flags: review?(roc)
Attachment #747207 - Flags: review?(mstange) → review+
(Assignee)

Updated

4 years ago
Depends on: 870176
(Assignee)

Updated

4 years ago
Depends on: 870211
Attachment #747210 - Flags: review?(roc) → review+
Comment on attachment 745794 [details] [diff] [review]
Use TextureImageCGL again, and used TiledTexturedImage for large textures

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

::: gfx/gl/GLContextProviderCGL.mm
@@ +374,5 @@
> +      nsRefPtr<TextureImage> t = new gl::TiledTextureImage(this, aSize, aContentType, aFlags);
> +      return t.forget();
> +    }
> +
> +    return CreateTextureImageInternal(aSize, aContentType, 

2 extra spaces in this hunk

::: gfx/gl/GLContextProviderEGL.cpp
@@ +1537,5 @@
>  already_AddRefed<TextureImage>
>  GLContextEGL::TileGenFunc(const nsIntSize& aSize,
> +                          TextureImage::ContentType aContentType,
> +                          TextureImage::Flags aFlags)
> +

Missing '{'?
Attachment #745794 - Flags: review?(bgirard) → review+
(Assignee)

Comment 27

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/751fedabab31
https://hg.mozilla.org/integration/mozilla-inbound/rev/b58dac401ceb
https://hg.mozilla.org/integration/mozilla-inbound/rev/fed9c75cd9fd
https://hg.mozilla.org/integration/mozilla-inbound/rev/098ef52af05c
https://hg.mozilla.org/integration/mozilla-inbound/rev/0f3d7df79e32
https://hg.mozilla.org/integration/mozilla-inbound/rev/789f09e5ce7c
https://hg.mozilla.org/integration/mozilla-inbound/rev/1e746a304a6f
https://hg.mozilla.org/integration/mozilla-inbound/rev/74ff7b69c638
https://hg.mozilla.org/integration/mozilla-inbound/rev/b5a96574e077
https://hg.mozilla.org/integration/mozilla-inbound/rev/47a188160811
https://hg.mozilla.org/integration/mozilla-inbound/rev/3ea828782568
Whiteboard: [leave open]
https://hg.mozilla.org/mozilla-central/rev/751fedabab31
https://hg.mozilla.org/mozilla-central/rev/b58dac401ceb
https://hg.mozilla.org/mozilla-central/rev/fed9c75cd9fd
https://hg.mozilla.org/mozilla-central/rev/098ef52af05c
https://hg.mozilla.org/mozilla-central/rev/0f3d7df79e32
https://hg.mozilla.org/mozilla-central/rev/789f09e5ce7c
https://hg.mozilla.org/mozilla-central/rev/1e746a304a6f
https://hg.mozilla.org/mozilla-central/rev/74ff7b69c638
https://hg.mozilla.org/mozilla-central/rev/b5a96574e077
https://hg.mozilla.org/mozilla-central/rev/47a188160811
https://hg.mozilla.org/mozilla-central/rev/3ea828782568
https://hg.mozilla.org/mozilla-central/rev/ffd964f43194

Updated

4 years ago
Assignee: nobody → matt.woodrow
Depends on: 872678
(Assignee)

Updated

4 years ago
Depends on: 845943
(Assignee)

Updated

4 years ago
Depends on: 749428
(Assignee)

Updated

4 years ago
Depends on: 874367
(Assignee)

Updated

4 years ago
Depends on: 874369
(Assignee)

Updated

4 years ago
Depends on: 874370
(Assignee)

Updated

4 years ago
Depends on: 873944
(Assignee)

Updated

4 years ago
Depends on: 874823
(Assignee)

Updated

4 years ago
Depends on: 875232
(Assignee)

Updated

4 years ago
Depends on: 877034
(Assignee)

Updated

4 years ago
Depends on: 877036
(Assignee)

Updated

4 years ago
Depends on: 877534
(Assignee)

Updated

4 years ago
Depends on: 877949
(Assignee)

Updated

4 years ago
Depends on: 882590
(Assignee)

Updated

4 years ago
No longer depends on: 845943
(Assignee)

Comment 29

4 years ago
Created attachment 764528 [details] [diff] [review]
Enable OMTC on 10.7 and 10.8
Attachment #764528 - Flags: review?(jmuizelaar)
Comment on attachment 764528 [details] [diff] [review]
Enable OMTC on 10.7 and 10.8

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

::: gfx/thebes/gfxPlatform.cpp
@@ +332,5 @@
>  #endif
>  
> +    bool useOffMainThreadCompositing = false;
> +    useOffMainThreadCompositing = GetPrefLayersOffMainThreadCompositionEnabled() ||
> +        Preferences::GetBool("browser.tabs.remote", false);

Fuse declaration with assignment

::: gfx/thebes/gfxPlatformGtk.cpp
@@ +503,5 @@
> +gfxPlatformGtk::SupportsOffMainThreadCompositing()
> +{
> +#ifdef MOZ_X11
> +  return (PR_GetEnv("MOZ_USE_OMTC") != nullptr) ||
> +         (PR_GetEnv("MOZ_OMTC_ENABLED") != nullptr);

Why two different env vars here?

::: gfx/thebes/gfxPlatformMac.cpp
@@ +435,5 @@
> +gfxPlatformMac::SupportsOffMainThreadCompositing()
> +{
> +  // 10.6.X has crashes on tinderbox with OMTC, so disable it
> +  // for now.
> +  return OSXVersion() >= 0x1070;

This gives us no way to test OMTC on 10.6. I think we need a separate "force" pref.
(Assignee)

Comment 31

4 years ago
Created attachment 764546 [details] [diff] [review]
Enable OMTC on 10.7 and 10.8 v2
Attachment #764528 - Attachment is obsolete: true
Attachment #764528 - Flags: review?(jmuizelaar)
Attachment #764546 - Flags: review?(roc)
(Assignee)

Comment 32

4 years ago
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #30)
> 
> ::: gfx/thebes/gfxPlatformGtk.cpp
> @@ +503,5 @@
> > +gfxPlatformGtk::SupportsOffMainThreadCompositing()
> > +{
> > +#ifdef MOZ_X11
> > +  return (PR_GetEnv("MOZ_USE_OMTC") != nullptr) ||
> > +         (PR_GetEnv("MOZ_OMTC_ENABLED") != nullptr);
> 
> Why two different env vars here?
> 

The scripts for running tests on tinderbox are global, not per-branch. With the refactoring, we wanted this pref enabled (for Ripc), but we didn't want it to be enabled on branches that didn't have the refactoring. So we needed separate env vars.

Once the refactoring reaches *all* branches, we can drop this again.
(Assignee)

Comment 33

4 years ago
Note that this is just moving X11 specific code out of gfxPlatform, there's no changes to the env vars.
Comment on attachment 764546 [details] [diff] [review]
Enable OMTC on 10.7 and 10.8 v2

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

::: gfx/thebes/gfxPlatformMac.cpp
@@ +438,5 @@
> +  // for now.
> +  if (OSXVersion() >= 0x1070) {
> +    return true;
> +  }
> +  return GetPrefLayersOffMainThreadCompositionForceEnabled();

I think this call should move up so that if the force pref is set, we don't even call SupportsOffMainThreadCompositing on any platform.
(Assignee)

Comment 35

4 years ago
The problem is that X11 *needs* at least one of those env vars set.

The env vars are also read at startup, and initialize threaded Xlib.

http://mxr.mozilla.org/mozilla-central/source/toolkit/xre/nsAppRunner.cpp#3372

If we allow the force-enabled pref to override this, then it's just going to be a broken configuration that crashes.
Comment on attachment 764546 [details] [diff] [review]
Enable OMTC on 10.7 and 10.8 v2

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

OK I guess.
Attachment #764546 - Flags: review?(roc) → review+
(Assignee)

Comment 37

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/b029e52490da

Updated

4 years ago
Depends on: 885655
https://hg.mozilla.org/mozilla-central/rev/b029e52490da

Updated

4 years ago
relnote-firefox: --- → ?

Updated

4 years ago
relnote-firefox: ? → 24+

Updated

4 years ago
Depends on: 888530

Updated

4 years ago
Depends on: 888562

Updated

4 years ago
Depends on: 888048
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #17)
> Comment on attachment 745803 [details] [diff] [review]
> Send the values of mScaleToSize/mScaleMode across IPC
> 
> Review of attachment 745803 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: gfx/layers/ImageLayers.h
> @@ +56,5 @@
> >    void SetScaleToSize(const gfxIntSize &aSize, ScaleMode aMode)
> >    {
> >      mScaleToSize = aSize;
> >      mScaleMode = aMode;
> > +    Mutated();
> 
> Please make this "Mutated()" conditional on size/mode changing.

It would probably be a good idea to add an assertion that the layer properties have actually changed if we add an AttributeChange op for a layer.

Comment 40

4 years ago
The relnote has a typo (if I'm not mistaken), it should be: 
> … for OSX renders smoother …

(and not "render's").
(Assignee)

Updated

4 years ago
Depends on: 885994
(Assignee)

Updated

4 years ago
Depends on: 886667
(Assignee)

Updated

4 years ago
Depends on: 886892
(Assignee)

Updated

4 years ago
Depends on: 887963
(Assignee)

Updated

4 years ago
Depends on: 858914
Depends on: 902591
(Assignee)

Updated

4 years ago
Depends on: 899478
Depends on: 912678

Comment 41

4 years ago
Comment on attachment 745793 [details] [diff] [review]
Use the right texture target when allocating textures

># HG changeset patch
># User Matt Woodrow <mwoodrow@mozilla.com>
># Date 1367384858 -43200
># Node ID 45493544fd0b4320573b6d00a53bb10e10b4e2d8
># Parent  8a2fb3738795e6209df068e57088574c7707145c
>imported patch fix-video
>
>diff --git a/gfx/layers/opengl/TextureHostOGL.cpp b/gfx/layers/opengl/TextureHostOGL.cpp
>--- a/gfx/layers/opengl/TextureHostOGL.cpp
>+++ b/gfx/layers/opengl/TextureHostOGL.cpp
>@@ -46,29 +46,36 @@ CreateTextureHostOGL(SurfaceDescriptorTy
> 
>   NS_ASSERTION(result, "Result should have been created.");
> 
>   result->SetFlags(aTextureFlags);
>   return result.forget();
> }
> 
> static void
>-MakeTextureIfNeeded(gl::GLContext* gl, GLuint& aTexture)
>+MakeTextureIfNeeded(gl::GLContext* gl, GLenum aTarget, GLuint& aTexture)
> {
>   if (aTexture != 0)
>     return;
> 
>+  GLenum target = aTarget;
>+  // GL_TEXTURE_EXTERNAL requires us to initialize the texture
>+  // using the GL_TEXTURE_2D attachment.
>+  if (target == LOCAL_GL_TEXTURE_EXTERNAL) {
>+    target = LOCAL_GL_TEXTURE_2D;
>+  }
>+
>   gl->fGenTextures(1, &aTexture);
> 
>-  gl->fBindTexture(LOCAL_GL_TEXTURE_2D, aTexture);
>+  gl->fBindTexture(target, aTexture);
> 
>-  gl->fTexParameteri(LOCAL_GL_TEXTURE_2D, LOCAL_GL_TEXTURE_MIN_FILTER, LOCAL_GL_LINEAR);
>-  gl->fTexParameteri(LOCAL_GL_TEXTURE_2D, LOCAL_GL_TEXTURE_MAG_FILTER, LOCAL_GL_LINEAR);
>-  gl->fTexParameteri(LOCAL_GL_TEXTURE_2D, LOCAL_GL_TEXTURE_WRAP_S, LOCAL_GL_CLAMP_TO_EDGE);
>-  gl->fTexParameteri(LOCAL_GL_TEXTURE_2D, LOCAL_GL_TEXTURE_WRAP_T, LOCAL_GL_CLAMP_TO_EDGE);
>+  gl->fTexParameteri(target, LOCAL_GL_TEXTURE_MIN_FILTER, LOCAL_GL_LINEAR);
>+  gl->fTexParameteri(target, LOCAL_GL_TEXTURE_MAG_FILTER, LOCAL_GL_LINEAR);
>+  gl->fTexParameteri(target, LOCAL_GL_TEXTURE_WRAP_S, LOCAL_GL_CLAMP_TO_EDGE);
>+  gl->fTexParameteri(target, LOCAL_GL_TEXTURE_WRAP_T, LOCAL_GL_CLAMP_TO_EDGE);
> }
> 
> static gl::TextureImage::Flags
> FlagsToGLFlags(TextureFlags aFlags)
> {
>   uint32_t result = TextureImage::NoFlags;
> 
>   if (aFlags & UseNearestFilter)
>@@ -265,17 +272,17 @@ SharedTextureHostOGL::SwapTexturesImpl(c
>     mFormat = FormatFromShaderType(mShaderProgram);
>     mTextureTransform = handleDetails.mTextureTransform;
>   }
> }
> 
> bool
> SharedTextureHostOGL::Lock()
> {
>-  MakeTextureIfNeeded(mGL, mTextureHandle);
>+  MakeTextureIfNeeded(mGL, mTextureTarget, mTextureHandle);
> 
>   mGL->fActiveTexture(LOCAL_GL_TEXTURE0);
>   mGL->fBindTexture(mTextureTarget, mTextureHandle);
>   if (!mGL->AttachSharedHandle(mShareType, mSharedHandle)) {
>     NS_ERROR("Failed to bind shared texture handle");
>     return false;
>   }
>

Updated

4 years ago
Depends on: 890997

Updated

4 years ago
Blocks: 916812

Updated

4 years ago
Depends on: 913407

Updated

4 years ago
Depends on: 921323
Blocks: 759252

Updated

4 years ago
Depends on: 927290
No longer depends on: 885655
No longer depends on: 887963
No longer depends on: 888562
No longer depends on: 927290
There are further enhancements that can and should be made (see meta bug 958195), but OMTC on the Mac is done and shipping, so we're closing this bug.
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Whiteboard: [leave open]

Updated

3 years ago
Blocks: 958480

Updated

3 years ago
No longer blocks: 958480
Target Milestone: --- → mozilla27
Depends on: 1244684
You need to log in before you can comment on or make changes to this bug.