612335 - Read back WebGL canvases directly into shared surface

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Oleg Romashin (:romaxa)]

Attachment: ReadPixels directly to mBackBuffer

I think we can create overloaded ::Updated method for BasicShadowableCanvasLayer and make it rendering with -1 composite

Comment 1

[Oleg Romashin (:romaxa)]

Attachment: Right patch

oh, previous patch was a bit hacky and crashy...

Also we still need to do y-flip

Comment 2

[Oleg Romashin (:romaxa)]

Attachment: Add y-flip for remote sw canvas

Comment 3

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Comment on attachment 490656 [details] [diff] [review]
Add y-flip for remote sw canvas

Don't we only want to y-flip for webgl canvases?  Remember that this code is used for both 2d and 3d.

Comment 4

[Doug Turner (:dougt)]

can you detect that case and only flip y for the 3d case?

Comment 5

[Oleg Romashin (:romaxa)]

Comment on attachment 490656 [details] [diff] [review]
Add y-flip for remote sw canvas

This patch is obsolete, and right patch about canvas y-flip is in bug 606218

Comment 6

[Oleg Romashin (:romaxa)]

Attachment: Updated patch

I know nobody care about canvas webgl rendering right now, because in long future we are going to move WegGL context to Chrome, and do IPC calls for each webgl command... anyway here is the patch which allow us cutoff one memcpy in remote fennec

Comment 7

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Comment on attachment 502206 [details] [diff] [review]
Updated patch

We don't want to duplicate all this code; it should be easy to refactor BasicCanvasLayer::Updated to avoid duplication.

Please request review on v2 from :vlad, since judging by bug 606218 comment 6 he might not want this patch yet.

Comment 8

[Oleg Romashin (:romaxa)]

Attachment: Optimize remote canvas rendering into shared surface

Comment 9

[Oleg Romashin (:romaxa)]

Comment on attachment 536205 [details] [diff] [review]
Optimize remote canvas rendering into shared surface

Canvas YFlip moved to Chrome process, that should not be a problem because it just different composite direction in SW mode, and GPU does it for free.

Comment 10

[Oleg Romashin (:romaxa)]

With simple top command I can see that this patch drop plugin-container CPU usage from 30%->20%

Comment 11

[Oleg Romashin (:romaxa)]

Attachment: Optimize remote canvas rendering into shared surface

Ups, previous version was breaking normal canvas2D which was stored in mSurface, this version fixing that problem

Comment 12

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Comment on attachment 536243 [details] [diff] [review]
Optimize remote canvas rendering into shared surface

> void
> BasicShadowableCanvasLayer::Paint(gfxContext* aContext)
> {
>-  BasicCanvasLayer::Paint(aContext);
>   if (!HasShadow())
>     return;

This will completely break same-process <canvas> rendering ...

>-  // It'd be nice to draw directly into the shmem back buffer.
>-  // Doing so is complex -- for 2D canvases, we'd need to copy
>-  // changed areas, much like we do for Thebes layers, as well as
>-  // do all sorts of magic to swap out the surface underneath the
>-  // canvas' thebes/cairo context.

This comment is still valid, but I don't think it's really
helping anyone.

>+  if (!backSurface || backSurface->GetType() != gfxASurface::SurfaceTypeImage) {

Probably better to drop the ImageSurface check and let
UpdateSurface() deal.  We'll soon be sharing X surfaces, e.g.
I'm fine fixing this in a followup if you file it.

r- for breaking same-process rendering.

Comment 13

[Oleg Romashin (:romaxa)]

> > void
> > BasicShadowableCanvasLayer::Paint(gfxContext* aContext)
> > {
> >-  BasicCanvasLayer::Paint(aContext);
> >   if (!HasShadow())
> >     return;
> 
> This will completely break same-process <canvas> rendering ...
err... how? BasicShadowableCanvasLayer::Paint - shadowable is only for cross process rendering isn't it?

Comment 14

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

In MOZ_IPC and now all builds since MOZ_IPC is gone, all layers are Shadowable by default, except for those used by temporary layer managers.  With same-process rendering, Shadowable layers just don't have shadows, the |!HasShadow()| there.

Comment 15

[Oleg Romashin (:romaxa)]

Attachment: Fixed comments

Comment 16

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Comment on attachment 537626 [details] [diff] [review]
Fixed comments

I think this is OK.  Please be sure to run through tryserver.

Comment 17

[Oleg Romashin (:romaxa)]

Attachment: Fixed normal canvas rendering (mDirty issue)

Ok, I found android canvas reftests failing because mDirty is always false.

Comment 18

[Oleg Romashin (:romaxa)]

Attachment: interdiff from previous r+ patch

Comment 19

[Oleg Romashin (:romaxa)]

Try build looks good with last patch
http://tbpl.mozilla.org/?tree=Try&rev=620d9b977955

Comment 20

[Oleg Romashin (:romaxa)]

http://hg.mozilla.org/mozilla-central/rev/9f18296db571