Bug 104672

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.

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.

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

(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.

(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.

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).

(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.

(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.

(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.

Created attachment 180659 [details] WIP patch Some minor fix in the sizes and style cleaning.

(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.

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

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

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

Created attachment 197390 [details] Patch

Created attachment 197396 [details] Patch rebased on master

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.

Comment on attachment 197396 [details] Patch rebased on master Clearing flags on attachment: 197396 Committed r148247: <http://trac.webkit.org/changeset/148247>

All reviewed patches have been landed. Closing bug.

Re-opened since this is blocked by bug 114490

(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.

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

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.

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

(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. :)