WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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-
Details
Formatted Diff
Diff
WIP patch
(19.04 KB, patch)
2012-12-24 02:30 PST
,
Alejandro G. Castro
no flags
Details
Formatted Diff
Diff
Patch
(26.92 KB, patch)
2013-04-10 13:00 PDT
,
Martin Robinson
no flags
Details
Formatted Diff
Diff
Patch rebased on master
(26.90 KB, patch)
2013-04-10 13:26 PDT
,
Martin Robinson
no flags
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
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
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)
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
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 13
2012-12-24 02:45:34 PST
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
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
Created
attachment 197390
[details]
Patch
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.
Top of Page
Format For Printing
XML
Clone This Bug