Bug 31517

Summary: [GTK] WebGL support
Product: WebKit Reporter: Zan Dobersek <zan>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, bfulgham, bjt23, bunk, cedricv, christophe.public, eric, joone.hur, joone, kevin.cs.oh, krit, mrobinson, naiem.shaik, otte, sa, scottt.tw, skyul, thierry.reding, webkit.review.bot, wjjeon, xanthor+bz
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Bug Depends on: 51716, 51722    
Bug Blocks: 57421    
Attachments:
Description Flags
Work-In-Progress of WebGL implementation
none
Cairo patch
none
webkit-webgk-wip-rebased.patch
none
Initial patch with WebGL support for the GTK+ port
none
Patch with Benjamin's suggestions
none
Patch updated to ToT
none
Patch with missing source files
none
revised patch
none
Patch adding missing shim table entry none

Description Zan Dobersek 2009-11-15 08:15:40 PST
This bug oversees the implementation of WebGL for the Gtk port.
Comment 1 Zan Dobersek 2009-11-15 08:58:54 PST
Created attachment 43247 [details]
Work-In-Progress of WebGL implementation

Warning - this patch is pretty outdated (2 or more weeks), so it probably won't apply easily or build properly.
It also contains some debugging leftovers and unnecessary stuff (methods, lines etc.)

This implementation requires use of cairo's OpenGL backend. A recent version or even trunk should be used, along with another patch applied. That patch will be attached to this bug.

There are some notable problems with this implementation, but they can be fixed.
The surface is not properly painted on the canvas. The surface is correctly shown on the canvas only if the canvashas size of 300x150px, which is the default canvas size. If the canvas is resized to a bigger size, the surface resizes also, but only the rectangle (x= 0, y = 0, width = 300, height = 150) of the surface is shown. When resized to a smaller size, the surface is shown completely, but unevenly, relatively to the canvas's size.

The other problem is that the OpenGL content is rendered to the cairo's surface upside down - OpenGL is using lower-left coordinate system while images use upper-left coordinate system. This is described as twelveth pitfall at [1] and should be simple to fix.

Other than that, the performance is, I feel, good. I've tested the webgl manual tests and they all worked pretty well apart from the mentioned problems. I experienced crashes on olliej's WebGL example[2], though.
 
[1] http://www.opengl.org/resources/features/KilgardTechniques/oglpitfall/
[2] http://nerget.com/webgl
Comment 2 Zan Dobersek 2009-11-15 09:01:09 PST
Created attachment 43248 [details]
Cairo patch

This patch adds functionality to the cairo's OpenGL backend that is needed for the WebGL impl. to work.
Comment 3 Priit Laes (IRC: plaes) 2010-01-14 08:11:03 PST
Created attachment 46570 [details]
webkit-webgk-wip-rebased.patch

I took Zan's patch and rebased it (lots of changes have been happened in tree since the patch).

Please note that I only "#if 0"-ed out the changed implementations of changed API in GraphicsContext3D class and stubbed the non-existing functions.
Comment 4 Cedric Vivier 2010-03-25 20:21:12 PDT
I think the GTK port should not link to libGL but use a dynamic dlopen/dlsym-based approach like what the QT port is doing.

It probable make sense to use QT approach in base GraphicsContext3D since it can be shared among ports (Gtk/Qt/Chromium/Win/...) that cannot link to libGL (only Mac port can link libGL for sure afaik) ; only port-specific initialization stuff should be required in the ports, makes WebGL easier to test and less probability of behavior differences between ports (wrt to arguments checking etc).
Comment 5 Martin Robinson 2011-02-07 12:49:46 PST
Created attachment 81506 [details]
Initial patch with WebGL support for the GTK+ port
Comment 6 WebKit Review Bot 2011-02-07 13:51:30 PST
Attachment 81506 [details] did not build on mac:
Build output: http://queues.webkit.org/results/7707522
Comment 7 Martin Robinson 2011-02-08 11:41:16 PST
Created attachment 81670 [details]
Patch with Benjamin's suggestions
Comment 8 Martin Robinson 2011-03-14 13:11:21 PDT
Created attachment 85706 [details]
Patch updated to ToT
Comment 9 Philippe Normand 2011-03-15 05:05:05 PDT
I get a bunch of errors at link time, like this one:

./.libs/libwebkitgtk-1.0.so: undefined reference to `sh::SearchSymbol::foundMatch() const'
./.libs/libwebkitgtk-1.0.so: undefined reference to `sh::SearchSymbol::SearchSymbol(std::basic_string<char, std::char_traits<char>, pool_allocator<char> > const&)'
./.libs/libwebkitgtk-1.0.so: undefined reference to `WebCore::WebGLExtension::WebGLExtension()'
./.libs/libwebkitgtk-1.0.so: undefined reference to `CollectAttribsUniforms::CollectAttribsUniforms(std::vector<TVariableInfo, std::allocator<TVariableInfo> >&, std::vector<TVariableInfo, std::allocator<TVariableInfo> >&)'
./.libs/libwebkitgtk-1.0.so: undefined reference to `WebCore::WebGLExtension::~WebGLExtension()'
collect2: ld returned 1 exit status
Comment 10 Philippe Normand 2011-03-15 05:19:05 PDT
Adding

Source/WebCore/html/canvas/WebGLExtension.{h,cpp}
Source/ThirdParty/ANGLE/src/compiler/VariableInfo.{h,cpp}
Source/ThirdParty/ANGLE/src/compiler/SearchSymbol.{h,cpp}

in the build fixes those link errors :)
Comment 11 Martin Robinson 2011-03-15 08:37:23 PDT
Created attachment 85810 [details]
Patch with missing source files
Comment 12 Thierry Reding 2011-03-23 08:27:38 PDT
FWIW, I've been able to run all lessons from the following page successfully
with the latest patch (ID 85810) applied.

http://learningwebgl.com/blog/?page_id=1217

Running the WebGL Conformance Tests from the Khronos website below, however,
has the browser hanging after some time, consuming 100% CPU.

https://cvs.khronos.org/svn/repos/registry/trunk/public/webgl/sdk/tests/webgl-conformance-tests.html

Excellent work!
Comment 13 ChangSeok Oh 2011-03-25 03:50:11 PDT
Created attachment 86915 [details]
revised patch

I just revised Martin's patch to meet latest trunk(r81938).
Comment 14 ChangSeok Oh 2011-03-25 03:56:01 PDT
(In reply to comment #12)
> FWIW, I've been able to run all lessons from the following page successfully
> with the latest patch (ID 85810) applied.
> 
> http://learningwebgl.com/blog/?page_id=1217
> 
> Running the WebGL Conformance Tests from the Khronos website below, however,
> has the browser hanging after some time, consuming 100% CPU.
> 
> https://cvs.khronos.org/svn/repos/registry/trunk/public/webgl/sdk/tests/webgl-conformance-tests.html
> 
> Excellent work!

I also faced an issue while running the test that Thierry mentioned on Ubuntu 10.10. In my case. It was crash.
No problem to run learningwebgl tutorial, too.
Comment 15 ChangSeok Oh 2011-03-29 00:55:07 PDT
Martin, I think following line is missed in initializeOpenGLShims().
> ASSIGN_FUNCTION_TABLE_ENTRY(glUniform2fv, success);

It cause a crash while running the WebGL Conformance Tests Thierry mentioned. :)
Comment 16 Martin Robinson 2011-03-29 12:31:45 PDT
Created attachment 87394 [details]
Patch adding missing shim table entry
Comment 17 Martin Robinson 2011-03-29 12:32:23 PDT
(In reply to comment #15)
> Martin, I think following line is missed in initializeOpenGLShims().
> > ASSIGN_FUNCTION_TABLE_ENTRY(glUniform2fv, success);
> 
> It cause a crash while running the WebGL Conformance Tests Thierry mentioned. :)

Thank you! I've uploaded a patch fixing this issue.
Comment 18 Gustavo Noronha (kov) 2011-04-04 07:23:29 PDT
Comment on attachment 87394 [details]
Patch adding missing shim table entry

View in context: https://bugs.webkit.org/attachment.cgi?id=87394&action=review

It looks good to me. Like I've noted before I am no expert in OpenGL, but given this has been looked over by people who are, and it's essentially glue code I'll say go ahead after fixing/clarifying my comments =)

> Source/WebCore/platform/graphics/gtk/GraphicsContext3DInternal.cpp:60
> +        addedAtExitHandler = false;

Looks like you want true here? I guess this raises the pressure on us to find a cross-platform way of cleaning up on exit heh. Didn't we have something added for the icon database at some point?

> Source/WebCore/platform/graphics/gtk/GraphicsContext3DInternal.cpp:102
> +    static bool initialized = false;
> +    static bool success = true;
> +    if (!initialized) {
> +        success = initializeOpenGLShims();
> +        initialized = true;
> +    }
> +    if (!success)
> +        return 0;

Just to make sure I understand this - if we fail to initialize the shims we're essentially out of luck and won't be able to support webgl, and there's no point in trying to initialize them again, so we cache the failure, that sounds good yeah.

> Source/WebKit/gtk/webkit/webkitwebsettings.cpp:924
> +    * Since: 1.3.7

Needs to be 1.3.14, or would it be 1.3.15? =)
Comment 19 Martin Robinson 2011-04-04 14:38:16 PDT
(In reply to comment #18)

Thank you very kindly for the review.

> > Source/WebCore/platform/graphics/gtk/GraphicsContext3DInternal.cpp:60
> > +        addedAtExitHandler = false;
> 
> Looks like you want true here? I guess this raises the pressure on us to find a cross-platform way of cleaning up on exit heh. Didn't we have something added for the icon database at some point?

You're right! I will fix this before landing.
> 
> > Source/WebCore/platform/graphics/gtk/GraphicsContext3DInternal.cpp:102
> > +    static bool initialized = false;
> > +    static bool success = true;
> > +    if (!initialized) {
> > +        success = initializeOpenGLShims();
> > +        initialized = true;
> > +    }
> > +    if (!success)
> > +        return 0;
> 
> Just to make sure I understand this - if we fail to initialize the shims we're essentially out of luck and won't be able to support webgl, and there's no point in trying to initialize them again, so we cache the failure, that sounds good yeah.

Exactly right.

> > Source/WebKit/gtk/webkit/webkitwebsettings.cpp:924
> > +    * Since: 1.3.7
> 
> Needs to be 1.3.14, or would it be 1.3.15? =)

Will fix this!
Comment 20 Martin Robinson 2011-04-04 15:13:20 PDT
Committed r82878: <http://trac.webkit.org/changeset/82878>
Comment 21 WebKit Review Bot 2011-04-04 15:29:59 PDT
http://trac.webkit.org/changeset/82878 might have broken GTK Linux 32-bit Release
Comment 22 Won Jeon 2011-05-12 14:00:02 PDT
Any plan to revive this CR and fix it soon? Thanks.
Comment 23 Martin Robinson 2011-05-13 11:32:04 PDT
(In reply to comment #22)
> Any plan to revive this CR and fix it soon? Thanks.

I'm not sure I understand. The patch landed and the bug was resolved.
Comment 24 Won Jeon 2011-05-13 11:40:47 PDT
I updated the source code yesterday and tested it on my 32-bit Linux machine (w/ NVidia GeForce 8600 GTS), with the build switches 'build-webkit --gtk --3d-canvas --enable-webgl' but it didn't work. When I executed the browser by 'run-launcher --gtk' and test it with HTML5 test (http://www.html5test.com), it fails in WebGL test. Am I missing anything?
Comment 25 Martin Robinson 2011-05-13 12:12:40 PDT
(In reply to comment #24)
> I updated the source code yesterday and tested it on my 32-bit Linux machine (w/ NVidia GeForce 8600 GTS), with the build switches 'build-webkit --gtk --3d-canvas --enable-webgl' but it didn't work. When I executed the browser by 'run-launcher --gtk' and test it with HTML5 test (http://www.html5test.com), it fails in WebGL test. Am I missing anything?

You need to enable the WebGL setting in WebKitWebViewSettings.
Comment 26 Won Jeon 2011-05-13 16:09:14 PDT
Where and how can it be enabled?
Comment 27 Martin Robinson 2011-05-13 16:39:52 PDT
(In reply to comment #26)
> Where and how can it be enabled?

g_object_set(webkit_web_view_get_settings(web_view), "enable-webgl", TRUE, NULL);

Hopefully we'll have the online documentation updated soon.
Comment 28 Won Jeon 2011-05-13 16:55:05 PDT
I inserted it after createWindow(&webView) in the main() of GtkLauncher.c but it's still not working. Whenever GtkLauncher tries to load WebGL pages, it has "SSL handshake failed" error.
Comment 29 Martin Robinson 2011-05-13 17:06:10 PDT
(In reply to comment #28)
> I inserted it after createWindow(&webView) in the main() of GtkLauncher.c but it's still not working. Whenever GtkLauncher tries to load WebGL pages, it has "SSL handshake failed" error.

This is an unrelated issue with libsoup. Let's move this discussion off of Bugzilla, perhaps to the WebKitGTK+ mailing list http://webkitgtk.org/?page=contact. That would be a much more better place for this kind of discussions.