Bug 1633432 Comment 1 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

Original comment by
on 
(In reply to Jamie Nicol [:jnicol] from comment #0)
> I did a try run of webrender mochitests for bug 1585050. In the media suite, we reliably hit [this assertion](https://searchfox.org/mozilla-central/rev/55a4faa52f72918efa51150d127aebdc057dc6cf/gfx/webrender_bindings/RenderAndroidSurfaceTextureHostOGL.cpp#190).
> 
> 
> I think the solution is to set `mPrepareStatus = STATUS_NONE` in `DetachedFromGLContext()`. Does that sound right to you, Sotaro?

I have a concern that it seems to make mPrepareStatus unpredictable for RenderAndroidSurfaceTextureHostOGL. And also for android::Surface Texture, mSurfTex->UpdateTexImage()/mSurfTex->ReleaseTexImage() needs to be called in RenderAndroidSurfaceTextureHostOGL::NotifyNotUsed().

We call  GeckoSurfaceTexture::DetachAllFromGLContext() in RenderCompositorEGL::Pause(). Can we remove the EnsureAttachedToGLContext() call in the Pause()? I added it by mimicking CompositorOGL. But it seems not necessary for RenderCompositorEGL, since it uses shared GL context.

If we remove the call, it seems that we do not need "set `mPrepareStatus = STATUS_NONE`".
Revision 1 by
on 
(In reply to Jamie Nicol [:jnicol] from comment #0)
> I did a try run of webrender mochitests for bug 1585050. In the media suite, we reliably hit [this assertion](https://searchfox.org/mozilla-central/rev/55a4faa52f72918efa51150d127aebdc057dc6cf/gfx/webrender_bindings/RenderAndroidSurfaceTextureHostOGL.cpp#190).
> 
> 
> I think the solution is to set `mPrepareStatus = STATUS_NONE` in `DetachedFromGLContext()`. Does that sound right to you, Sotaro?

I have a concern that it seems to make mPrepareStatus unpredictable for RenderAndroidSurfaceTextureHostOGL. And also for android::Surface Texture, mSurfTex->UpdateTexImage()/mSurfTex->ReleaseTexImage() needs to be called in RenderAndroidSurfaceTextureHostOGL::NotifyNotUsed().

We call  GeckoSurfaceTexture::DetachAllFromGLContext() in RenderCompositorEGL::Pause(). Can we remove the EnsureAttachedToGLContext() call in the Pause()? I added it by mimicking CompositorOGL. But it seems not necessary for RenderCompositorEGL, since it uses shared GL context.

If we remove the call, it seems that we do not need "set `mPrepareStatus = STATUS_NONE`".

We might need to call the DetachAllFromGLContext() during shutting down a shared gl context.
Revision 2 by
on 
(In reply to Jamie Nicol [:jnicol] from comment #0)
> I did a try run of webrender mochitests for bug 1585050. In the media suite, we reliably hit [this assertion](https://searchfox.org/mozilla-central/rev/55a4faa52f72918efa51150d127aebdc057dc6cf/gfx/webrender_bindings/RenderAndroidSurfaceTextureHostOGL.cpp#190).
> 
> 
> I think the solution is to set `mPrepareStatus = STATUS_NONE` in `DetachedFromGLContext()`. Does that sound right to you, Sotaro?

I have a concern that it seems to make mPrepareStatus unpredictable for RenderAndroidSurfaceTextureHostOGL. And also for android::Surface Texture, mSurfTex->UpdateTexImage()/mSurfTex->ReleaseTexImage() needs to be called in RenderAndroidSurfaceTextureHostOGL::NotifyNotUsed().

We call  GeckoSurfaceTexture::DetachAllFromGLContext() in RenderCompositorEGL::Pause(). Can we remove the EnsureAttachedToGLContext() call in the Pause()? I added it by mimicking CompositorOGL. But it seems not necessary for RenderCompositorEGL, since it uses shared GL context.

If we remove the call, it seems that we do not need "set `mPrepareStatus = STATUS_NONE`".

Thoug, we might need to call the DetachAllFromGLContext() during shutting down a shared gl context.
Revision 3 by
on 
(In reply to Jamie Nicol [:jnicol] from comment #0)
> I did a try run of webrender mochitests for bug 1585050. In the media suite, we reliably hit [this assertion](https://searchfox.org/mozilla-central/rev/55a4faa52f72918efa51150d127aebdc057dc6cf/gfx/webrender_bindings/RenderAndroidSurfaceTextureHostOGL.cpp#190).
> 
> 
> I think the solution is to set `mPrepareStatus = STATUS_NONE` in `DetachedFromGLContext()`. Does that sound right to you, Sotaro?

I have a concern that it seems to make mPrepareStatus unpredictable for RenderAndroidSurfaceTextureHostOGL. And also for android::Surface Texture, mSurfTex->UpdateTexImage()/mSurfTex->ReleaseTexImage() needs to be called in RenderAndroidSurfaceTextureHostOGL::NotifyNotUsed().

We call  GeckoSurfaceTexture::DetachAllFromGLContext() in RenderCompositorEGL::Pause(). Can we remove the EnsureAttachedToGLContext() call in the Pause()? I added it by mimicking CompositorOGL. But it seems not necessary for RenderCompositorEGL, since it uses shared GL context.

If we remove the call, it seems that we do not need "set `mPrepareStatus = STATUS_NONE`".

Though, we might need to call the DetachAllFromGLContext() during shutting down a shared gl context.

Back to Bug 1633432 Comment 1