RESOLVED FIXED 104672
[GTK] Add accelerated 2D canvas support using cairo-gl
https://bugs.webkit.org/show_bug.cgi?id=104672
Summary [GTK] Add accelerated 2D canvas support using cairo-gl
Alejandro G. Castro
Reported 2012-12-11 06:55:10 PST
With this bug we will track patches to implement accelerated 2D canvas support using the cairo-gl backend.
Attachments
WIP patch (19.43 KB, patch)
2012-12-11 07:51 PST, Alejandro G. Castro
eflews.bot: commit-queue-
WIP patch (19.04 KB, patch)
2012-12-24 02:30 PST, Alejandro G. Castro
no flags
Patch (26.92 KB, patch)
2013-04-10 13:00 PDT, Martin Robinson
no flags
Patch rebased on master (26.90 KB, patch)
2013-04-10 13:26 PDT, Martin Robinson
no flags
Alejandro G. Castro
Comment 1 2012-12-11 07:51:20 PST
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
Comment 2 2012-12-11 10:53:35 PST
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 3 2012-12-11 11:56:44 PST
Dominik Röttsches (drott)
Comment 4 2012-12-13 01:27:44 PST
(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
Comment 5 2012-12-13 01:32:48 PST
(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.
Comment 6 2012-12-21 18:07:38 PST
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
Comment 7 2012-12-23 09:32:45 PST
(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.
Comment 8 2012-12-23 11:10:38 PST
(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
Comment 9 2012-12-23 12:08:12 PST
(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
Comment 10 2012-12-24 02:30:42 PST
Created attachment 180659 [details] WIP patch Some minor fix in the sizes and style cleaning.
Alejandro G. Castro
Comment 11 2012-12-24 02:32:36 PST
(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 12 2012-12-24 02:45:04 PST
kov's GTK+ EWS bot
Comment 13 2012-12-24 02:45:34 PST
Kyungjin Kim
Comment 14 2013-02-07 17:09:03 PST
*** Bug 108743 has been marked as a duplicate of this bug. ***
Martin Robinson
Comment 15 2013-04-10 13:00:00 PDT
Martin Robinson
Comment 16 2013-04-10 13:26:02 PDT
Created attachment 197396 [details] Patch rebased on master
Alejandro G. Castro
Comment 17 2013-04-11 16:22:32 PDT
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 18 2013-04-11 16:38:17 PDT
Comment on attachment 197396 [details] Patch rebased on master Clearing flags on attachment: 197396 Committed r148247: <http://trac.webkit.org/changeset/148247>
Martin Robinson
Comment 19 2013-04-11 16:38:22 PDT
All reviewed patches have been landed. Closing bug.
WebKit Commit Bot
Comment 20 2013-04-11 22:59:44 PDT
Re-opened since this is blocked by bug 114490
Zan Dobersek
Comment 21 2013-04-11 23:06:29 PDT
(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
Comment 22 2013-04-12 00:38:11 PDT
... and rolled back in in r148264. Cairo now compiles. Closing as fixed. http://trac.webkit.org/changeset/148264
Zan Dobersek
Comment 23 2013-04-12 00:39:46 PDT
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)
Comment 24 2013-04-12 07:51:31 PDT
Out of curiosity, is there any improvements performance-wise, when using cairo-gl? Maybe some traces to veify with cairo-perf-trace? :)
Martin Robinson
Comment 25 2013-04-12 08:20:22 PDT
(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. :)
Note You need to log in before you can comment on or make changes to this bug.