Last Comment Bug 872003 - Fade from light to dark could be smoother / faster
: Fade from light to dark could be smoother / faster
Status: RESOLVED FIXED
reader [mentor=margaret][lang=css]
:
Product: Firefox for Android
Classification: Client Software
Component: Reader View (show other bugs)
: unspecified
: x86 Mac OS X
: -- normal (vote)
: Firefox 24
Assigned To: Nicolas Carlo [:nickecarlo]
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-05-14 07:22 PDT by Ian Barlow (:ibarlow)
Modified: 2013-05-30 09:13 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
WIP (855 bytes, patch)
2013-05-24 09:11 PDT, Nicolas Carlo [:nickecarlo]
margaret.leibovic: feedback+
Details | Diff | Splinter Review
bug-872003.patch Make fade from light to dark smoother (712 bytes, patch)
2013-05-29 08:33 PDT, Nicolas Carlo [:nickecarlo]
margaret.leibovic: review+
Details | Diff | Splinter Review

Description Ian Barlow (:ibarlow) 2013-05-14 07:22:17 PDT
When we toggle from light to dark to auto, the fade transition is a little choppy. Can we smooth it out a bit, and make it a little faster (roughly 30%) while we're at it?
Comment 1 :Margaret Leibovic 2013-05-14 08:01:53 PDT
(In reply to Ian Barlow (:ibarlow) from comment #0)
> When we toggle from light to dark to auto, the fade transition is a little
> choppy. Can we smooth it out a bit, and make it a little faster (roughly
> 30%) while we're at it?

I'm using CSS transitions, so if it's choppy, we should ask some platform friends to take a look.

Right now it's set to 0.7s, so maybe we should take it down to 500ms or 450ms?
Comment 2 Nicolas Carlo [:nickecarlo] 2013-05-23 10:55:45 PDT
I was taking a look at this bug this morning. I'd love to take this on, if possible.

Making it faster is not going to be that difficult and I'll see what I can find out about making it smoother.

Just to be sure, this is aboutReader.css file we're talking about (the body attribute)?
Comment 3 :Margaret Leibovic 2013-05-23 12:06:59 PDT
(In reply to Nicolas Carlo from comment #2)
> I was taking a look at this bug this morning. I'd love to take this on, if
> possible.

Excellent!

> Making it faster is not going to be that difficult and I'll see what I can
> find out about making it smoother.
> 
> Just to be sure, this is aboutReader.css file we're talking about (the body
> attribute)?

Yes, that's correct.

If you find a faster transition value that you think is good, you should post an APK in this bug for ibarlow (our UX designer) to play with, and he can give you feedback about whether it should be faster or slower. (I think APKs may be too large to add as a bugzilla attachment, so we usually use dropbox or another public server space as a way to share.)
Comment 4 Nicolas Carlo [:nickecarlo] 2013-05-23 16:54:57 PDT
(In reply to Ian Barlow (:ibarlow) from comment #0)
> When we toggle from light to dark to auto, the fade transition is a little
> choppy. Can we smooth it out a bit, and make it a little faster (roughly
> 30%) while we're at it?

Here's the link to the APK with the changes (described below) applied. Let me know if that's what you wanted or if you want me to change it further.

https://dl.dropboxusercontent.com/u/22352304/fennec-24.0a1.en-US.android-i386.apk

Changes made to aboutReader.css file:

- Changed transition-duration of body attribute from 0.7s to 0.4s (I tested it also with 0.25s but that was too snappy, in my opinion).

- Added transition-timing-function with the value of curve-bezier(0.215, 0.61, 0.355, 1). This function is added automatically, if not explicitly added in the file, with a default value of "ease" which makes the animation look choppy a little bit. (I picked the easeOutCubic value from this webpage http://easings.net, if you're not happy with it I can try others).
Comment 5 Nicolas Carlo [:nickecarlo] 2013-05-24 09:11:43 PDT
Created attachment 753834 [details] [diff] [review]
WIP
Comment 6 :Margaret Leibovic 2013-05-24 13:51:52 PDT
Here's a non-i386 build that I made, in case ibarlow needs that to test:
https://dl.dropboxusercontent.com/u/3358452/transition-tweak.apk
Comment 7 :Margaret Leibovic 2013-05-24 13:54:39 PDT
Comment on attachment 753834 [details] [diff] [review]
WIP

Codewise, this looks good to me. I'll wait to hear from ibarlow before giving an r+.

Trying this out myself, the transition felt a little choppy still, maybe we should play around with different timing functions.
Comment 8 Nicolas Carlo [:nickecarlo] 2013-05-25 10:21:29 PDT
(In reply to :Margaret Leibovic from comment #7)
> Comment on attachment 753834 [details] [diff] [review]
> WIP
> 
> Codewise, this looks good to me. I'll wait to hear from ibarlow before
> giving an r+.
> 
> Trying this out myself, the transition felt a little choppy still, maybe we
> should play around with different timing functions.

I think we might need to do the transitions a lot faster than they are getting done right now. There are a lot of faster options for curve-bezier that I'm going to try once I get a little bit of time.

The linear option also results in choppiness so the only thing I can think of to do is to choose values that will make the transition happen faster in the beginning and slow it down toward the end.

I'll test it and report back here. Do you want me to keep posting WIP patches or should I use the mozconfig for the nightly builds and just post links to APK's? (I was thinking of using a lot of different options and posting various different APK's in one comment for you guys to try).
Comment 9 Ian Barlow (:ibarlow) 2013-05-27 12:59:34 PDT
This looks great to me. Though I am on a Nexus 4, so if you feel it's choppy on other devices and want to smooth it out, by all means go for it.
Comment 10 Nicolas Carlo [:nickecarlo] 2013-05-28 08:57:27 PDT
(In reply to Ian Barlow (:ibarlow) from comment #9)
> This looks great to me. Though I am on a Nexus 4, so if you feel it's choppy
> on other devices and want to smooth it out, by all means go for it.

I have been testing this over the weekend on my Motorola Razr i and no matter what setting I choose it is choppy. It gets smoother after changing modes a couple of times. I even tried the setting at 0.2s which made it really snappy (probably not something the setting you'd want to release the app with) but I could still see some choppiness the first couple of times. Afterward it became more normal/smoother.

I'd like to see what Margaret thinks about all of this.
Comment 11 Mark Finkle (:mfinkle) (use needinfo?) 2013-05-28 08:59:27 PDT
Nicolas - Just wondering. Are you making true x86 builds for the Razr i ? That device will run ARM builds too, in emulation, which is pretty slow.
Comment 12 Nicolas Carlo [:nickecarlo] 2013-05-28 09:24:52 PDT
(In reply to Mark Finkle (:mfinkle) from comment #11)
> Nicolas - Just wondering. Are you making true x86 builds for the Razr i ?
> That device will run ARM builds too, in emulation, which is pretty slow.

I'm not really sure what you mean by "true x86 builds", I'm fairly new at all of this. I'll paste my .mozconfig which might answer your question:


ac_add_options --with-android-ndk="$HOME/android-ndk-r8e"
 ac_add_options --with-android-sdk="$HOME/android-sdk-macosx/platforms/android-16"
 ac_add_options --with-android-version=9

 # android options
 ac_add_options --enable-application=mobile/android
 ac_add_options --target=i386-linux-android
 ac_add_options --with-ccache

 mk_add_options MOZ_OBJDIR=./objdir-droid

ac_add_options --disable-crashreporter

I tried using an .apk built with ac_add_options --target=arm-linux-androideabi but the app kept crashing on startup.

I am, of course, open to any advice whatsoever to changes in my .mozconfig if you think my configurations are screwing up the testing on my part.
Comment 13 Mark Finkle (:mfinkle) (use needinfo?) 2013-05-28 09:31:40 PDT
(In reply to Nicolas Carlo from comment #12)
> (In reply to Mark Finkle (:mfinkle) from comment #11)
> > Nicolas - Just wondering. Are you making true x86 builds for the Razr i ?
> > That device will run ARM builds too, in emulation, which is pretty slow.
> 
> I'm not really sure what you mean by "true x86 builds", I'm fairly new at
> all of this. I'll paste my .mozconfig which might answer your question:

>  ac_add_options --target=i386-linux-android

Looks OK
Comment 14 Nicolas Carlo [:nickecarlo] 2013-05-28 14:42:20 PDT
Here are some of the builds I've made and tried (I tried them on i386, these are arm builds, it should be the same but I haven't been able to test them since I don't have a device that would run them and they also crash on emulators, probably something I'm doing wrong that I haven't been able to figure out), I'll provide the description of settings with each one:

1. https://dl.dropboxusercontent.com/u/22352304/fennec-24.0a1.en-US.android-arm2.apk
transition-duration: 0.4s;
transition-timing-function: cubic-bezier(0.19,1,0.22,1); /*easeOutExpo curve at easings.net*/

2. https://dl.dropboxusercontent.com/u/22352304/fennec-24.0a1.en-US.android-arm3.apk
transition-duration: 0.2s
transition-timing-function: cubic-bezier(0.19,1,0.22,1); /*easeOutExpo curve at easings.net*/
(NOTE: This was the only one that looked smooth on my Motorola, but it is also a bit unnatural since its really fast and snappy)

3. https://dl.dropboxusercontent.com/u/22352304/fennec-24.0a1.en-US.android-arm4.apk
transition-duration: 0.4s;
transition-timing-function: linear;

4. https://dl.dropboxusercontent.com/u/22352304/fennec-24.0a1.en-US.android-arm5.apk
transition-duration: 0.4s;
(NOTE: This gets rid of the transition-timing-function, if you guys think that this works as well as the other ones then I think it might be better to NOT have the extra function in the code. This function is implicitly called with the setting set to "ease".)

Anyway, let me know what you think.
Comment 15 Nicolas Carlo [:nickecarlo] 2013-05-28 14:56:49 PDT
Forgot to mention that the transition gets smoother after a couple of times of switching between modes.
Comment 16 :Margaret Leibovic 2013-05-28 16:07:46 PDT
I think I prefer option 4. Option 2 felt too fast, and I found that option 3 was jankier on my Galaxy Nexus (I tested on that, since it's slower than the Nexus 4, but has enough memory for reader mode to be supported).

I had trouble installing option 1. Here's the error I got:
$ adb install ~/Downloads/fennec-24.0a1.en-US.android-arm2.apk 
EOCD not found, not Zip
file '/Users/leibovic/Downloads/fennec-24.0a1.en-US.android-arm2.apk' is not a valid zip file
rm failed for /data/local/tmp/fennec-24.0a1.en-US.android-arm2.apk, No such file or directory
Comment 17 Nicolas Carlo [:nickecarlo] 2013-05-28 17:41:07 PDT
Not sure what the problem with the file for the first option is (this link says that it could be a corrupted file http://www.anddev.org/sdk-adt-emulator-problems-f16/adb-install-not-working-for-downloaded-files-t16845.html the file may have gotten corrupted while downloading?). But I tested it here with the emulator it installed fine (it doesn't run of course, which I'm not sure why these files don't run on emulators). I have uploaded the file again (the link's below), the file should be 23742731 bytes in size.

https://dl.dropboxusercontent.com/u/22352304/fennec-24.0a1.en-US.android-arm2.apk

Hopefully you'll be able to install the file this time.

Anyway, I'll wait for ibarlow's final verdict on it all and then post a patch.
Comment 18 Ian Barlow (:ibarlow) 2013-05-29 06:54:59 PDT
4 is my preference, too. Thanks for leaving options to try!
Comment 19 Nicolas Carlo [:nickecarlo] 2013-05-29 08:33:13 PDT
Created attachment 755399 [details] [diff] [review]
bug-872003.patch Make fade from light to dark smoother

This patch changes the transition-duration to 0.4s. It is the same settings as the ones applied on option # 4 in the APK files posted for testing.
Comment 20 :Margaret Leibovic 2013-05-29 12:09:59 PDT
Comment on attachment 755399 [details] [diff] [review]
bug-872003.patch Make fade from light to dark smoother

Thanks for being so thorough investigating different options, and for posting builds for us to try. It's awesome to see such attention to detail :)
Comment 21 Nicolas Carlo [:nickecarlo] 2013-05-29 12:24:10 PDT
(In reply to :Margaret Leibovic from comment #20)
> Comment on attachment 755399 [details] [diff] [review]
> bug-872003.patch Make fade from light to dark smoother
> 
> Thanks for being so thorough investigating different options, and for
> posting builds for us to try. It's awesome to see such attention to detail :)

You're welcome. I'm loving being able to contribute however much I can.

Do I (and can I) mark bugs as checkin-needed after getting r+ or do you (or the mentors) have to do that?
Comment 22 :Margaret Leibovic 2013-05-29 13:44:58 PDT
(In reply to Nicolas Carlo from comment #21)
> (In reply to :Margaret Leibovic from comment #20)
> > Comment on attachment 755399 [details] [diff] [review]
> > bug-872003.patch Make fade from light to dark smoother
> > 
> > Thanks for being so thorough investigating different options, and for
> > posting builds for us to try. It's awesome to see such attention to detail :)
> 
> You're welcome. I'm loving being able to contribute however much I can.
> 
> Do I (and can I) mark bugs as checkin-needed after getting r+ or do you (or
> the mentors) have to do that?

Yes, you can do that as soon as you have a patch that's ready to land (checkin-needed is just a way for people without commit access to get their patches landed). I see your patch already has a correctly formatted commit message, so it's ready to be checked in!

(I'll just mark it, since I'm already here :)
Comment 24 Ryan VanderMeulen [:RyanVM] 2013-05-30 09:13:16 PDT
https://hg.mozilla.org/mozilla-central/rev/0316bf3f5b48

Note You need to log in before you can comment on or make changes to this bug.