Closed Bug 937713 Opened 6 years ago Closed 6 years ago

[B2G] Display does not respond or responds too slowly to device rotation.

Categories

(Core Graveyard :: Widget: Gonk, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:1.4+, firefox29 wontfix, firefox30 fixed, firefox31 fixed, b2g-v1.2 wontfix, b2g-v1.3 wontfix, b2g-v1.4 fixed, b2g-v2.0 fixed)

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

People

(Reporter: dwatson, Assigned: mwu)

References

(Depends on 1 open bug)

Details

(Whiteboard: [TPE_GFX], [ perf-reviewed ] [CR 624040])

Attachments

(2 files, 6 obsolete files)

Description:
After selecting a photo in Gallery and rotating the phone 180°, the screen orientation does not rotate properly and/or is very slow at transitioning.

Repro Steps:
1. Updated Buri to Build ID: 20131112004004
2. Open the Gallery app
3. Select a photo/ picture
4. Rotate the phone 90 degrees
5. Rotate the phone 180 degrees

Actual Results:
The orientation does not rotate 180 degrees gracefully.

Expected Results:
When the phone is rotated in the gallery app the photo will shift according to the which way the phone is turned.

Environmental Variables
Device: Buri v1.2 COM RIL
Build ID: 20131112004004
Gecko: http://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/566e850868c7
Gaia: 35b1c6b669b7bf08126df451221113e72642abca
Platform Version: 26.0
RIL Version: 01.02.00.019.106
Base Build: 20131104

Notes:
Sometimes the phone needs extra shaking or the users has to slowly turn the phone 90 degrees at a time to shift.
Does this reproduce on 1.1?
Keywords: perf, qawanted
QA Contact: sparsons
I was able to repro this issue on Leo 1.1 Build ID: 20131112041200

Gaia   d9c6b0c3384625e467f892c9dafbc40cfe132180
SourceStamp 31fa87bfba88
BuildID 20131112041200
Version 18.0
Keywords: qawanted
We believe this is a functional issue, in the app or the underlying API.
Flags: needinfo?(hkoka)
Keywords: perf
Whiteboard: [ perf-reviewed ]
The phone orientation is not being considered in the gallery app. I can also reproduce this on Leo (1.1) and on Buri (1.2). 

NI djf for comment on the fix. We can then decide the target milestone. 

Thanks
Hema
Flags: needinfo?(dflanagan)
Flags: needinfo?(hkoka)
I see slow rotation on my 1.3 Helix as well.  

But I'm almost 100% sure that this is not a gallery bug. Gallery doesn't do anything to handle rotation. That is all handled by gecko.  I'm re-categorizing it as Gaia::System, though I suspect it is actually a gecko or gonk thing.

It seems to happen in Video, too, which is further indication that it is not our issue.  
Does the browser app still rotate? We should check it there.

Also, the title for this bug is really misleading because this isn't about rotating pictures, it is about rotating the phone.  I'll edit it.

I don't know who works on the gonk level screen rotation code, but I bet that John knows because he was involved in the bug where we changed the camera to use the accelerometer instead of the orientation sensor.

John: do you know who we should bring this bug up with?
Component: Gaia::Gallery → Gaia::System
Flags: needinfo?(dflanagan) → needinfo?(johu)
Summary: [B2G][Gallery] Rotating a selected picture 180° does not always rotate/ or is slow → [B2G] Display does not respond or responds too slowly to device rotation.
I had tested with Browser app. It also rotates slowly. When I hold the device portrait and rotate 180 degrees rapidly, the device may not rotate the screen until I shake slightly.

Marco: would you mind to find someone to check this bug?
Component: Gaia::System → General
Flags: needinfo?(johu) → needinfo?(mchen)
It seems there is a Bug 902821 with patch for this issue but it was not landed because no blocking flags.
Flags: needinfo?(mchen)
Hi Vincent,

May I know this bug is the same with your Bug 902821 which had a patch but closed with "wont fix"?
Flags: needinfo?(vlin)
Yap~
From the Description, I think it's quite similar to Bug 902821, but not sure if the root cause is the same. Bug 902821 is closed with "wont fix", but the fix had already been landed on Master for a while. You can double check nsWindow.cpp for the fix.
Flags: needinfo?(vlin)
Hi Viral,

Could you help to verify that OrientationObserver notified nsWindow about orientation change in time?
If not, we need to find the root cause.
If yes, we need graphic guys to help this case.

Thanks.
Flags: needinfo?(vwang)
Hi Vincent,

OrientationObserver notify nsWindows and SetRotation with correct rotation.
But the response is slow in 180 degree rotation. (I test in Browser)

Maybe the patch on Bug 902821 also need to land on 1.2?
Flags: needinfo?(vwang) → needinfo?(vlin)
(In reply to viral [:viralwang] from comment #11)
> Hi Vincent,
> 
> OrientationObserver notify nsWindows and SetRotation with correct rotation.
> But the response is slow in 180 degree rotation. (I test in Browser)
> 
> Maybe the patch on Bug 902821 also need to land on 1.2?

Which code line did you check for the notification to nsWindows with rotation ?

I just review nsWindow.cpp. The patch is already on 1.2 and actually exists in Master for a while.
Flags: needinfo?(vlin)
Hi Vincent,

Here's the diff in my testing, log shows OrientationObserver notify nsWindow and SetRotation as our expect.
But display didn't update after that.

@@ -799,6 +799,7 @@ nsScreenGonk::SetRotation(uint32_t aRotation)
                                sVirtualBounds.height,
                                !i);
 
+LOGE("SetRotation: %d", aRotation);
     nsAppShell::NotifyScreenRotation();
 
     return NS_OK;
Flags: needinfo?(vlin)
Just roughly investigate this by GDB...

nsWindow::DoDraw() seems abandoned(not called any more) which means there might be a design change on painting window i.e the patch of Bug 902821 is useless now.
Flags: needinfo?(vlin)
Assignee: nobody → vlin
There're basically two issues on this Bug.

1. Rotation from 0 to 90. Display reacts so slowly.

nsView::WindowResized() costs more than 100ms on CSS::ComputeStyleChangeFor
nsLayoutUtils::PaintFrame() costs around 100ms as well on PaintThebes().

2. Rotation from 90 to 180. Display doesn't response.

That seems due to reflow relies on DoSetWindowDimensions.
http://dxr.mozilla.org/mozilla-central/source/view/src/nsViewManager.cpp#196
The implementation won't do ResizeReflow if dimension is not changes.

How rotation with 180 degree change can rely on ?
ni from ROC, DBaron
Flags: needinfo?(roc)
Flags: needinfo?(dbaron)
(In reply to Vincent Lin[:vilin] from comment #15)
> There're basically two issues on this Bug.
> 
> 1. Rotation from 0 to 90. Display reacts so slowly.
> 
> nsView::WindowResized() costs more than 100ms on CSS::ComputeStyleChangeFor
> nsLayoutUtils::PaintFrame() costs around 100ms as well on PaintThebes().
> 
> 2. Rotation from 90 to 180. Display doesn't response.

Correct "from 90 to 180" to "from 0 to 180" or "from 90 to 270"
which changes orientation, but doesn'y change dimension.
Attached patch patch in nsViewManager.cpp (obsolete) — Splinter Review
This patch makes ResizeRelow() executed for 180 rotation change which fixed "no response" issue and induces additional ~25ms costs on PresShell::Paint(BuildDisplayList, ProcessDisplayItems).
Attachment #8348821 - Flags: feedback?(roc)
Attachment #8348821 - Flags: feedback?(dbaron)
Attachment #8348821 - Attachment is patch: true
Attachment #8348821 - Attachment mime type: text/x-patch → text/plain
Comment on attachment 8348821 [details] [diff] [review]
patch in nsViewManager.cpp

Could you explain why we'd want to do a resize reflow if the dimensions have not changed?  The reason to do the reflow is to update layout information as a result of the change in dimensions; if there's no change in dimensions we shouldn't need to change anything.
(In reply to David Baron [:dbaron] (needinfo? me) (UTC-5) from comment #19)
> Comment on attachment 8348821 [details] [diff] [review]
> patch in nsViewManager.cpp
> 
> Could you explain why we'd want to do a resize reflow if the dimensions have
> not changed?  The reason to do the reflow is to update layout information as
> a result of the change in dimensions; if there's no change in dimensions we
> shouldn't need to change anything.

The problem is the orientation changes and doesn't update to Compositor. It'll be an issue on B2G for mobile device which can be rotated. User will see the upside down view if device is rotation directly with 180 degree change. The patch is kind of tricky, but it finally triggers the update to Compositor.
I think we should have a separate method in nsIWidgetListener to indicate that the orientation has changed.
Flags: needinfo?(roc)
Depends on: 951846
No longer depends on: 951846
Depends on: 951846
Depends on: 952131
Create Bug 952131 for layout performance issue.

This Bug is going to follow up "no response" issue which is not performance-wise.
Attached patch bug-fix-937713.patch (obsolete) — Splinter Review
Request a repaint in case device's dimension wasn't changed while orientation was changed with 180-degree.
Attachment #8355124 - Flags: feedback?(roc)
Attachment #8355124 - Flags: feedback?(dbaron)
(In reply to Vincent Lin[:vilin] from comment #23)
> Created attachment 8355124 [details] [diff] [review]
> bug-fix-937713.patch
> 
> Request a repaint in case device's dimension wasn't changed while
> orientation was changed with 180-degree.

Correction :

Request a repaint in case Window's dimension wasn't changed while device's orientation was changed with 180-degree.
Comment on attachment 8355124 [details] [diff] [review]
bug-fix-937713.patch

We want to allow this function to be called multiple times with the same dimensions without doing extra work.  If there's some type of change that requires a repaint but doesn't change the window then the code that causes a repaint should be specific to handling that change.
Attachment #8355124 - Flags: feedback?(dbaron) → feedback-
Flags: needinfo?(dbaron)
Attachment #8348821 - Flags: feedback?(dbaron) → feedback-
And let me be extra clear:  that means I think you should be changing different code to fix this bug:  certainly a different function, and probably a totally different part of the system.
(In reply to David Baron [:dbaron] (needinfo? me) (UTC-8) from comment #26)
> And let me be extra clear:  that means I think you should be changing
> different code to fix this bug:  certainly a different function, and
> probably a totally different part of the system.

Can we still call nsView::RequestRepaint() function, but make it more specific for handling 180-degree orientation change ? Probably fix it in nsWindow::Resize with reference to nsScreenGonk::GetRotation.

Or any concern about calling it but not inventing a new one(similar one) for repainting ?
Flags: needinfo?(dbaron)
I don't understand why the existing code doesn't work. On any orientation change, Gonk nsWindow::Resize gets called at least once with aRepaint true. Then we call gWindowToRedraw->Invalidate(sVirtualBounds) which should trigger a repaint event which does a layer transaction with the correct orientation. What's going wrong here? (Admittedly, I don't understand why we would have more than one window in sTopWindows or it's OK to have gRedrawWindow be a global variable so that we can only repaint the last window that had Invalidate() called on it.)
(In reply to Vincent Lin[:vilin] from comment #27)
> Can we still call nsView::RequestRepaint() function, but make it more
> specific for handling 180-degree orientation change ? Probably fix it in
> nsWindow::Resize with reference to nsScreenGonk::GetRotation.
> 
> Or any concern about calling it but not inventing a new one(similar one) for
> repainting ?

I think roc's a better person for answering these questions than I am, but I agree that it would be useful first to better understand why additional invalidation is needed here.
Flags: needinfo?(dbaron)
(In reply to David Baron [:dbaron] (needinfo? me) (UTC-8) from comment #29)
> (In reply to Vincent Lin[:vilin] from comment #27)
> > Can we still call nsView::RequestRepaint() function, but make it more
> > specific for handling 180-degree orientation change ? Probably fix it in
> > nsWindow::Resize with reference to nsScreenGonk::GetRotation.
> > 
> > Or any concern about calling it but not inventing a new one(similar one) for
> > repainting ?
> 
> I think roc's a better person for answering these questions than I am, but I
> agree that it would be useful first to better understand why additional
> invalidation is needed here.

The comment here assumes "Layout will generate repaint requests during reflow".
http://mxr.mozilla.org/mozilla-central/source/layout/base/nsDocumentViewer.cpp#1873

But the assumption is wrong for 180-degree orientation change.

I also noticed that in nsScreenGonk::SetRotation() there seems one possible path for invalidating.
https://mxr.mozilla.org/mozilla-central/source/widget/gonk/nsWindow.cpp#768

The call flow is supposed to be as following.
nsWindow::Resize -> nsWindow::Invalidate -> mozilla::NotifyEvent() ---> nsAppShell::ProcessNextNativeEvent -> nsWindow::DoDraw() -> WillPaintWindow/PaintWindow/DidPaintWindow

But the problems here is that gWindowToRedraw is somehow null. Which makes the invalidation skipped.
https://mxr.mozilla.org/mozilla-central/source/widget/gonk/nsWindow.cpp#382
Flags: needinfo?(roc)
Just saw this comment after I posted Comment 30.

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #28)
> I don't understand why the existing code doesn't work. On any orientation
> change, Gonk nsWindow::Resize gets called at least once with aRepaint true.

The comment here assumes "Layout will generate repaint requests during reflow".
http://mxr.mozilla.org/mozilla-central/source/layout/base/nsDocumentViewer.cpp#1873
So aRepaint is false here.

But the assumption seems wrong for 180-degree orientation change. (So we have this Bug now.)

> Then we call gWindowToRedraw->Invalidate(sVirtualBounds) which should
> trigger a repaint event which does a layer transaction with the correct

The call flow is supposed to be as following.
nsScreenGonk::SetRotation() -> nsWindow::Resize -> nsWindow::Invalidate -> mozilla::NotifyEvent() ---> nsAppShell::ProcessNextNativeEvent -> nsWindow::DoDraw() -> WillPaintWindow/PaintWindow/DidPaintWindow

But the problems here is that gWindowToRedraw is somehow null. Which makes the invalidation skipped.
https://mxr.mozilla.org/mozilla-central/source/widget/gonk/nsWindow.cpp#382

> orientation. What's going wrong here? (Admittedly, I don't understand why we
> would have more than one window in sTopWindows or it's OK to have
> gRedrawWindow be a global variable so that we can only repaint the last
> window that had Invalidate() called on it.)

I saw sTopWindows.Length() == 1 in nsScreenGonk::SetRotation

What happens to Invalidate() was it'll return at the point.
https://mxr.mozilla.org/mozilla-central/source/widget/gonk/nsWindow.cpp#422
Whiteboard: [ perf-reviewed ] → [TPE_GFX], [ perf-reviewed ]
We should not be relying on nsWindow::Resize doing anything here. It's expected that if you call Resize with the same size as the current size (which is true here), it doesn't do anything. I think nsScreenGonk::SetRotation should call Invalidate on all the windows (at least all the toplevel windows) if the orientation actually changed. But it sounds like that might not fix the bug anyway.

(In reply to Vincent Lin[:vilin] from comment #31)
> I saw sTopWindows.Length() == 1 in nsScreenGonk::SetRotation
> 
> What happens to Invalidate() was it'll return at the point.
> https://mxr.mozilla.org/mozilla-central/source/widget/gonk/nsWindow.cpp#422

So the window's topmost ancestor (which could be the window itself; is it?) is not in the sTopWindows list. That seems wrong, but I'm not familiar with Gonk's window setup so I can't say for sure. The gWindowToRedraw stuff doesn't look good to me.
Flags: needinfo?(roc)
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #32)
> We should not be relying on nsWindow::Resize doing anything here. It's
> expected that if you call Resize with the same size as the current size
> (which is true here), it doesn't do anything. I think
> nsScreenGonk::SetRotation should call Invalidate on all the windows (at
> least all the toplevel windows) if the orientation actually changed. But it
> sounds like that might not fix the bug anyway.
> 
> (In reply to Vincent Lin[:vilin] from comment #31)
> > I saw sTopWindows.Length() == 1 in nsScreenGonk::SetRotation
> > 
> > What happens to Invalidate() was it'll return at the point.
> > https://mxr.mozilla.org/mozilla-central/source/widget/gonk/nsWindow.cpp#422
> 
> So the window's topmost ancestor (which could be the window itself; is it?)
> is not in the sTopWindows list. That seems wrong, but I'm not familiar with
> Gonk's window setup so I can't say for sure. The gWindowToRedraw stuff
> doesn't look good to me.

Exactly speaking, mParent is null here. (assigned in Create())
https://mxr.mozilla.org/mozilla-central/source/widget/gonk/nsWindow.cpp#420

gWindowToRedraw saves the latest invalidated window by the flow "show()->BringToTop()->Invalidate()". But none of nsWindow instance passed those condition checks in the flow.
Flags: needinfo?(roc)
I'm not the best person to help here since I don't know the Gonk nsWindow code.

It seems that sTopWindows contains every Gonk nsWindow, even the non-toplevel ones. That is confusing :-(.

The code in Resize that invalidates gWindowToRedraw seems wrong. It seems much better to call this->Invalidate(). I think that will work OK because nsWindow::Invalidate ignores all windows other than the one (I presume there's only going to be one!) whose parent is sTopWindows[0], which I presume is the only window whose contents actually get painted.

Then, nsScreenGonk::SetRotation should pass true for the aRepaint parameter when it calls Resize. Right now it only passes it for sTopWindows[0], whose Invalidate() will be ignored.
Flags: needinfo?(roc)
Attached patch proposed fix (obsolete) — Splinter Review
What I just suggested. I have no idea if this works.
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #35)
> Created attachment 8355984 [details] [diff] [review]
> proposed fix
> 
> What I just suggested. I have no idea if this works.

Not works.
Invalidate() still returns here.
https://mxr.mozilla.org/mozilla-central/source/widget/gonk/nsWindow.cpp#423

ni mwu
Do you know more about gonk window ? Or someone may know the history ?

The problem is display doesn't response to device orientation change  with 180-degree.

nsWindow.cpp#423 results in our expected flow stopped at nsWindow::Invalidate.
nsScreenGonk::SetRotation() -> nsWindow::Resize -> nsWindow::Invalidate -> mozilla::NotifyEvent() ---> nsAppShell::ProcessNextNativeEvent -> nsWindow::DoDraw() -> WillPaintWindow/PaintWindow/DidPaintWindow

For orientation change with 90 or 270 degree, display update relies on reflow.
Flags: needinfo?(mwu)
(In reply to Vincent Lin[:vilin] from comment #36)
> Not works.
> Invalidate() still returns here.
> https://mxr.mozilla.org/mozilla-central/source/widget/gonk/nsWindow.cpp#423

You mean it returns there for all windows?

Does that mean that sTopWindows[0] has no children?
I dug into this a bit further.

The invalidate/resize/etc. code doesn't matter afaict. OMTC takes care of everything and the drawing path in nsWindow.cpp doesn't do anything anymore.

So, I tried calling CompositorParent::ScheduleRenderOnCompositorThread. That doesn't work either.

AFAICT, that's because CompositorParent doesn't know about the new orientation. I don't know the code here well enough to figure out the right way to update the orientation.
Flags: needinfo?(mwu)
We update the orientation by scheduling a main-thread layers transaction which publishes a new layer tree with the correct orientation. That's why Invalidate is important.
This seems to work for me. More comments to follow.
Assignee: vlin → mwu
Attachment #8348821 - Attachment is obsolete: true
Attachment #8355124 - Attachment is obsolete: true
Attachment #8355984 - Attachment is obsolete: true
Comment on attachment 8358266 [details] [diff] [review]
Replace gWindowToRedraw with a simpler mechanism

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

::: widget/gonk/nsWindow.cpp
@@ +190,5 @@
>      }
>  
> +    nsWindow *targetWindow = (nsWindow *)sTopWindows[0];
> +    while (targetWindow->GetLastChild())
> +        targetWindow = (nsWindow *)targetWindow->GetLastChild();

This patch removes gWindowToRedraw, and instead picks an child at the bottom of the tree. gWindowToRedraw was originally introduced since I didn't want to recursively check if child windows cover the parent window to determine if we should draw the window - which is what the Android nsWindow.cpp does. There's really only one window we want to draw. (and we currently crash if we try to draw more than one, though that is likely easy to fix) I'd love to know if there's a better approach, or if trying to draw every child is the only right solution.

@@ +195,3 @@
>  
> +    nsIntRegion region = sTopWindows[0]->mDirtyRegion;
> +    sTopWindows[0]->mDirtyRegion.SetEmpty();

This stuff only seems to apply to non-OMTC, so it might be worth getting rid of mDirtyRegion later.

@@ -453,5 @@
>      switch (aDataType) {
>      case NS_NATIVE_WINDOW:
>          return GetGonkDisplay()->GetNativeWindow();
> -    case NS_NATIVE_WIDGET:
> -        return this;

This was removed since we don't really have a native widget/window object. Removing it allows aParent in nsWindow::Create to be set, which then allows BaseCreate to AddChild to the parent. Child windows weren't being added to the parent before.

@@ +763,4 @@
>          sVirtualBounds = gScreenBounds;
>      }
>  
> +    nsAppShell::NotifyScreenRotation();

Part of this notifies hal of the screen rotation. This was moved up to make sure gecko is notified before invalidating the window.
Attachment #8358266 - Flags: review?(roc)
Comment on attachment 8358266 [details] [diff] [review]
Replace gWindowToRedraw with a simpler mechanism

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

::: widget/gonk/nsWindow.cpp
@@ +195,3 @@
>  
> +    nsIntRegion region = sTopWindows[0]->mDirtyRegion;
> +    sTopWindows[0]->mDirtyRegion.SetEmpty();

Yes!

@@ +421,4 @@
>      nsWindow *parent = mParent;
>      while (parent && parent != sTopWindows[0])
>          parent = parent->mParent;
> +    if (parent != sTopWindows[0] && this != sTopWindows[0])

We're assuming here that there are no grandchild windows?

If so, let's assert that somewhere during window creation.
Attachment #8358266 - Flags: review?(roc) → review+
Comment on attachment 8358266 [details] [diff] [review]
Replace gWindowToRedraw with a simpler mechanism

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

::: widget/gonk/nsWindow.cpp
@@ +421,4 @@
>      nsWindow *parent = mParent;
>      while (parent && parent != sTopWindows[0])
>          parent = parent->mParent;
> +    if (parent != sTopWindows[0] && this != sTopWindows[0])

I wasn't making that assumption, though it does seem true in practice. "parent" isn't actually the parent of this - it's the top node of the window tree. I'll s/parent/top/ to make this more clear.
Attachment #8358266 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/efdeaff02f48
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.3 C2/1.4 S2(17jan)
Depends on: 958771
Backed out for causing bug 958771.
https://hg.mozilla.org/integration/b2g-inbound/rev/fbf1ad27d7ac
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: 1.3 C2/1.4 S2(17jan) → ---
Duplicate of this bug: 988883
blocking-b2g: --- → 1.4?
Whiteboard: [TPE_GFX], [ perf-reviewed ] → [TPE_GFX], [ perf-reviewed ] [CR 624040]
Ravi

Please help understand with QC what the expected time is? We've lived with 20sec for 3 releases now.
Flags: needinfo?(rdandu)
(In reply to Preeti Raghunath(:Preeti) from comment #49)
> Ravi
> 
> Please help understand with QC what the expected time is? We've lived with
> 20sec for 3 releases now.

I think you meant to say 2 seconds, not 20 seconds.
Component: General → Widget: Gonk
Product: Firefox OS → Core
The idea should be to try to improve the product over subsequent releases. Please remember that this is being launched on a quad core device and this does not happen on other platforms.
blocking-b2g: 1.4? → 1.4+
mwu: as discussed on irc, lets get to the bottom of this and fix it for 1.4
Flags: needinfo?(rdandu)
(In reply to Jason Smith [:jsmith] from comment #50)
> (In reply to Preeti Raghunath(:Preeti) from comment #49)
> > Ravi
> > 
> > Please help understand with QC what the expected time is? We've lived with
> > 20sec for 3 releases now.
> 
> I think you meant to say 2 seconds, not 20 seconds.

Hi, Can any explain me the delay seen in https://bugzilla.mozilla.org/show_bug.cgi?id=988883#c0 ?

I think that delay between nsScreenGonk::SetRotation(uint32_t aRotation) [1] and HwcComposer2D::TryRender() needs to be minimized. 

[1] https://github.com/mozilla/integration-mozilla-inbound/blob/master/widget/gonk/OrientationObserver.cpp#L253
Mwu,

Please make sure to close this bug once the dup is fixed
Flags: needinfo?(mwu)
(In reply to Preeti Raghunath(:Preeti) from comment #54)
> Mwu,
> 
> Please make sure to close this bug once the dup is fixed

I don't know what you mean. What dup?
Flags: needinfo?(mwu)
(In reply to Michael Wu [:mwu] from comment #55)
> (In reply to Preeti Raghunath(:Preeti) from comment #54)
> > Mwu,
> > 
> > Please make sure to close this bug once the dup is fixed
> 
> I don't know what you mean. What dup?

See that dup is canceled. Please make sure this is on your radar.
(In reply to Preeti Raghunath(:Preeti) from comment #56)
> (In reply to Michael Wu [:mwu] from comment #55)
> > (In reply to Preeti Raghunath(:Preeti) from comment #54)
> > > Mwu,
> > > 
> > > Please make sure to close this bug once the dup is fixed
> > 
> > I don't know what you mean. What dup?
> 
> See that dup is canceled. Please make sure this is on your radar.

I still don't understand what you're saying. Obviously, it is on my radar - see comment 52.
Can you please help me for #comment 53
Flags: needinfo?(mwu)
(In reply to Tapas Kumar Kundu from comment #53)
> Hi, Can any explain me the delay seen in
> https://bugzilla.mozilla.org/show_bug.cgi?id=988883#c0 ?
> 

Most of the delay in 180 degree rotation is caused by this bug, due to not invalidating the window. Delays in any other rotations are caused by other issues.
Flags: needinfo?(mwu)
Unbitrotted.
Attachment #8358294 - Attachment is obsolete: true
Accidentally broke the patch while unbitrotting. This one works.
Attachment #8404211 - Attachment is obsolete: true
https://hg.mozilla.org/integration/b2g-inbound/rev/0ea0efa73e05

I disabled the 336744-1.html crashtest which caused the frequent intermittent orange reported in bug 958771 . It tests something which isn't supported on the gonk widget backend (multiple top level windows) and only just happened to not crash before this patch.
https://hg.mozilla.org/mozilla-central/rev/0ea0efa73e05
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Duplicate of this bug: 988883
Duplicate of this bug: 993923
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.