Closed Bug 977880 Opened 10 years ago Closed 10 years ago

Screen flicker in FFOS Settings app

Categories

(Core :: Graphics: Layers, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla31
blocking-b2g 1.4+
Tracking Status
firefox29 --- wontfix
firefox30 --- fixed
firefox31 --- fixed
b2g-v1.4 --- fixed
b2g-v2.0 --- fixed

People

(Reporter: diego, Assigned: sotaro)

References

()

Details

(Keywords: regression, Whiteboard: [CR 623404])

Attachments

(2 files, 10 obsolete files)

56 bytes, text/plain
Details
32.47 KB, patch
sotaro
: review+
Details | Diff | Splinter Review
STR:

1. Go to Settings -> Display -> Screen timeout
2. Select any of the options

There is a distinct flicker that shows the wallpaper briefly. It is reproducible with HWC enabled and disabled.
Attached file Screen recording
Sotaro,

I see this in the QRD8x26. Can you reproduce this as well?
Flags: needinfo?(sotaro.ikeda.g)
Hmm, I tested on nexus-4 and nexus-5. But I can not reproducible the symptom. It might be similar to Bug 962152, not clear yet. By the way, I've heard that I am going to receive QRD8x26 soon.
Flags: needinfo?(sotaro.ikeda.g)
Whiteboard: [CR 623404]
tiling(Bug 963073) might fix the problem.
From the analysis in Bug 977975, Compositor take too long time for OpenGL rendering. And tiling seems to mitigate the problem.
Depends on: 977975
Diego, it is not related to Bug 977975. Instead, it is related to https://bugzilla.mozilla.org/show_bug.cgi?id=974152.
I can't reproduce this anymore. Looks like there was a mystery fix sometime in the past 4 days.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → WORKSFORME
It's back! I can see this issue even with the patch from bug 974152. The video flicker is gone so it it was probably never related to bug 974152.

Sotaro,

Can you reproduce this on v1.4 KK?
Status: RESOLVED → REOPENED
Flags: needinfo?(sotaro.ikeda.g)
Resolution: WORKSFORME → ---
(In reply to Diego Wilson [:diego] from comment #8)
> It's back! I can see this issue even with the patch from bug 974152. The
> video flicker is gone so it it was probably never related to bug 974152.
> 
> Sotaro,
> 
> Can you reproduce this on v1.4 KK?

I can not reproduce the problem on nexus-5. I still did not receive QRD device... Last week, I was in Taipei, and heard that QC sent incorrect HW and needed to ask to send correct one.
Flags: needinfo?(sotaro.ikeda.g)
Diego, can you confirm that if the problem happens when tiling is disabled.
Flags: needinfo?(dwilson)
How do I disable tiling?
Flags: needinfo?(dwilson) → needinfo?(sotaro.ikeda.g)
Change "Layers:Enable Tiles" setting by Settings app and reboot the phone. Within "settings->Developer", there is "Layers:Enable Tiles".
Flags: needinfo?(sotaro.ikeda.g)
Current tiling thebes layer does not care about fence. The layer is enabled on a scrollable layer.
I borrowed QRD8x26 from mikeh. The rom seems old. I am going to try to create a recent rom.
> Current tiling thebes layer does not care about fence. The layer is enabled on a scrollable layer.

You got it! The issue goes away if I disable tiling. I'm guessing there's still some fence work to be done there?
Flags: needinfo?(sotaro.ikeda.g)
(In reply to Diego Wilson [:diego] from comment #15)
> > Current tiling thebes layer does not care about fence. The layer is enabled on a scrollable layer.
> 
> You got it! The issue goes away if I disable tiling. I'm guessing there's
> still some fence work to be done there?

Yea, fences delivery for tiling is necessary. I am going to work for it.
Flags: needinfo?(sotaro.ikeda.g)
Set 1.4? flag.
blocking-b2g: --- → 1.4?
Assignee: nobody → sotaro.ikeda.g
Keywords: regression
Blocks: b2g-tiling
Component: Graphics → Graphics: Layers
(In reply to Sotaro Ikeda [:sotaro] from comment #16)
> (In reply to Diego Wilson [:diego] from comment #15)
> > > Current tiling thebes layer does not care about fence. The layer is enabled on a scrollable layer.
> > 
> > You got it! The issue goes away if I disable tiling. I'm guessing there's
> > still some fence work to be done there?
> 
> Yea, fences delivery for tiling is necessary. I am going to work for it.

Do we need tiling to use hwcomposer for 1.4? It doesn't sound easy. IIRC, non-tiled thebes layers only use hwcomposer when unrotated, which doesn't happen often while scrolling.
(In reply to Nicolas Silva [:nical] from comment #18)
> 
> Do we need tiling to use hwcomposer for 1.4? It doesn't sound easy. IIRC,
> non-tiled thebes layers only use hwcomposer when unrotated, which doesn't
> happen often while scrolling.

HwComposer is used even for OpenGL composition case in JB/KK. OpenGL Composition result is composed to FrameBuffer layer. The layer is rendered by HwComposer. The layer's AcquireFence is used to prevent a tearing problem happen on OpenGL composition. It is done in Bug 974152.
Sotaro, can you give a brief overview on what happens when "Layers: Enable Tiles" is enabled. Does it mean, instead of having a single layer buffer, there are multiple tiles (what size?) of buffers for a layer. I believed it should affect only the Rendering phase. But now the sync fences are being used, so the acquire and release fence of a layer needs to take care of all tiles assigned to it (by dup?) in framework. How does it impact Composition phase ?
Flags: needinfo?(sotaro.ikeda.g)
1.4+ since it seems to be a titling regression
blocking-b2g: 1.4? → 1.4+
(In reply to Sushil from comment #20)
> Sotaro, can you give a brief overview on what happens when "Layers: Enable
> Tiles" is enabled. Does it mean, instead of having a single layer buffer,
> there are multiple tiles (what size?) of buffers for a layer.

Yes. Multiple tiles are used for a scrollable thebes layer. Therefore one layer has multiples of tiles. HwcComposer2D and fence handling implementation expects one buffer for one layer.

> I believed it
> should affect only the Rendering phase. But now the sync fences are being
> used, so the acquire and release fence of a layer needs to take care of all
> tiles assigned to it (by dup?) in framework. How does it impact Composition
> phase ?

About fence, need to extend CompositorOGL::SetFBAcquireFence() to handle tiles. And TiledContentHost(tiled thebes layer) uses different way of buffer swapping than ContentHost(thebes layer). Need to change TiledContentHost as to return fence to client side.
Flags: needinfo?(sotaro.ikeda.g)
Confirmed that release fences are delivered to child side. A way of tile recycle is same as default.
Comment on attachment 8396118 [details] [diff] [review]
wip patch - Handle ReleaseFence on tiled thebes layer

nical, can I have a feedback to the patch?
Attachment #8396118 - Flags: feedback?(nical.bugzilla)
Attachment #8396118 - Flags: feedback?(nical.bugzilla) → feedback+
Attachment #8396118 - Flags: feedback?(chrislord.net)
Comment on attachment 8396118 [details] [diff] [review]
wip patch - Handle ReleaseFence on tiled thebes layer

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

Looks good! All my comments are basically just 'document it better'.

::: gfx/layers/TiledLayerBuffer.h
@@ +18,5 @@
>  #include "nsRect.h"                     // for nsIntRect
>  #include "nsRegion.h"                   // for nsIntRegion
>  #include "nsTArray.h"                   // for nsTArray
>  
> +#if MOZ_WIDGET_GONK && ANDROID_VERSION >= 17

Elsewhere, we would write the first bit as defined(MOZ_WIDGET_GONK). Not sure that it matters.

@@ +211,5 @@
>     */
>    virtual const nsIntRegion& GetValidLowPrecisionRegion() const = 0;
> +
> +#if MOZ_WIDGET_GONK && ANDROID_VERSION >= 17
> +  virtual void SetReleaseFence(const android::sp<android::Fence>& aReleaseFence) = 0;

Documentation needed.

::: gfx/layers/client/TextureClient.h
@@ +339,5 @@
>    {
>      return mReleaseFenceHandle;
>    }
>  
> +  virtual void WaitReleaseFence() {}

Documentation.

::: gfx/layers/client/TextureClientPool.cpp
@@ +43,5 @@
>    // Try to fetch a client from the pool
>    RefPtr<TextureClient> textureClient;
>    if (mTextureClients.size()) {
>      textureClient = mTextureClients.top();
> +    textureClient->WaitReleaseFence();

I'll just mention so that it's on record (but I'm sure you're aware) - this will only stop us returning a TextureClient that has a fence at this point in time, but another user could easily use it after this point. We still need to make sure that we don't return clients that are in use (which we can currently tell by looking at the associated read-lock).

::: gfx/layers/composite/TextureHost.cpp
@@ +261,5 @@
>  }
>  
>  void
> +TextureHost::CompositorRecycle()
> +{ 

Trailing whitespace.

::: gfx/layers/composite/TextureHost.h
@@ +286,5 @@
>    static TemporaryRef<TextureHost> Create(const SurfaceDescriptor& aDesc,
>                                            ISurfaceAllocator* aDeallocator,
>                                            TextureFlags aFlags);
>  
> +  void CompositorRecycle();

Documentation.

::: gfx/layers/composite/ThebesLayerComposite.cpp
@@ +92,5 @@
>  ThebesLayerComposite::GetTiledLayerComposer()
>  {
> +  //MOZ_ASSERT(mBuffer && mBuffer->IsAttached());
> +  if (!mBuffer || !mBuffer->IsAttached()) {
> +    return nullptr;

Is this related? Do we want to NS_NOTREACHED here, or is it reasonable? If the latter, documentation.

::: gfx/layers/opengl/CompositorOGL.cpp
@@ +1223,5 @@
>      }
>      return;
>    }
>  
> +  // Set FBAcquireFence to tiles

Although the pre-existing documentation is as bad, it'd be nice if this explained a bit better what's happening here...

::: gfx/layers/opengl/TextureHostOGL.h
@@ +169,5 @@
>     * Return a releaseFence's Fence and clear a reference to the Fence.
>     */
>    virtual android::sp<android::Fence> GetAndResetReleaseFence();
> +
> +  virtual android::sp<android::Fence> GetReleaseFence();

Documentation.
Attachment #8396118 - Flags: feedback?(chrislord.net) → feedback+
Thanks for the quick feedbacks! I am going to update the patch soon.
> ::: gfx/layers/composite/ThebesLayerComposite.cpp
> @@ +92,5 @@
> >  ThebesLayerComposite::GetTiledLayerComposer()
> >  {
> > +  //MOZ_ASSERT(mBuffer && mBuffer->IsAttached());
> > +  if (!mBuffer || !mBuffer->IsAttached()) {
> > +    return nullptr;
> 
> Is this related? Do we want to NS_NOTREACHED here, or is it reasonable? If
> the latter, documentation.

Want to return nullptr when ThebesLayerComposite is not ready for composition. I changed the code because GetTiledLayerComposer() is called from CompositorOGL::SetFBAcquireFence() even when mBuffer is nullptr.
(In reply to Sotaro Ikeda [:sotaro] from comment #28)
> > ::: gfx/layers/composite/ThebesLayerComposite.cpp
> > @@ +92,5 @@
> > >  ThebesLayerComposite::GetTiledLayerComposer()
> > >  {
> > > +  //MOZ_ASSERT(mBuffer && mBuffer->IsAttached());
> > > +  if (!mBuffer || !mBuffer->IsAttached()) {
> > > +    return nullptr;
> > 
> > Is this related? Do we want to NS_NOTREACHED here, or is it reasonable? If
> > the latter, documentation.
> 
> Want to return nullptr when ThebesLayerComposite is not ready for
> composition. I changed the code because GetTiledLayerComposer() is called
> from CompositorOGL::SetFBAcquireFence() even when mBuffer is nullptr.

Sounds fine - do we want to do something like:

if (!mBuffer) {
  return;
}
MOZ_ASSERT(mBuffer->IsAttached());

though? Or can this be called with an unattached buffer?
(In reply to Chris Lord [:cwiiis] from comment #29)
> (In reply to Sotaro Ikeda [:sotaro] from comment #28)
> > > ::: gfx/layers/composite/ThebesLayerComposite.cpp
> > > @@ +92,5 @@
> > > >  ThebesLayerComposite::GetTiledLayerComposer()
> > > >  {
> > > > +  //MOZ_ASSERT(mBuffer && mBuffer->IsAttached());
> > > > +  if (!mBuffer || !mBuffer->IsAttached()) {
> > > > +    return nullptr;
> > > 
> > > Is this related? Do we want to NS_NOTREACHED here, or is it reasonable? If
> > > the latter, documentation.
> > 
> > Want to return nullptr when ThebesLayerComposite is not ready for
> > composition. I changed the code because GetTiledLayerComposer() is called
> > from CompositorOGL::SetFBAcquireFence() even when mBuffer is nullptr.
> 
> Sounds fine - do we want to do something like:
> 
> if (!mBuffer) {
>   return;
> }
> MOZ_ASSERT(mBuffer->IsAttached());
> 
> though? Or can this be called with an unattached buffer?

Thanks. The above seems fine. It should not be called with an unattached buffer.
Attachment #8396118 - Attachment is obsolete: true
Fix nits.
Attachment #8396504 - Attachment is obsolete: true
Fix nits.
Attachment #8396508 - Attachment is obsolete: true
Hmm, by applying attachment 8396515 [details] [diff] [review] on nexus-5, video playback seems to have flickering. The symptom is same as Bug 981532 :-( On default nexus-5 ROM, I do not see the flicker.
After applying attachment 8396515 [details] [diff] [review], valid fence did not delivered to OMXCodec. In CompositorOGL::SetFBAcquireFence(), I put "return" in wrong place :-( By fixing it. Video regression is fixed.

The following is a fixed version.
-----------------------
  TiledLayerComposer* composer = nullptr;
  LayerComposite* shadow = aLayer->AsLayerComposite();
  if (shadow) {
    composer = shadow->GetTiledLayerComposer();
    if (composer) {
      composer->SetReleaseFence(new android::Fence(GetGonkDisplay()->GetPrevFBAcquireFd()));
      return;
    }
  }
Fix video playback's flickering regression.
Attachment #8396515 - Attachment is obsolete: true
Diego, can you check if attachment 8396597 [details] [diff] [review] fix the problem?
Flags: needinfo?(dwilson)
Attachment #8396597 - Flags: review?(nical.bugzilla)
Attachment #8396597 - Flags: review?(chrislord.net)
Comment on attachment 8396597 [details] [diff] [review]
patch v4 - Handle ReleaseFence on tiled thebes layer

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

Good stuff, r+ with the comments addressed. None of the comments are critical imo. I'm unsure what the recycling bit was about, so I hope nical can give that more scrutiny, but it looked benign. Appreciate the additional documentation :)

::: gfx/layers/TiledLayerBuffer.h
@@ +215,5 @@
> +  /**
> +   * Store a fence that will signal when the current buffer is no longer being read.
> +   * Similar to android's GLConsumer::setReleaseFence()
> +   */
> +  virtual void SetReleaseFence(const android::sp<android::Fence>& aReleaseFence) = 0;

Because we're touching TiledLayerBuffer, you might want to get BenWa's approval for this - is there a way this could be on the implementation rather than on this interface? (i.e. on ClientTiledLayerBuffer)

::: gfx/layers/client/TextureClientPool.cpp
@@ +43,5 @@
>    // Try to fetch a client from the pool
>    RefPtr<TextureClient> textureClient;
>    if (mTextureClients.size()) {
>      textureClient = mTextureClients.top();
> +    textureClient->WaitReleaseFence();

Note for self, this will require a change to my patch in bug 984618 if we decide to commit that.

::: gfx/layers/composite/TextureHost.h
@@ +287,5 @@
>                                            ISurfaceAllocator* aDeallocator,
>                                            TextureFlags aFlags);
>  
>    /**
> +   * Tell to TextureChild that TextureHost is recycled.

I think this comment could be a bit better - I'm not really sure what recycled means in this context, or when it ought to be called.

::: gfx/layers/composite/TiledContentHost.cpp
@@ +31,5 @@
>    , mHasDoubleBufferedTiles(false)
>    , mUninitialized(true)
>  {}
>  
> +static void RecycleCallback(TextureHost* textureHost, void* aClosure) {

nits, static void should be on a separate line and the opening function brace should be on a separate line.

::: gfx/layers/opengl/CompositorOGL.cpp
@@ +1218,5 @@
>    if (visibleRegion.IsEmpty()) {
>        return;
>    }
>  
> +  // Set FBAcquireFence to ContainerLayer's childs

s/to/on/

@@ +1227,5 @@
>      }
>      return;
>    }
>  
> +  // Set FBAcquireFence as tiles' ReleaseFence to TiledLayerComposer.

s/to/on/

::: gfx/layers/opengl/TextureHostOGL.h
@@ +175,5 @@
> +
> +  /**
> +   * Hold previous ReleaseFence to prevent Fence delivery failure via gecko IPC.
> +   * Fence is a kernel object and it's lifetime is managed by a reference count.
> +   * Until the Fence is delivered to client side, need to hold Fence on host side. 

nits, trailing space and s/it's/its/
Attachment #8396597 - Flags: review?(chrislord.net) → review+
I can't reproduce the issue on the tip of v1.4. But then again, it has disappeared and reappeared before.
Flags: needinfo?(dwilson)
Apply the comments. Carry "r=cwiiis".
Attachment #8396597 - Attachment is obsolete: true
Attachment #8396597 - Flags: review?(nical.bugzilla)
Attachment #8396888 - Flags: review?(nical.bugzilla)
Attachment #8396888 - Flags: review?(bgirard)
Fix crash problem by changing TiledLayerBufferComposite::SetReleaseFence().
Attachment #8396903 - Flags: review?(nical.bugzilla)
Attachment #8396903 - Flags: review?(bgirard)
Attachment #8396888 - Attachment is obsolete: true
Attachment #8396888 - Flags: review?(nical.bugzilla)
Attachment #8396888 - Flags: review?(bgirard)
Fix build failures on some platforms.
Attachment #8396903 - Attachment is obsolete: true
Attachment #8396903 - Flags: review?(nical.bugzilla)
Attachment #8396903 - Flags: review?(bgirard)
Attachment #8397050 - Flags: review?(nical.bugzilla)
Attachment #8397050 - Flags: review?(bgirard)
This is not reproducible on Buri with/without Hardware Composer Enabled on master now

Gaia      80af23f8c74d9d2e9388d8ed3c204040b5c528ec
Gecko     https://hg.mozilla.org/mozilla-central/rev/c69c55582faa
BuildID   20140326040202
Version   31.0a1
On ICS device (like buri), this problem does not happen. genlock protect from the problem. The problem could happen only JB or newer gonk.
But even on nenxu-4, nexus-5, QRD devices, I could not reproduce the problem by myself.
Attachment #8397050 - Flags: review?(nical.bugzilla) → review+
Comment on attachment 8397050 [details] [diff] [review]
patch v7 - Handle ReleaseFence on tiled thebes layer (r=cwiiis)

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

::: gfx/layers/client/SimpleTextureClientPool.cpp
@@ +63,5 @@
>    // Try to fetch a client from the pool
>    RefPtr<TextureClient> textureClient;
>    if (mAvailableTextureClients.size()) {
>      textureClient = mAvailableTextureClients.top();
> +    textureClient->WaitReleaseFence();

Shouldn't we look for a texture client that doesn't require waiting for a fence and maybe going as far as allocating for a new object instead of waiting for a fence or get content we're not going to use re-use anyways? (Where just going to reuse the storage). If so we should have a comment on why we wait for the fence instead of doing something else.

::: gfx/layers/client/TextureClientPool.cpp
@@ +43,5 @@
>    // Try to fetch a client from the pool
>    RefPtr<TextureClient> textureClient;
>    if (mTextureClients.size()) {
>      textureClient = mTextureClients.top();
> +    textureClient->WaitReleaseFence();

Similar to the other pool

::: gfx/layers/composite/TextureHost.cpp
@@ +760,5 @@
> +#if defined(MOZ_WIDGET_GONK) && ANDROID_VERSION >= 17
> +  if (mTextureHost) {
> +    TextureHostOGL* hostOGL = mTextureHost->AsHostOGL();
> +    android::sp<android::Fence> fence = hostOGL->GetAndResetReleaseFence();
> +    if (fence.get() && fence->isValid()) {

What happens if this fails? It looks from the code that we need to fence for Android >= 17 but if we don't get a valid fence we should pass a null fence handle.

::: gfx/layers/composite/TiledContentHost.h
@@ +140,5 @@
> +#if defined(MOZ_WIDGET_GONK) && ANDROID_VERSION >= 17
> +  virtual void SetReleaseFence(const android::sp<android::Fence>& aReleaseFence);
> +#endif
> +
> +  // Recycle callback fro TextureHost.

for

::: gfx/layers/opengl/CompositorOGL.cpp
@@ +1206,5 @@
>  void
>  CompositorOGL::SetFBAcquireFence(Layer* aLayer)
>  {
> +  // OpenGL does not provide ReleaseFence for rendering.
> +  // Instead use FBAcquireFence as layer buffer's ReleaseFence

I have trouble understanding this code because FBAcquireFence isn't documented.

::: gfx/layers/opengl/GrallocTextureClient.cpp
@@ +193,5 @@
> +#if defined(MOZ_WIDGET_GONK) && ANDROID_VERSION >= 17
> +   if (mReleaseFenceHandle.IsValid()) {
> +     android::sp<Fence> fence = mReleaseFenceHandle.mFence;
> +#if ANDROID_VERSION == 17
> +     fence->waitForever(1000, "GrallocTextureClientOGL::Lock");

Why do we only wait 1000 for Android v17 and not 18+?
Attachment #8397050 - Flags: review?(bgirard) → review-
(In reply to Benoit Girard (:BenWa) from comment #47)
> Comment on attachment 8397050 [details] [diff] [review]
> patch v7 - Handle ReleaseFence on tiled thebes layer (r=cwiiis)
> 
> Review of attachment 8397050 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: gfx/layers/client/SimpleTextureClientPool.cpp
> @@ +63,5 @@
> >    // Try to fetch a client from the pool
> >    RefPtr<TextureClient> textureClient;
> >    if (mAvailableTextureClients.size()) {
> >      textureClient = mAvailableTextureClients.top();
> > +    textureClient->WaitReleaseFence();
> 
> Shouldn't we look for a texture client that doesn't require waiting for a
> fence and maybe going as far as allocating for a new object instead of
> waiting for a fence or get content we're not going to use re-use anyways?
> (Where just going to reuse the storage). If so we should have a comment on
> why we wait for the fence instead of doing something else.
> 
> ::: gfx/layers/client/TextureClientPool.cpp
> @@ +43,5 @@
> >    // Try to fetch a client from the pool
> >    RefPtr<TextureClient> textureClient;
> >    if (mTextureClients.size()) {
> >      textureClient = mTextureClients.top();
> > +    textureClient->WaitReleaseFence();
> 
> Similar to the other pool

We only return clients to the pool that are imminently going to be released - this is rarely hit, and when it is, the cost of waiting is usually less than the cost of a new allocation (which involves a compositor round-trip). If we had other clients to reuse, we could reuse another instead of waiting though, there's certainly an argument to be made for that.
About "textureClient->WaitReleaseFence()", I, BenWa and jrmuizel talked about it. Even in ICS, genlock do the same thing in ICS(hamachi) implicitly. The genlock's wait happens when drawing. WaitReleaseFence() is a little bit sooner than genlock but. It is almost same timing. So, it should not make more harm than current ICS situation. We are agreed not to touch this code.

As a separate bug, we are going to investigate, Which could get better performance between std::stack  std::queue as BufferPool implementation. Current implementation uses stack.
Apply comments.
Attachment #8397050 - Attachment is obsolete: true
Attachment #8397421 - Flags: review?(bgirard)
Attachment #8397421 - Flags: review?(bgirard)
Add one comment.
Attachment #8397421 - Attachment is obsolete: true
Attachment #8397425 - Flags: review?(bgirard)
Comment on attachment 8397425 [details] [diff] [review]
patch v9 - Handle ReleaseFence on tiled thebes layer (r=cwiiis, nical)

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

::: gfx/layers/client/TextureClient.cpp
@@ +102,5 @@
> +    if (aFence.type() != aFence.Tnull_t) {
> +      FenceHandle fence = aFence.get_FenceHandle();
> +      if (fence.IsValid() && mTextureClient) {
> +        mTextureClient->SetReleaseFenceHandle(aFence);
> +      }

Lets also write why failure handling isn't required here.
Attachment #8397425 - Flags: review?(bgirard) → review+
Apply the comment.
Attachment #8397425 - Attachment is obsolete: true
Attachment #8397488 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/19bcfceeb477
Status: REOPENED → RESOLVED
Closed: 10 years ago10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Has this made it to the "second" B2G build today?  If so, and it can be tested and verified, let's uplift to aurora today.
Flags: needinfo?(sotaro.ikeda.g)
Flags: needinfo?(npark)
Keywords: checkin-needed
Whiteboard: [CR 623404] → [CR 623404][checkin-needed-aurora]
(In reply to Milan Sreckovic [:milan] from comment #56)
> Has this made it to the "second" B2G build today?  If so, and it can be
> tested and verified, let's uplift to aurora today.

What is second B2G build? There is no mozilla build ROM that can check this problem. The problem is only confirmed by diego's QRD device.
Flags: needinfo?(sotaro.ikeda.g)
Flags: needinfo?(npark)
Depends on: 990063
Depends on: 989215
Depends on: 990080
Flags: in-moztrap?
Found Test Case: https://moztrap.mozilla.org/manage/case/3450/

STR needs to added to verify this bug to the existing test case.
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Flags: in-moztrap? → in-moztrap?(ychung)
Added an extra step and expected result to cover this issue in this moztrap test case https://moztrap.mozilla.org/manage/cases/?filter-id=3450
Flags: needinfo?(ktucker)
Flags: in-moztrap?(ychung)
Flags: in-moztrap+
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: