990338 - Use Moz2D for updating CanvasClient

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Comment 1

[Matt Woodrow (:mattwoodrow)]

Attachment: Use Moz2d for CanvasClient

This will bring back the performance regression from last time we tried this, the following patches clean everything up and fix it.

Comment 2

[Matt Woodrow (:mattwoodrow)]

Attachment: Clean up CopyableCanvasLayer

This removes a bunch of dead code and simplifies more.

And important change is UpdateTarget previously was calling PaintWithOpacity which applied the mask layer (incorrectly), the Y-flip, and maybe some opacity and an operator (but we never passed values that used this functionality).

This patches no longer does this, since these are all things that should happen in the layers system, and not things we need to do to the texture client's data.

Comment 3

[Matt Woodrow (:mattwoodrow)]

Attachment: Remove FillWithMask

This is dead code now.

Comment 4

[Matt Woodrow (:mattwoodrow)]

Attachment: Pass the flip flag through layers

This fixes a regression with BasicCompositor with the earlier patch. Since we longer do the Y-flip when copying to the TextureClient we need to instead set it as a texture flag.

Comment 5

[Matt Woodrow (:mattwoodrow)]

Attachment: Use CopySurface instead of FillRect

FillRect/DrawSurface with OPERATOR_SOURCE is slow because it usually creates a temporary surface to emulate unboundedness. Using CopySurface instead fixes the performance issue.

We can also fix our Moz2D backends to be smarter here, I'll probably do that as well in another bug.

Comment 6

[Nicolas Silva [:nical]]

Comment on attachment 8399755 [details] [diff] [review]
Clean up CopyableCanvasLayer

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

sweet

Comment 7

[Matt Woodrow (:mattwoodrow)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e915878751ba
https://hg.mozilla.org/integration/mozilla-inbound/rev/f19aba4974b8
https://hg.mozilla.org/integration/mozilla-inbound/rev/80b89d34acbb
https://hg.mozilla.org/integration/mozilla-inbound/rev/cd723ed732e3
https://hg.mozilla.org/integration/mozilla-inbound/rev/d1f8ac35bdd4

Comment 8

[Daniel Holbert [:dholbert]]

This may have caused a small MozAfterPaint talos regression on Ubuntu, per thread here:
  https://groups.google.com/d/msg/mozilla.dev.tree-management/7UA_3MOPU9I/ED1Q2bzCn98J

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/e915878751ba
https://hg.mozilla.org/mozilla-central/rev/f19aba4974b8
https://hg.mozilla.org/mozilla-central/rev/80b89d34acbb
https://hg.mozilla.org/mozilla-central/rev/cd723ed732e3
https://hg.mozilla.org/mozilla-central/rev/d1f8ac35bdd4

Comment 10

[Firefox Commit Bot]

The following changesets are now in Firefox Nightly:

> e915878751ba Bug 990338 - Use Moz2D for updating CanvasClient. r=nical
> f19aba4974b8 Bug 990338 - Clean up CopyableCanvasLayer. r=nical
> 80b89d34acbb Bug 990338 - Remove FillWithMask since it's dead code. r=nical
> cd723ed732e3 Bug 990338 - Pass CanvasLayer Y-flip flag through layers. r=nical
> d1f8ac35bdd4 Bug 990338 - Use CopySurface when copying data to a TextureClient for better performance. r=nical

Nightly Build Information:

        ID: 20140402030201
 Changeset: 4941a2ac0786109b08856738019b016a6c5a66a6
   Version: 31.0a1
      TBPL: https://tbpl.mozilla.org/?rev=4941a2ac0786
       URL: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-02-03-02-01-mozilla-central

Download Links:

>         Linux x86: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-02-03-02-01-mozilla-central/firefox-31.0a1.en-US.linux-i686.tar.bz2
>      Linux x86_64: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-02-03-02-01-mozilla-central/firefox-31.0a1.en-US.linux-x86_64.tar.bz2
> Linux x86_64 ASAN: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-02-03-02-01-mozilla-central/firefox-31.0a1.en-US.linux-x86_64-asan.tar.bz2
>               Mac: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-02-03-02-01-mozilla-central/firefox-31.0a1.en-US.mac.dmg
>             Win32: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-02-03-02-01-mozilla-central/firefox-31.0a1.en-US.win32.installer.exe
>             Win64: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-02-03-02-01-mozilla-central/firefox-31.0a1.en-US.win64-x86_64.installer.exe

Previous Nightly Build Information:

        ID: 20140401030203
 Changeset: 1417d180a1d8665b1a91b897d1cc4cc31e7980d4
   Version: 31.0a1
      TBPL: https://tbpl.mozilla.org/?rev=1417d180a1d8
       URL: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-01-03-02-03-mozilla-central

Comment 11

[Joel Maher ( :jmaher ) (UTC -8)]

as per comment 5, it seems that we are expecting a perf hit here.  On OSX 10.6, we have a canvasmark hit:
http://graphs.mozilla.org/graph.html#tests=[[297,63,21]]&sel=1395673504140,1396883104140&displayrange=7&datatype=running

retriggers showing the failure:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&fromchange=f5c5742ad004&tochange=e06713a76a41&jobname=mozilla-inbound%20talos%20chromez

Comment 12

[Joel Maher ( :jmaher ) (UTC -8)]

Hi Matt,

Are we expecting a 5% hit on osx 10.6 for this?  Is that well understood?  I am fine closing this as resolved as long we folks are aware of it.

Comment 13

[Matt Woodrow (:mattwoodrow)]

It wasn't expected, but we've filed bug 990854 to fix it.