Bug 104672 – [GTK] Add accelerated 2D canvas support using cairo-gl

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Alejandro G. Castro]

With this bug we will track patches to implement accelerated 2D canvas support using the cairo-gl backend.

[Alejandro G. Castro]

Created attachment 178805 [details]
WIP patch

This patch is a work in progress, I've updated and fixed some problems with the patch mrobinson implemented some months ago. Tests and comments are welcome, we will be working on it. If you want to test it you need to apply the patch in the bug blocking this one, compile cairo with gl support, enable accelerated2dCanvasEnabled in Settings.in and build with option --enable-accelerated-2d-canvas.

[WebKit Review Bot]

Attachment 178805 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/GNUmakefile.am', u'Source/W..." exit_code: 1
Source/WebCore/platform/graphics/glx/GLContextGLX.cpp:26:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebCore/platform/graphics/cairo/ImageBufferCairo.cpp:44:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebCore/platform/graphics/cairo/ImageBufferCairo.cpp:107:  Use 0 instead of NULL.  [readability/null] [5]
Source/WebCore/platform/graphics/cairo/ImageBufferCairo.cpp:110:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/graphics/cairo/ImageBufferCairo.cpp:111:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/graphics/cairo/ImageBufferCairo.cpp:209:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/graphics/cairo/ImageBufferCairo.cpp:210:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/graphics/egl/GLContextEGL.cpp:25:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 8 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EFL EWS Bot]

Comment on attachment 178805 [details]
WIP patch

Attachment 178805 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/15280182

[Dominik Röttsches (drott)]

(In reply to comment #0)
> With this bug we will track patches to implement accelerated 2D canvas support using the cairo-gl backend.

Do you have any comparisons before and after? That'd be interesting to hear.

[Martin Robinson]

(In reply to comment #4)
> (In reply to comment #0)
> > With this bug we will track patches to implement accelerated 2D canvas support using the cairo-gl backend.
> 
> Do you have any comparisons before and after? That'd be interesting to hear.

It's very much dependent on your hardware. For instance, current mesa-based drivers do not support multisampling, so they will go down the slow cairo path when anti-aliasing is turned on.

[arno.]

Hi,
I tried this patch which looks really interesting.
But there seems to be a problem with putByteArray: the first row is correct. Then all other rows are the same as first row

see this example:
http://renevier.net/misc/webkit_104672.html
and the result:
http://renevier.net/misc/webkit_104672_bug.jpg

Anyway, why not call glReadPixels and glTexSubImage2D directly? That would save many data copies between the gl-surface and the mapped image. And even a glReadPixels which is performed at the start of putByteArray while in theory we don't need to read previous pixel data when we just upload new data).

[Martin Robinson]

(In reply to comment #6)

> Anyway, why not call glReadPixels and glTexSubImage2D directly? That would save many data copies between the gl-surface and the mapped image. And even a glReadPixels which is performed at the start of putByteArray while in theory we don't need to read previous pixel data when we just upload new data).

Cairo's map-to-image and unmap-image effectively just do a glReadPixels and glTexSubImage2D. In the future the may be optimized further with PBOs as well. There shouldn't be any extra data copies, as far as I know.

[arno.]

(In reply to comment #7)


> Cairo's map-to-image and unmap-image effectively just do a glReadPixels and glTexSubImage2D. In the future the may be optimized further with PBOs as well.

In the patch, there is a map at the start of getImageData and an unmap at its end. Same thing for putByteArray.

But getImageData only needs to read pixels from graphic card, and putByteArray only needs to puts pixels to graphic card. So, there may be an extra glTexSubImage2D in getImageData and an extra glReadPixels in putByteArray. 


> There shouldn't be any extra data copies, as far as I know.

ok, sorry about that.

[Martin Robinson]

(In reply to comment #8)

> In the patch, there is a map at the start of getImageData and an unmap at its end. Same thing for putByteArray.
> 
> But getImageData only needs to read pixels from graphic card, and putByteArray only needs to puts pixels to graphic card. So, there may be an extra glTexSubImage2D in getImageData and an extra glReadPixels in putByteArray. 

Ah, I see what you mean now. Yeah, there's definitely a chance that we can avoid an extra read here. Doing pixel upload is already pretty slow, so we likely won't be getting great performance here anyway, but your suggestion makes a lot of sense.

[Alejandro G. Castro]

Created attachment 180659 [details]
WIP patch

Some minor fix in the sizes and style cleaning.

[Alejandro G. Castro]

(In reply to comment #4)
> (In reply to comment #0)
> > With this bug we will track patches to implement accelerated 2D canvas support using the cairo-gl backend.
> 
> Do you have any comparisons before and after? That'd be interesting to hear.

The idea about uploading the work in progress patch is trying to get those figures and proposals from everyone interested in these solution. We still need to make sure in which situations and how to integrate it the best way.

[EFL EWS Bot]

Comment on attachment 180659 [details]
WIP patch

Attachment 180659 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/15491480

[kov's GTK+ EWS bot]

Comment on attachment 180659 [details]
WIP patch

Attachment 180659 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/15491481

[Kyungjin Kim]

*** Bug 108743 has been marked as a duplicate of this bug. ***

[Martin Robinson]

Created attachment 197390 [details]
Patch

[Martin Robinson]

Created attachment 197396 [details]
Patch rebased on master

[Alejandro G. Castro]

Comment on attachment 197396 [details]
Patch rebased on master

LGTM. Great, after this time testing let's add it to the repository and work from that.

[Martin Robinson]

Comment on attachment 197396 [details]
Patch rebased on master

Clearing flags on attachment: 197396

Committed r148247: <http://trac.webkit.org/changeset/148247>

[Martin Robinson]

All reviewed patches have been landed.  Closing bug.

[WebKit Commit Bot]

Re-opened since this is blocked by bug 114490

[Zan Dobersek]

(In reply to comment #20)
> Re-opened since this is blocked by bug 114490

Rolled it out as Cairo does not build on the builders due to missing EGL headers. I don't have the permissions to install those dependencies myself so I'll get someone to do that for me. After that I'll get the patch landed again.

[Zan Dobersek]

... and rolled back in in r148264. Cairo now compiles. Closing as fixed.
http://trac.webkit.org/changeset/148264

[Zan Dobersek]

Oh, and sorry for the noise, the rollout was done with too much haste as it didn't take long to get someone to install those headers.

[Sergio Correia (qrwteyrutiyoup)]

Out of curiosity, is there any improvements performance-wise, when using cairo-gl? Maybe some traces to veify with cairo-perf-trace? :)

[Martin Robinson]

(In reply to comment #24)
> Out of curiosity, is there any improvements performance-wise, when using cairo-gl? Maybe some traces to veify with cairo-perf-trace? :)

It depends quite a bit on the system, but sometimes this does yield a performance boost. Part of making sure that it's consistently faster though, is being able to easily test CairoGL. :)