Closed
Bug 539062
Opened 15 years ago
Closed 15 years ago
Less checkerboarding
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(fennec1.0+)
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
fennec | 1.0+ | --- |
People
(Reporter: stechz, Assigned: stechz)
References
Details
Attachments
(1 file)
Makes sure prefetcher starts up whenever dirty tiles are created or when viewport size changes.
Assignee | ||
Updated•15 years ago
|
Attachment #421112 -
Flags: review?(pavlov)
Attachment #421112 -
Flags: review?(21)
Comment 1•15 years ago
|
||
Globally the patch looks ok for me. One question thought.
>+ /** Crawler will recalculate the tiles it is supposed to fetch in the background. */
>+ recenterCrawler: function recenterCrawler() {
>+ let cr = this._criticalRect;
>+ this._crawler = new TileManager.CrawlIterator(this._tileCache, cr.clone());
>+ },
>+
Can we avoid to create a new object each time we recenter the crawler? It doesn't looks like a cheap operation.
Assignee | ||
Comment 2•15 years ago
|
||
> Can we avoid to create a new object each time we recenter the crawler? It
> doesn't looks like a cheap operation.
The third patch does just that.
In this case it's not any worse than it was before--crawliterators are created whenever the first critical paint occurs.
Assignee | ||
Comment 3•15 years ago
|
||
(and by first, I mean the first paint since the last endCriticalMove)
Updated•15 years ago
|
tracking-fennec: --- → 1.0+
Comment 4•15 years ago
|
||
Comment on attachment 421112 [details] [diff] [review] Patch >- if (!tile.boundRect.intersects(criticalRect)) >+ if (!tile.boundRect.intersects(criticalRect)) { >+ // Dirty tile outside of viewport. Just remove and redraw later. > this._removeTileSafe(tile); >- else >+ crawler.enqueue(tile.i, tile.j); >+ outsideIsDirty = true; >+ } else { > criticalIsDirty = true; >+ } > > tile.markDirty(rects[i]); > >- if (crawler) >- crawler.enqueue(tile.i, tile.j); >- > END_FOREACH_IN_RECT > } Notice removed guard for |crawler| being null prior to |.enqueue|. Can still be null. Patch in "blocking" bug 538669 fixes this by initializing in the constructor, but as-is this is unsafe in this change alone, so perhaps add the null guard back here and remove in next patch. >- if (tile.isDirty()) { >- self._renderTile(tile); >- } > self._appendTileSafe(tile); >+ self._renderTile(tile); > } Bring back the render-before-append order as before, for consistency if anything.
Comment 5•15 years ago
|
||
Comment on attachment 421112 [details] [diff] [review] Patch (In reply to comment #2) > > Can we avoid to create a new object each time we recenter the crawler? It > > doesn't looks like a cheap operation. > > The third patch does just that. > > In this case it's not any worse than it was before--crawliterators are created > whenever the first critical paint occurs. Right.
Attachment #421112 -
Flags: review?(21) → review+
Assignee | ||
Updated•15 years ago
|
Attachment #421112 -
Flags: review?(rfrostig)
Comment 6•15 years ago
|
||
Comment on attachment 421112 [details] [diff] [review] Patch r+ with above comments.
Attachment #421112 -
Flags: review?(rfrostig) → review+
Updated•15 years ago
|
Attachment #421112 -
Flags: review?(pavlov) → review+
Assignee | ||
Comment 7•15 years ago
|
||
Pushed http://hg.mozilla.org/mobile-browser/rev/a81494dcc9be
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Updated•15 years ago
|
Flags: in-testsuite?
You need to log in
before you can comment on or make changes to this bug.
Description
•