Closed Bug 872003 Opened 11 years ago Closed 11 years ago

Fade from light to dark could be smoother / faster

Categories

(Firefox for Android Graveyard :: Reader View, defect)

x86
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 24

People

(Reporter: ibarlow, Assigned: nickecarlo)

Details

(Whiteboard: reader [mentor=margaret][lang=css])

Attachments

(1 file, 1 obsolete file)

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?
(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?
Whiteboard: reader → reader [mentor=margaret][lang=css]
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)?
(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.)
Assignee: nobody → nickecarlo
(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).
Attached patch WIP (obsolete) — Splinter Review
Attachment #753834 - Flags: feedback?(margaret.leibovic)
Flags: needinfo?(ibarlow)
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 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.
Attachment #753834 - Flags: feedback?(margaret.leibovic) → feedback+
(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).
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.
Flags: needinfo?(ibarlow)
(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.
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.
(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.
(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
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.
Flags: needinfo?(margaret.leibovic)
Forgot to mention that the transition gets smoother after a couple of times of switching between modes.
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
Flags: needinfo?(margaret.leibovic) → needinfo?(ibarlow)
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.
4 is my preference, too. Thanks for leaving options to try!
Flags: needinfo?(ibarlow)
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.
Attachment #753834 - Attachment is obsolete: true
Attachment #755399 - Flags: review?(margaret.leibovic)
Attachment #755399 - Attachment is patch: true
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 :)
Attachment #755399 - Flags: review?(margaret.leibovic) → review+
(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?
(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 :)
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/0316bf3f5b48
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 24
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: