Closed
Bug 624454
Opened 13 years ago
Closed 13 years ago
Tune displayport size
Categories
(Firefox for Android Graveyard :: Panning/Zooming, defect)
Tracking
(fennec2.0+)
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
fennec | 2.0+ | --- |
People
(Reporter: cjones, Assigned: wesj)
References
Details
(Whiteboard: [part 1 landed - more coming?])
Attachments
(2 files, 2 obsolete files)
926 bytes,
patch
|
mfinkle
:
review+
|
Details | Diff | Splinter Review |
794 bytes,
patch
|
stechz
:
review+
|
Details | Diff | Splinter Review |
There are two desiderata to be balanced (1) Larger displayport allows fast-panning longer in the chrome process before requesting re-rendering from content (2) Smaller displayport allows content to re-render more quickly Tuning this size is really something we want to measure quantitatively over a benchmark suite.
Reporter | ||
Comment 1•13 years ago
|
||
Oops, there's also (3) Smaller displayport requires less memory
tracking-fennec: --- → ?
Updated•13 years ago
|
tracking-fennec: ? → 2.0+
Updated•13 years ago
|
Assignee: nobody → jones.chris.g
Reporter | ||
Comment 2•13 years ago
|
||
This is a fennec frontend project, out of my wheelhouse.
Assignee: jones.chris.g → nobody
Updated•13 years ago
|
Assignee: nobody → ben
Comment 3•13 years ago
|
||
I tried toolkit.browser.cacheRatioHeight", 4000, and it feels much better on simple page, at least single kinetic scroll in landscape mode does not show checkerboard. time: avail: size: rss: clean: shared: private: change: 4000 height content 20:14:06 203252 145096 47388 540 884 23988 +0 kB chrome 20:14:22 203280 156580 60100 11868 3080 20408 +0 kB 2200 height content: 20:11:53 197260 137784 44428 524 884 21040 +0 kB chrome: 20:11:38 197268 156828 61360 12012 3076 21620 +0 kB
Updated•13 years ago
|
Assignee: ben → wjohnston
Comment 5•13 years ago
|
||
is this just a meta bug now?
Reporter | ||
Comment 6•13 years ago
|
||
Why do you think that? Finding an optimal displayport for scrolling benchmark(s) while minimizing memory usage seems pretty well scoped. What this project lacks is the scrolling benchmark(s).
Comment 7•13 years ago
|
||
(In reply to comment #5) > is this just a meta bug now? Nope. Getting new pref values, like Oleg tried in comment 3, is what this particular bug is about. Balancing memory use with decrease in checkerboarding. We have metrics for memory, but objective metrics for panning / checkerboarding are lacking. We have been using subjective evaluation for checkerboarding. We could add some timing code to see how much time is spent rendering the checkerboard background - I think it is only rendered as needed, so less checkerboarding means less time spent rendering it. Until we have objective metrics, subjective will have to be good enough: * How much vertical panning is checkerboard free? * Zoom in and try vertical panning again * How much horizontal panning is checkerboard free? * Zoom in and try again
Assignee | ||
Comment 9•13 years ago
|
||
Talked to stechz about this. He's done some pretty extensive panning in his days, and seems to think that just increasing the cache width will give us what we want.
Attachment #516307 -
Flags: review?(mark.finkle)
Comment 10•13 years ago
|
||
Comment on attachment 516307 [details] [diff] [review] Patch I'm fine with this change - horizontal checkerboarding , especially while zoomed, is bad right now. I do want to now if we can extend the vertical direction as well. Changing the height to 3000 or 4000 would help squash more checkerboarding, and it looks like memory usage does not go up very much. Tell me why we don't need (or want) to do the vertical too
Comment 11•13 years ago
|
||
(In reply to comment #10) > Changing the height to 3000 or 4000 would help squash more checkerboarding, and > it looks like memory usage does not go up very much. > > Tell me why we don't need (or want) to do the vertical too As comment 0 notes, the other disadvantage of a too-large displayport is that it takes longer to redraw. You'll run into checkerboard less often, but it may stick around longer when you do. So we should test this before making any height increase. (And we should re-evaluate after bug 634397 lands, since that will change the results.)
Assignee | ||
Comment 12•13 years ago
|
||
Oops. Wrong patch.
Attachment #516307 -
Attachment is obsolete: true
Attachment #516426 -
Flags: review?(21)
Attachment #516307 -
Flags: review?(mark.finkle)
Assignee | ||
Comment 13•13 years ago
|
||
Argh. nope. wrong bug.
Attachment #516426 -
Attachment is obsolete: true
Attachment #516428 -
Flags: review?(mark.finkle)
Attachment #516426 -
Flags: review?(21)
Comment 14•13 years ago
|
||
Comment on attachment 516428 [details] [diff] [review] Patch v1 Let's land this and get feedback on the change
Attachment #516428 -
Flags: review?(mark.finkle) → review+
Assignee | ||
Comment 15•13 years ago
|
||
http://hg.mozilla.org/mobile-browser/rev/0811c1cc3170 Wait to mark fixed? I am working on a different patch to help quantize checkerboard area shown/sec during a pan.
Status: NEW → ASSIGNED
Updated•13 years ago
|
Whiteboard: [needs-landing]
Comment 16•13 years ago
|
||
Actually height increase definitely needed + repainting visible area at first.
Updated•13 years ago
|
Whiteboard: [needs-landing] → [part 1 landed - more coming?]
Comment 17•13 years ago
|
||
what's the plan here? Can this be closed? Perhaps follow up work should be done in another bug.
Assignee | ||
Comment 18•13 years ago
|
||
Given we are expecting big changes in checkerboarding, we will likely need to revisit this later. Since we're just playing with prefs, I think the plan was to hold off on closing this until after that.
Assignee | ||
Comment 19•13 years ago
|
||
This feels a better to me.
Attachment #518441 -
Flags: review?(ben)
Comment 20•13 years ago
|
||
Comment on attachment 518441 [details] [diff] [review] Patch 2 That's what I've been playing with too.
Attachment #518441 -
Flags: review?(ben) → review+
Assignee | ||
Comment 21•13 years ago
|
||
pushed: http://hg.mozilla.org/mobile-browser/rev/34ca5bea3ebe closing this bug now.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Comment 22•13 years ago
|
||
Is there a way to test this on a user-facing side?
You need to log in
before you can comment on or make changes to this bug.
Description
•