WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
35153
[Qt] WebGL support missing in QtWebKit
https://bugs.webkit.org/show_bug.cgi?id=35153
Summary
[Qt] WebGL support missing in QtWebKit
Jarkko Sakkinen
Reported
2010-02-19 06:09:14 PST
WebGL support for QtWebKit is missing.
Attachments
WebGL support for QtWebKit
(70.58 KB, patch)
2010-02-19 07:32 PST
,
Jarkko Sakkinen
no flags
Details
Formatted Diff
Diff
Qt WebGL support
(70.71 KB, patch)
2010-02-19 08:00 PST
,
Jarkko Sakkinen
no flags
Details
Formatted Diff
Diff
Qt WebGL support
(73.56 KB, patch)
2010-02-23 04:49 PST
,
Jarkko Sakkinen
no flags
Details
Formatted Diff
Diff
Qt WebGL support
(73.75 KB, patch)
2010-02-23 05:13 PST
,
Jarkko Sakkinen
no flags
Details
Formatted Diff
Diff
Qt WebGL support
(83.77 KB, patch)
2010-02-23 06:17 PST
,
Jarkko Sakkinen
no flags
Details
Formatted Diff
Diff
Qt WebGL support
(83.79 KB, patch)
2010-02-23 08:58 PST
,
Jarkko Sakkinen
no flags
Details
Formatted Diff
Diff
Qt WebGL support
(83.81 KB, patch)
2010-02-23 10:32 PST
,
Jarkko Sakkinen
kenneth
: review-
Details
Formatted Diff
Diff
Qt WebGL support
(76.51 KB, patch)
2010-02-24 02:52 PST
,
Jarkko Sakkinen
kenneth
: review-
Details
Formatted Diff
Diff
Qt WebGL support
(75.68 KB, patch)
2010-02-24 05:23 PST
,
Jarkko Sakkinen
kenneth
: review+
Details
Formatted Diff
Diff
Qt WebGL support
(76.16 KB, patch)
2010-02-24 06:03 PST
,
Jarkko Sakkinen
kenneth
: review+
Details
Formatted Diff
Diff
Qt WebGL support
(76.17 KB, patch)
2010-02-25 04:53 PST
,
Jarkko Sakkinen
no flags
Details
Formatted Diff
Diff
Show Obsolete
(10)
View All
Add attachment
proposed patch, testcase, etc.
Jarkko Sakkinen
Comment 1
2010-02-19 06:21:57 PST
http://bugreports.qt.nokia.com/browse/QTWEBKIT-18
Jarkko Sakkinen
Comment 2
2010-02-19 07:32:36 PST
Created
attachment 49078
[details]
WebGL support for QtWebKit NOTES: * Works only when accelerated composition is not enabled in compilation * Added --webgl command line switch to QGVLauncher, added toggle button to QtLauncher Why GraphicsLayer (accelerated composition layer) doesn't handle WebGL? Missing methods: * setContentsToGraphicsContext3D * setGraphicsContext3DNeedsDisplay I thinks adding support for content caching is subtask for this. WebGL support can be tested by compiling with WTF_USE_ACCELERATED_COMPOSITING=0.
WebKit Review Bot
Comment 3
2010-02-19 07:36:10 PST
Attachment 49078
[details]
did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 Last 3072 characters of output: Tab found; better to use spaces [whitespace/tab] [1] WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp:237: Tab found; better to use spaces [whitespace/tab] [1] WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp:238: Tab found; better to use spaces [whitespace/tab] [1] WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp:239: Tab found; better to use spaces [whitespace/tab] [1] WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp:240: Tab found; better to use spaces [whitespace/tab] [1] WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp:241: Tab found; better to use spaces [whitespace/tab] [1] WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp:242: Tab found; better to use spaces [whitespace/tab] [1] WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp:243: Tab found; better to use spaces [whitespace/tab] [1] WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp:244: Tab found; better to use spaces [whitespace/tab] [1] WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp:245: Tab found; better to use spaces [whitespace/tab] [1] WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp:246: Tab found; better to use spaces [whitespace/tab] [1] WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp:247: Tab found; better to use spaces [whitespace/tab] [1] WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp:248: Tab found; better to use spaces [whitespace/tab] [1] WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp:249: Tab found; better to use spaces [whitespace/tab] [1] WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp:251: Tab found; better to use spaces [whitespace/tab] [1] WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp:252: Tab found; better to use spaces [whitespace/tab] [1] WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp:253: Tab found; better to use spaces [whitespace/tab] [1] WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp:254: Tab found; better to use spaces [whitespace/tab] [1] WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp:255: Tab found; better to use spaces [whitespace/tab] [1] WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp:256: Tab found; better to use spaces [whitespace/tab] [1] WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp:257: Tab found; better to use spaces [whitespace/tab] [1] WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp:258: Tab found; better to use spaces [whitespace/tab] [1] WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp:259: Tab found; better to use spaces [whitespace/tab] [1] WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp:460: One line control clauses should not use braces. [whitespace/braces] [4] WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp:606: Extra space after ( in function call [whitespace/parens] [4] WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp:1585: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4] Ignoring "WebKit/qt/Api/qwebsettings.h": this file is exempt from the style guide. Total errors found: 82 in 8 files If any of these errors are false positives, please file a bug against check-webkit-style.
Jarkko Sakkinen
Comment 4
2010-02-19 07:50:02 PST
Created subtask for accelerated compositioning changes:
http://bugreports.qt.nokia.com/browse/QTWEBKIT-91
Jarkko Sakkinen
Comment 5
2010-02-19 08:00:25 PST
Created
attachment 49080
[details]
Qt WebGL support Coding style fixes.
Noam Rosenthal
Comment 6
2010-02-19 08:08:28 PST
I see that when converting from WebCore::Image* we use QPixmap::toImage(). If we're in the OpenGL graphicssystem, that would be an unnecessary bottleneck. Anyway we could use the standard QPixmap->texture conversions from QtOpenGL?
Jarkko Sakkinen
Comment 7
2010-02-22 03:18:28 PST
You mean texImage2D and texSubImage2D methods that take Image* parameter? I would actually go even more conservative for this initial patch on Qt WebGL support because those two functions are not usuall that frequently called in common OpenGL code. See this changeset in gitorious:
http://gitorious.org/webkit/jarkkos-webkit/commit/521a68ff041db6e72d52439ed9fd02481eafb672
What do you think? After this patch has been accepted with possible corrections I think it might be good idea to keep this implementation fairly conservative until possible issues on different targets have been fixed.
Jarkko Sakkinen
Comment 8
2010-02-22 05:39:40 PST
I'm working on Fremantle fixes (see
http://gitorious.org/~sakkinen/webkit/jarkkos-webkit
). Review of this can wait until I get that escher demo succesfully running on N900 :)
Noam Rosenthal
Comment 9
2010-02-22 09:40:31 PST
Sure - I'm fine with keeping it traditional. Maybe some comments along the line of "FIXME: This can be optimized when using the OpenGL paint-engine" etc. Same goes for beginPaint, by the way - if we're on a QGLWidget / opengl graphicssystem, we don't want to go through a QImage - but a FIXME comment at this stage is enough I think. btw I'm not a reviewer - just commenting :)
Ariya Hidayat
Comment 10
2010-02-22 19:33:14 PST
> I'm working on Fremantle fixes (see >
http://gitorious.org/~sakkinen/webkit/jarkkos-webkit
). Review of this can wait > until I get that escher demo succesfully running on N900 :)
In this case, it would be wise to clear the review flag until you want it to be reviewed again.
Jarkko Sakkinen
Comment 11
2010-02-22 20:37:39 PST
Git version now runs most of the WebGL tests on N900 but they need default precision to be appended (like precision mediump float;). Should there be default precision appended to the shader code if it isn't defined? On desktop, precision stuff must be of course parsed out before compiling the shader. This is also said in
http://www.khronos.org/webgl/wiki/Testing/Conformance
but it doesn't say anything what to do if precision isn't defined (like it isn't in most of the webgl examples).
Jarkko Sakkinen
Comment 12
2010-02-23 04:49:53 PST
Created
attachment 49277
[details]
Qt WebGL support Now works tested also on N900 and fixed issues on that platform.
WebKit Review Bot
Comment 13
2010-02-23 04:58:13 PST
Attachment 49277
[details]
did not build on qt: Build output:
http://webkit-commit-queue.appspot.com/results/300462
Jarkko Sakkinen
Comment 14
2010-02-23 05:13:53 PST
Created
attachment 49279
[details]
Qt WebGL support
Kenneth Rohde Christiansen
Comment 15
2010-02-23 05:46:29 PST
Comment on
attachment 49279
[details]
Qt WebGL support
> + > +typedef void (APIENTRY *glActiveTextureType) (GLenum); > +typedef void (APIENTRY *glAttachShaderType) (GLuint, GLuint); > +typedef void (APIENTRY *glBindAttribLocationType) (GLuint, GLuint, const char *); > +typedef void (APIENTRY *glBindBufferType) (GLenum, GLuint);
Lost of these have wrong coding style, like the * should be left aligned, thus: +typedef void (APIENTRY* glBindAttribLocationType) (GLuint, GLuint, const char*) We are pretty strict about coding style in WebKit, thus r-. Please make use of the tool check-webkit-style which can be found in WebKitTools/Scripts
Jarkko Sakkinen
Comment 16
2010-02-23 06:06:45 PST
Forgot to run prepage-ChangeLog script :( Sorry!
Jarkko Sakkinen
Comment 17
2010-02-23 06:17:04 PST
Created
attachment 49284
[details]
Qt WebGL support Forgot to add ChangeLog entry to previous patches, sorry!
Jarkko Sakkinen
Comment 18
2010-02-23 06:44:32 PST
Note: apply change below if you want to try it out. See discussion in
http://bugreports.qt.nokia.com/browse/QTWEBKIT-91
on accelerated compositing ############################################################################## diff --git a/WebKit.pri b/WebKit.pri index 907a92c..837ce67 100644 --- a/WebKit.pri +++ b/WebKit.pri @@ -49,7 +49,7 @@ building-libs { } DEPENDPATH += $$PWD/WebKit/qt/Api } -greaterThan(QT_MINOR_VERSION, 5):DEFINES += WTF_USE_ACCELERATED_COMPOSITING +#greaterThan(QT_MINOR_VERSION, 5):DEFINES += WTF_USE_ACCELERATED_COMPOSITING !mac:!unix|symbian { DEFINES += USE_SYSTEM_MALLOC ##############################################################################
Jarkko Sakkinen
Comment 19
2010-02-23 08:14:19 PST
Comment on
attachment 49284
[details]
Qt WebGL support going to fix coding style issues.
Jarkko Sakkinen
Comment 20
2010-02-23 08:58:57 PST
Created
attachment 49297
[details]
Qt WebGL support - Fixed reported style issue. - Went through code manually to find more style issues (by looking at the style guide same time) - Run check-webkit-style script
Jarkko Sakkinen
Comment 21
2010-02-23 10:32:57 PST
Created
attachment 49302
[details]
Qt WebGL support Fixed whitespace style issues in ChangeLog files.
Kenneth Rohde Christiansen
Comment 22
2010-02-23 12:10:47 PST
Comment on
attachment 49302
[details]
Qt WebGL support I had a discussion with Jarkko on IRC and he is a.o updating the patch to make use of a template for all calls similar to: #ifdef ... enableVertexAttribArray = reinterpret_cast<glEnableVertexAttribArrayType>(getProcAddress(ctx, "glEnableVertexAttribArray")); #else enableVertexAttribArray = glEnableVertexAttribArray #endif that will simplify the code somewhat, as well as fixing coding styles issues and the like.
Jarkko Sakkinen
Comment 23
2010-02-24 02:52:48 PST
Created
attachment 49372
[details]
Qt WebGL support Style fixed
Kenneth Rohde Christiansen
Comment 24
2010-02-24 04:39:00 PST
Comment on
attachment 49372
[details]
Qt WebGL support Please remove the changes to QGVLauncher as it is being removed. Also generally you should use webcore types inside webcore. I would also like you to use webcore logging instead of qWarning. If you intend to land this through the commit queue, you need to remove the 'No new tests. (OOPS!)' from the ChangeLog.
Jarkko Sakkinen
Comment 25
2010-02-24 05:23:30 PST
Created
attachment 49381
[details]
Qt WebGL support
Kenneth Rohde Christiansen
Comment 26
2010-02-24 05:42:10 PST
Comment on
attachment 49381
[details]
Qt WebGL support Looks good! some final comments on IRC that you can fix if you would like.
Jarkko Sakkinen
Comment 27
2010-02-24 06:03:37 PST
Created
attachment 49383
[details]
Qt WebGL support Final polishing of comments etc style :)
Jarkko Sakkinen
Comment 28
2010-02-25 04:53:41 PST
Created
attachment 49475
[details]
Qt WebGL support - Fixed compiler warning: forgot to remove now unused local variable ctx - Fixed compiler error on desktop compilation: GraphicsContext3D::DEPTH_COMPONENT24 -> GraphicsContext3D::DEPTH_COMPONENT NOTE: If WTF_USE_ACCELERATED_COMPOSITING is disabled the current qtwebkit master does not compile. Resolved this by following change (not included in this patch since this is not part of this): diff --git a/WebCore/css/MediaQueryEvaluator.cpp b/WebCore/css/MediaQueryEvaluator.cpp index 4963ed4..6864231 100644 --- a/WebCore/css/MediaQueryEvaluator.cpp +++ b/WebCore/css/MediaQueryEvaluator.cpp @@ -47,7 +47,7 @@ #include "PlatformScreen.h" #include <wtf/HashMap.h> -#if ENABLE(3D_RENDERING) +#if ENABLE(3D_RENDERING) && USE(ACCELERATED_COMPOSITING) #include "RenderLayerCompositor.h" #endif @@ -472,10 +472,12 @@ static bool transform_3dMediaFeatureEval(CSSValue* value, RenderStyle*, Frame* f bool returnValueIfNoParameter; int have3dRendering; -#if ENABLE(3D_RENDERING) +#if ENABLE(3D_RENDERING) bool threeDEnabled = false; +#if USE(ACCELERATED_COMPOSITING) if (RenderView* view = frame->contentRenderer()) threeDEnabled = view->compositor()->hasAcceleratedCompositing(); +#endif returnValueIfNoParameter = threeDEnabled; have3dRendering = threeDEnabled ? 1 : 0;
Jarkko Sakkinen
Comment 29
2010-02-25 05:04:22 PST
Changed test page to
http://learningwebgl.com/lessons/lesson15/index.html
instead of
http://wakaba.c3.cx/w/escher_droste.html
because khronos specification says that only GLSL 1.0 should be supported [1]. [1]
https://cvs.khronos.org/svn/repos/registry/trunk/public/webgl/doc/spec/WebGL-spec.html#4.4
WebKit Commit Bot
Comment 30
2010-02-25 07:07:03 PST
Comment on
attachment 49475
[details]
Qt WebGL support Clearing flags on attachment: 49475 Committed
r55235
: <
http://trac.webkit.org/changeset/55235
>
WebKit Commit Bot
Comment 31
2010-02-25 07:07:10 PST
All reviewed patches have been landed. Closing bug.
Kent Hansen
Comment 32
2010-03-22 09:30:58 PDT
(In reply to
comment #28
)
> Created an attachment (id=49475) [details] > Qt WebGL support
Hi, This change adds public API, but lacks documentation for QWebSettings::WebGLEnabled; it should have an entry under "\enum QWebSettings::WebAttribute" in qwebsettings.cpp with a description of what it does and what the default value is. Could you add it? Thanks!
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