Closed Bug 741693 Opened 8 years ago Closed 8 years ago

Adjust ease-out curve while zooming out to try and reduce checkerboarding

Categories

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

ARM
Android
defect

Tracking

()

RESOLVED FIXED
Firefox 14
Tracking Status
firefox14 --- fixed
blocking-fennec1.0 --- beta+

People

(Reporter: cjones, Assigned: kats)

References

Details

(Whiteboard: [viewport])

Attachments

(3 files)

Reproducible on any page: zoom out quickly.

android-browser looks a lot better when zooming out.  It might be repainting faster than us, and possibly using other trickery, but there is one difference that I think I can see: it simply zooms out more slowly.  Firefox-android zooms out very quickly, which feels nice and responsive, but also makes it hard for the content thread to catch up.

We should try zooming out with an ease-out curve (and maybe some artificial lag) and compare the perceptual difference.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: the_jay_bug
Reopening to track the second suggestion in comment 0, which is to adjust the ease-out curve and try some artificial lag.
Status: RESOLVED → REOPENED
Component: Graphics → General
Product: Core → Fennec Native
QA Contact: thebes → general
Resolution: DUPLICATE → ---
Summary: Firefox-android has lots of checkerboarding on zoom-out → Adjust ease-out curve while zooming out to try and reduce checkerboarding
Whiteboard: [viewport]
For reference, the ease-out curve is in PanZoomController.java; we can adjust the numbers and/or add more values into the list to slow down the zooming. Based on the comment, the current values are taken from the CSS Transitions spec.
blocking-fennec1.0: --- → ?
Assignee: nobody → ibarlow
blocking-fennec1.0: ? → beta+
Sounds good, I'm all for easing transitions. 

JP is working to set other panning and zooming parameters as prefs that we can adjust and optimize. Is that something we could build into this as well?
Kats - Can you get the preferences in place?
Assignee: ibarlow → bugmail.mozilla
Priority: -- → P1
This was turning out ugly no matter which way I did it, so I decided to go with the more modular approach of having separate sets of preferences in GeckoLayerClient and PanZoomController. It's slightly less efficient because it sends two messages instead of one but prevents excessive cross-package dependencies. Patch #2 moves the Axis preferences out of GeckoLayerClient as well.
Attachment #615183 - Flags: review?(chrislord.net)
Comment on attachment 615183 [details] [diff] [review]
(1/3) Pref the zoom animation

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

Looks good to me.

::: mobile/android/base/ui/PanZoomController.java
@@ +233,5 @@
> +                    values[i] = Float.parseFloat(st.nextToken());
> +                }
> +                ZOOM_ANIMATION_FRAMES = values;
> +            }
> +        } catch (Exception e) {

I don't like the idea of just catching any old Exception, I guess there could only be NumberFormatException (or whatever it is) here?
Attachment #615183 - Flags: review?(chrislord.net) → review+
Comment on attachment 615184 [details] [diff] [review]
(2/3) Move the axis pref-fetching into PanZoomController

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

Reducing cross-package dependency is nice, though it's a shame the code size has increased.

::: mobile/android/base/ui/PanZoomController.java
@@ +217,1 @@
>                  for (int i = jsonPrefs.length() - 1; i >= 0; i--) {

It'd be good to factor out and share this for-loop prefs checking code at some point.

@@ +231,5 @@
> +                // check for null to make sure the batch of preferences we got notified
> +                // of are in the fact the ones we requested and not those requested by
> +                // other java code
> +                if (zoomAnimationFrames != null) {
> +                    setZoomAnimationFrames(zoomAnimationFrames);

Is there a possibility of zoomAnimationFrames being null even on a successful prefs get? (i.e. is that enough to determine if this was the correct prefs data?)
Attachment #615184 - Flags: review?(chrislord.net) → review+
(In reply to Chris Lord [:cwiiis] from comment #10)
> I don't like the idea of just catching any old Exception, I guess there
> could only be NumberFormatException (or whatever it is) here?

Ok, I'll change that to just catch the NumberFormatException then.

(In reply to Chris Lord [:cwiiis] from comment #11)
> It'd be good to factor out and share this for-loop prefs checking code at
> some point.

Ideally I'd also like to be able to listen for updates to prefs so we don't have to restart the app every time. To do that cleanly I'd like to introduce a new helper class that you can register with to get prefs and listen for updates and so on, but that's post-1.0 work.

> 
> Is there a possibility of zoomAnimationFrames being null even on a
> successful prefs get? (i.e. is that enough to determine if this was the
> correct prefs data?)

I don't think so. I did find it was coming out null because of bug 744501, and fixed that. I think there's no legitimate case where it should be null (i.e. it will only happen because of other bugs).
Comment on attachment 615185 [details] [diff] [review]
(3/3) The patch cpeterson promised me months ago

r+

Sorry, kats! I could have sworn I posted a patch for this promised fix, but I guess I dropped the ball.
Attachment #615185 - Flags: review?(cpeterson) → review+
You need to log in before you can comment on or make changes to this bug.