All users were logged out of Bugzilla on October 13th, 2018

Only repaint dirty parts

RESOLVED FIXED in fennec1.0b5

Status

RESOLVED FIXED
9 years ago
5 years ago

People

(Reporter: stechz, Assigned: stechz)

Tracking

Trunk
fennec1.0b5
x86
Mac OS X

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

9 years ago
Created attachment 402955 [details] [diff] [review]
Only repaint dirty parts of tiles

When we receive a repaint event, we currently invalidate entire tiles.  This patch invalidates only the portion of the tile that is actually dirty.
Attachment #402955 - Flags: review?(pavlov)
(Assignee)

Updated

9 years ago
tracking-fennec: --- → ?
(Assignee)

Updated

9 years ago
Attachment #402955 - Attachment is patch: true
Attachment #402955 - Attachment mime type: application/octet-stream → text/plain

Updated

9 years ago
Attachment #402955 - Flags: review?(pavlov) → review+
(Assignee)

Comment 1

9 years ago
Are we trying this for b4?
(Assignee)

Comment 2

9 years ago
Created attachment 404689 [details] [diff] [review]
Increase tile size and better dirtying of rectangle

Fennec becomes much more responsive for panning with a larger tilesize, so I've increased tilesize to 512x512.  Since we only rerender dirty parts, there is little downside.
Attachment #402955 - Attachment is obsolete: true
(In reply to comment #2)

> Fennec becomes much more responsive for panning with a larger tilesize, so I've
> increased tilesize to 512x512.  Since we only rerender dirty parts, there is
> little downside.

Much more responsive on n810/n900? or on desktop?
Comment on attachment 404689 [details] [diff] [review]
Increase tile size and better dirtying of rectangle

>+    if (tile.isDirty()) {
>+/*      let ctx = tile._canvas.getContext("2d");
>+      ctx.save();
>+      ctx.fillStyle = "rgba(0,255,0,.5)";
>+      ctx.translate(-tile.boundRect.left, -tile.boundRect.top);
>+      ctx.fillRect(tile._dirtyTileCanvasRect.left, tile._dirtyTileCanvasRect.top,
>+        tile._dirtyTileCanvasRect.width, tile._dirtyTileCanvasRect.height);
>+      ctx.restore();
>+      window.setTimeout(function(bv) {
>+        tile.render(bv);
>+      }, 1000, this._browserView); */
>+

I assume we can remove this commented block of code?

>-    } else {  // XXX this case is not very well-tested
>+    } else {
> 
>       if (!this._dirtyTileCanvasRect)
>-        this._dirtyTileCanvasRect = dirtyRect.intersect(this.boundRect);
>+        this._dirtyTileCanvasRect = dirtyRect.intersect(this.boundRect).round();
>       else if (dirtyRect.intersects(this.boundRect))
>-        this._dirtyTileCanvasRect.expandToContain(dirtyRect.intersect(this.boundRect));
>+        this._dirtyTileCanvasRect.expandToContain(dirtyRect.intersect(this.boundRect)).round();
> 
>     }

We can remove the extra whitespace at the top and bottom of this block

> 
>+    ctx.translate(x, y);
>     browserView.browserToViewportCanvasContext(ctx);
> 
>-    ctx.translate(x, y);
> 

And remove the extra blank line that will appear here

>     mouseDown: function mouseDown(cX, cY) {
>+//      this._dispatchMouseEvent("mousedown", cX, cY);
>+//      Util.executeSoon(this._browserView.renderNow);

and remove this

I wish we had tests to support 128, 256 or 512 as better for speed vs memory - but oh well
Attachment #404689 - Flags: review+
pushed with whitespace tweaks:
https://hg.mozilla.org/mobile-browser/rev/95f148771c77
Assignee: nobody → webapps
Target Milestone: --- → B5
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
If we feel this is sluggish on device, we can always backout the 512px canvas size.
What do you mean by dirty portions of tiles?
(Assignee)

Comment 8

9 years ago
> What do you mean by dirty portions of tiles?

Before this patch, if even the cursor needed to be repainted, we repainted the entire tile the cursor was located in.  Now, only the cursor is repainted.  If I did it right, there should be no discernible difference after this patch is applied.

> If we feel this is sluggish on device, we can always backout the 512px canvas
size.

At least in my testing (and dougt played some with it too) on the N900 it actually felt faster panning.  Even the actual rendering of tiles is as fast as 128x128 in my testing using Date.now dumps.   This could be explained by the reduced drawWindow calls.

> I assume we can remove this commented block of code?

Yes.  They are useful for figuring out what is being repainted, so I wouldn't mind seeing this as a debug option.
tracking-fennec: ? → ---
You need to log in before you can comment on or make changes to this bug.