Closed
Bug 705171
Opened 12 years ago
Closed 12 years ago
Use RENDERMODE_WHEN_DIRTY instead of RENDERMODE_CONTINUOUSLY
Categories
(Firefox for Android Graveyard :: General, defect, P1)
Tracking
(firefox11 fixed, fennec11+)
RESOLVED
FIXED
People
(Reporter: pcwalton, Assigned: cwiiis)
References
Details
Attachments
(1 file, 1 obsolete file)
13.93 KB,
patch
|
kats
:
review+
pcwalton
:
review+
|
Details | Diff | Splinter Review |
Currently, we constantly render even when the screen hasn't changed. This is a waste in many ways: battery life, most importantly, but also stealing CPU time from Gecko and other apps for no reason. We should call setRenderMode(RENDERMODE_WHEN_DIRTY) when setting up the LayerView and insert requestRender() calls as needed instead. This is a high priority bug IMHO; we can't ship if we kill the battery.
Comment 1•12 years ago
|
||
Once this is resolved, what is the best way to test this patch?
Updated•12 years ago
|
Assignee: nobody → pwalton
Priority: -- → P1
Assignee | ||
Comment 2•12 years ago
|
||
This does as suggested. I've not been able to get the FPS counter quite quite, but I wanted to get the patch up for review and we can work that bit out.
Attachment #577628 -
Flags: review?(pwalton)
Assignee | ||
Comment 3•12 years ago
|
||
On second thoughts, I think the frame counter is sort-of right, but doesn't take into account the time from render to vblank. Suggestions welcome.
Comment 4•12 years ago
|
||
Comment on attachment 577628 [details] [diff] [review] Draw on demand instead of continuously Overall looks pretty good to me. I can't see any issues with the patch. Just one minor nit below. > private void checkFPS() { >+ mFrameTime += mView.getRenderTime(); >+ mFrameCount ++; >+ > if (System.currentTimeMillis() >= mFrameCountTimestamp + 1000) { > mFrameCountTimestamp = System.currentTimeMillis(); > >+ mFrameTime = 1000000000L / (mFrameTime / mFrameCount); I'd prefer this line read more like "mFrameCount = (int)(mFramecount * 1000000000L / mFrameTime); // extrapolate FPS based on time taken by frames drawn" ... >- mFPSLayer.setText(mFrameCount + " FPS"); >+ mFPSLayer.setText(mFrameTime + " FPS"); ... and keep using mFrameCount in this line. It just seems odd to have a time variable being used as "frames per second".
Assignee | ||
Comment 5•12 years ago
|
||
Addressed the comment and fixed that I was using the incorrect beginTransaction function, which caused page updates without viewport updates to miss triggering a redraw.
Assignee: pwalton → chrislord.net
Attachment #577628 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #577628 -
Flags: review?(pwalton)
Attachment #577984 -
Flags: review?(kgupta)
Comment 6•12 years ago
|
||
Comment on attachment 577984 [details] [diff] [review] Draw on demand instead of continuously (address review comments) r+, but you should probably still get feedback from pcwalton on this before landing.
Attachment #577984 -
Flags: review?(kgupta) → review+
Assignee | ||
Comment 7•12 years ago
|
||
Comment on attachment 577984 [details] [diff] [review] Draw on demand instead of continuously (address review comments) Agreed.
Attachment #577984 -
Flags: review?(pwalton)
Reporter | ||
Comment 8•12 years ago
|
||
Comment on attachment 577984 [details] [diff] [review] Draw on demand instead of continuously (address review comments) Review of attachment 577984 [details] [diff] [review]: ----------------------------------------------------------------- r=me
Attachment #577984 -
Flags: review?(pwalton) → review+
Reporter | ||
Comment 9•12 years ago
|
||
Let's get this landed ASAP, this is a big win in all sorts of ways -- thanks for taking this on.
Assignee | ||
Comment 10•12 years ago
|
||
http://hg.mozilla.org/projects/birch/rev/b747973fdd09
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Updated•12 years ago
|
tracking-fennec: --- → 11+
Updated•12 years ago
|
status-firefox11:
--- → fixed
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•