Open Bug 783193 Opened 7 years ago Updated Last year

TabsTray opening/closing animation is not simultaneous for content and awesomebar

Categories

(Firefox for Android :: General, defect, P5)

ARM
Android
defect

Tracking

()

Tracking Status
firefox14 --- unaffected
firefox15 --- unaffected
firefox16 --- unaffected
firefox17 --- affected
firefox18 --- affected
fennec + ---

People

(Reporter: pretzer, Unassigned)

References

(Depends on 1 open bug)

Details

(Keywords: regression, Whiteboard: ui-responsiveness)

Attachments

(4 files)

TabsTray opening/closing animation is not simultaneous for content and awesomebar. The content animation seems to start earlier and "drag" the awesomebar animation behind it. The animation is rather quick so you have to watch closely but it's pretty visible on my phone.
I've made two mockups (one for the tray-opening and one for the tray-closing animation) of how this looks like, to make it more clear what I mean. 
(Note: These are no real screenshots but mockups by me, because it's impossible for me to capture the animation in a screenshot)

This must have regressed somewhere, because I don't think I saw this in the beginning when the animation landed first.
So I tested this on nightly. I did not check Aurora and Beta, but they are probably affected as well, since the animation is there too.

Also this is not happening on 'about:home'. I tested with 'google.com'
tracking-fennec: --- → ?
OS: Windows XP → Android
Hardware: x86 → ARM
I think I see what you mean but I wonder if it's just because I want to see it now. :P

I wonder if it's more noticeable on certain devices, due to performance. What device are you using to test?
I tested the current nightly on my Samsung Galaxy S2 with Android 4.0.3

I also tried Beta on the same device just now and the problem is not/hardly visible there for me.
I also tried Aurora and it's just like Beta. While it does feel a *tiny* bit laggish on Beta/Aurora when opening the tabs tray, I think it has always been like that and is nothing compared to Nightly.
That means something definitely regressed this for me in 17.

Can someone else reproduce this on Nightly (17) or is it just me?
The repositioning of the page in Gecko Layer (which is a SurfaceView) is done by Gecko -- which requires back and forth messaging. Hence this is the best it can get.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 767980
(In reply to Sriram Ramasubramanian [:sriram] from comment #6)
> Hence this is the best it can get.

While this is probably true for the animation in FF15/FF16, this is not what this bug is about. I'm talking about a clearly visible *regression* in the synchronicity of the animation. 
The animation is fine in 15/16 and regressed somewhere in the development cycle for FF17.


(In reply to Lucas Rocha (:lucasr) from comment #7)
> 
> *** This bug has been marked as a duplicate of bug 767980 ***

Therefore I do not think that this is a dupe. IMO we should track the specific regression here and investigate general improvements to the animation over there.


FYI: I'm seeing this on my Galaxy S2, Android 4.0.3, maybe it's device specific.
I can also reproduce this in the new Aurora (17) build now, so this is not due to some corruption in my Nightly build. It's moving the train.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Assignee: nobody → lucasr.at.mozilla
Whiteboard: ui-responsiveness
based on the conversation esp the comment in c4 from https://bugzilla.mozilla.org/show_bug.cgi?id=767980#c4  it sounds like this is a duplicate.

You can see the jankiness in 15/16 if you switch tabs.  It did get worse in 17/18.
(In reply to Sriram Ramasubramanian [:sriram] from comment #6)
> The repositioning of the page in Gecko Layer (which is a SurfaceView) is
> done by Gecko -- which requires back and forth messaging. Hence this is the
> best it can get.

Is this really the best it can get? Is there no way to synchronise the drawing during this animation?
(In reply to Chris Lord [:cwiiis] from comment #10)
> (In reply to Sriram Ramasubramanian [:sriram] from comment #6)
> > The repositioning of the page in Gecko Layer (which is a SurfaceView) is
> > done by Gecko -- which requires back and forth messaging. Hence this is the
> > best it can get.
> 
> Is this really the best it can get? Is there no way to synchronise the
> drawing during this animation?

Investigating this now.
tracking-fennec: ? → 18+
lucasr, would it be fruitful for QA to try to get a regression window for this?  Or is this something that dev already has a good understanding in how to fix?  Also if we were to investigate the issue, how would we go about benchmarking the asynch behavior?
FYI: this bug will be fixed once we start using TextureView and hardware acceleration for the animation (see bug 767980). There's no way to fix this when when we use SurfaceView.
Removing regerssionwindow-wanted based on comment 13.
Lucas, the landing of bug 767980 did not improve the synchronicity of the animation for me. In fact it's even slightly more visible because the animation seems to be a tad bit slower than before.
If I can be of any further help to debug this, let me know.
Status: REOPENED → NEW
Flags: needinfo?(lucasr.at.mozilla)
(In reply to Peter Retzer (:pretzer) from comment #15)
> Lucas, the landing of bug 767980 did not improve the synchronicity of the
> animation for me. In fact it's even slightly more visible because the
> animation seems to be a tad bit slower than before.
> If I can be of any further help to debug this, let me know.

The reason this is more visible now is because the animation is smoother (not slower). You're now able to see much more clearly the content lagging behind on each animation frame. Unfortunately, as I said on comment #13, there's no way to get this fixed while using SurfaceView.

This will be fixed once we enabled TextureView, which is explicitly disabled right now due to performance issues (see bug 792259). Note: TextureView is only available on ICS+ so devices with pre-ICS Android will always have this issue.

Setting the bug dependency for future reference.
Depends on: 792259
Flags: needinfo?(lucasr.at.mozilla)
tracking-fennec: 18+ → 20+
Taking this, got a patch that fixes it. I expect the regression coincided with it taking more time to render the tabs tray, but this patch ought to fix the issue entirely. Maybe.
Status: NEW → ASSIGNED
QA Contact: chrislord.net
This is a WIP patch that I think makes it better, but I'm not certain anymore...
During unrelated investigation I found SurfaceHolder.Callback2 [1] which might be useful here.

[1] http://developer.android.com/reference/android/view/SurfaceHolder.Callback2.html
tracking-fennec: 20+ → +
QA Contact: chrislord.net
I thought of another way to fix this - Rather than scrolling the SurfaceView, we could just offset in the GL code, then what the attached WIP patch tries to do (force a synchronous GL update before the next refresh) will work.

This ought to fix the gap and the various SurfaceView scroll issues there are around, though will be slower if the compositor can't keep up a good frame-rate while compositing.
Assignee: lucasr.at.mozilla → nobody
Status: ASSIGNED → NEW
Duplicate of this bug: 956939
filter on [mass-p5]
Priority: -- → P5
Our UI changed, this bug isn't valid anymore.
Status: NEW → RESOLVED
Closed: 7 years ago4 years ago
Resolution: --- → INVALID
I still see this issue on my GS4.

This bug is about the LayerView not animating with the toolbar, causing a space between the content and the toolbar.

If I had to guess, different interpolators or different animators, causing slight synchronization issues. Maybe related to bug 1222652?
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
This isn't a polish item. It sounds like it's just broken so we should look at fixing this.

CC-ing Barbara here so she's aware

Ni-ing Kevin to see if it's still a thing
No longer blocks: fennec-polish
Flags: needinfo?(kbrosnan)
We don't have content in the tab view any more.
Status: REOPENED → RESOLVED
Closed: 4 years ago3 years ago
Flags: needinfo?(kbrosnan)
Resolution: --- → INVALID
(In reply to Kevin Brosnan [:kbrosnan] from comment #26)
> We don't have content in the tab view any more.

This bug still exists as far as I know, what did you mean by this?
Flags: needinfo?(kbrosnan)
The tab view only shows tabs. There is no view into the gecko content, thus no errors miscalculating the height of the gecko content with or without the dynamic address bar. I was basing my comment on the screenshot from comment 24.
Flags: needinfo?(kbrosnan)
(In reply to Kevin Brosnan [:kbrosnan] from comment #28)
> The tab view only shows tabs. There is no view into the gecko content, thus
> no errors miscalculating the height of the gecko content with or without the
> dynamic address bar. I was basing my comment on the screenshot from comment
> 24.

The bug exhibited in comment #24 still happens (the black space between the bottom of the location bar and the top of the page). It doesn't tend to happen as bad as in that screenshot, but the compositing of the animation isn't synchronised with the compositing of the page during the animation, so there tends to be a small amount of judder where they mismatch (which is worse on low-end hardware, or when the phone is busy).

I'm going to reopen this one.
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
Status: REOPENED → NEW
You need to log in before you can comment on or make changes to this bug.