WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
97652
[EFL][WK2] Implement WebGL
https://bugs.webkit.org/show_bug.cgi?id=97652
Summary
[EFL][WK2] Implement WebGL
Regina Chung
Reported
2012-09-26 02:52:45 PDT
Enable WebGL for WebKit2 on EFL, with a GraphicsSurface based implementation.
Attachments
Patch
(28.30 KB, patch)
2012-10-19 03:43 PDT
,
Regina Chung
no flags
Details
Formatted Diff
Diff
Patch
(40.44 KB, patch)
2012-10-22 03:44 PDT
,
Regina Chung
no flags
Details
Formatted Diff
Diff
Patch
(24.93 KB, patch)
2012-10-24 00:22 PDT
,
Regina Chung
no flags
Details
Formatted Diff
Diff
Patch
(24.78 KB, patch)
2012-10-24 00:51 PDT
,
Regina Chung
no flags
Details
Formatted Diff
Diff
Patch
(24.78 KB, patch)
2012-10-25 19:04 PDT
,
Regina Chung
no flags
Details
Formatted Diff
Diff
Patch
(24.78 KB, patch)
2012-10-25 19:40 PDT
,
Regina Chung
no flags
Details
Formatted Diff
Diff
Patch
(24.78 KB, patch)
2012-10-25 19:43 PDT
,
Regina Chung
no flags
Details
Formatted Diff
Diff
Patch
(23.96 KB, patch)
2012-10-26 01:46 PDT
,
Regina Chung
no flags
Details
Formatted Diff
Diff
Show Obsolete
(6)
View All
Add attachment
proposed patch, testcase, etc.
Regina Chung
Comment 1
2012-10-19 03:43:35 PDT
Created
attachment 169592
[details]
Patch
WebKit Review Bot
Comment 2
2012-10-19 03:45:35 PDT
Attachment 169592
[details]
did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'Source/WebCore/ChangeLog', u..." exit_code: 1 Source/WebCore/platform/graphics/surfaces/efl/GraphicsSurfaceEfl.cpp:164: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/surfaces/efl/GraphicsSurfaceEfl.cpp:165: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 2 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.
Noam Rosenthal
Comment 3
2012-10-19 06:57:21 PDT
Any reason why not to use the existing GraphicsSurfaceGLX?
Regina Chung
Comment 4
2012-10-21 21:49:02 PDT
(In reply to
comment #3
)
> Any reason why not to use the existing GraphicsSurfaceGLX?
I was a bit hesitant about directly editing GraphicsSurfaceGLX, and thought of extracting the common code as a next step. But I guess I should just do that now :) I'm thinking of moving the QT dependent GraphicsSurfacePrivate code in GraphicsSurfaceGLX to GraphicsSurfaceQt and only leaving the corresponding code in GraphicsSurfaceEfl... Should that be okay with you?
Noam Rosenthal
Comment 5
2012-10-21 22:53:55 PDT
(In reply to
comment #4
)
> (In reply to
comment #3
) > > Any reason why not to use the existing GraphicsSurfaceGLX? > > I was a bit hesitant about directly editing GraphicsSurfaceGLX, and thought of extracting the common code as a next step. But I guess I should just do that now :) > > I'm thinking of moving the QT dependent GraphicsSurfacePrivate code in GraphicsSurfaceGLX to GraphicsSurfaceQt and only leaving the corresponding code in GraphicsSurfaceEfl... > Should that be okay with you?
Sounds like the right thing to do :)
Simon Hausmann
Comment 6
2012-10-21 23:08:05 PDT
Comment on
attachment 169592
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=169592&action=review
r- because of incorrect licensing/authorship
> Source/WebCore/platform/graphics/efl/GraphicsContext3DPrivate.cpp:206 > +#if USE(GRAPHICS_SURFACE) > +void GraphicsContext3DPrivate::createGraphicsSurfaces(const IntSize& size) > +{ > + if (size.isEmpty()) > + m_graphicsSurface.clear(); > + else > + m_graphicsSurface = GraphicsSurface::create(size, m_surfaceFlags); > +} > + > +uint32_t GraphicsContext3DPrivate::copyToGraphicsSurface() > +{ > + if (!m_graphicsSurface) > + return 0; > + > + makeContextCurrent(); > + m_graphicsSurface->copyFromTexture(m_context->m_texture, IntRect(0, 0, m_context->m_currentWidth, m_context->m_currentHeight)); > + return m_graphicsSurface->swapBuffers(); > +} > + > +GraphicsSurfaceToken GraphicsContext3DPrivate::graphicsSurfaceToken() const > +{ > + return m_graphicsSurface->exportToken(); > +} > +#endif
This looks like code that we could put into GraphicsContext3D.cpp if it's 100% identical between Qt and EFL.
> Source/WebCore/platform/graphics/surfaces/efl/GraphicsSurfaceEfl.cpp:2 > + * Copyright (C) 2012 Samsung Electronics
This file is a 90% copy of surfaces/qt/GraphicsSurfaceGLX.cpp. If you decide to fork it like this, then you need to retain the existing license and authorship. But I suggest to merge the efforts, i.e. create surfaces/glx/GraphicsSurfaceGLX.cpp with the little #ifdef'ed glue code remaining for interaction with the toolkit GL bits.
Regina Chung
Comment 7
2012-10-22 03:44:36 PDT
Created
attachment 169871
[details]
Patch
Regina Chung
Comment 8
2012-10-22 03:47:50 PDT
Tried to reuse exiting code (GraphicsSurfaceGLX) as much as possible.. About the GC3DPrivate functions. Yes, they are 100% identical but I'm not sure how to put them in GraphicsContext3D.cpp because they're 'GC3DPrivate' member functions. And it's GC3DPrivate that's inheriting TextureMapperPlatformLayer.
WebKit Review Bot
Comment 9
2012-10-22 03:50:20 PDT
Attachment 169871
[details]
did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'Source/WebCore/ChangeLog', u..." exit_code: 1 Source/WebCore/platform/graphics/surfaces/efl/GraphicsSurfaceEfl.cpp:97: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/surfaces/efl/GraphicsSurfaceEfl.cpp:98: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 2 in 15 files If any of these errors are false positives, please file a bug against check-webkit-style.
Zeno Albisser
Comment 10
2012-10-22 05:52:24 PDT
Comment on
attachment 169871
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=169871&action=review
> Source/WebCore/platform/graphics/efl/GraphicsContext3DPrivate.cpp:182 > +
We are wondering, if it would make sense to move the common code down to GraphicsContext3D instead of GraphicsContext3DPrivate, and therefore make GraphicsContext3D derive from TextureMapperPlatformLayer directly. This way we could get rid of quite some duplicated code and even improve the design, by having less many abstraction layers.
> Source/WebCore/platform/graphics/surfaces/efl/GraphicsSurfaceEfl.cpp:29 > +
This code mostly looks like a copy from GraphicsSurfaceGLX.cpp. It would be nice if we could share more of it. For Qt we could also operate on a pure GLXContext (i might need to tweak the Qt implementation a little bit) but we should be able to achieve this. So most of this code could actually remain in GraphicsSurfaceGLX and actually replace the current code. - As long as you only use GLX api.
> Source/WebCore/platform/graphics/surfaces/efl/GraphicsSurfaceEfl.cpp:122 > + glXMakeCurrent(m_display, m_detachedSurface, m_detachedContext);
Here we should probably do: m_detachedContext = 0; It is missing in our implementation as well. :)
> Source/WebCore/platform/graphics/surfaces/qt/GraphicsSurfaceGLX.h:57 > +#if PLATFORM(QT)
We will not need most of these #ifdefs if we manage to unify the code as stated above.
> Source/WebCore/platform/graphics/surfaces/qt/GraphicsSurfaceGLX.h:222 > + pGlXBindTexImageEXT = (PFNGLXBINDTEXIMAGEEXTPROC)glXGetProcAddress((GLubyte *) "glXBindTexImageEXT");
I think C-style casts are not allowed in WebKit code base.
> Source/WebCore/platform/graphics/surfaces/qt/GraphicsSurfaceQt.cpp:31 > +
This code should be wrapped by: #if HAVE(GLX) because we Qt also uses this file on Mac and Windows. Then again - As far as i see, all of it would go away under the premise that we can adjust the code as stated above.
Regina Chung
Comment 11
2012-10-22 19:13:40 PDT
(In reply to
comment #10
)
> (From update of
attachment 169871
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=169871&action=review
> > > Source/WebCore/platform/graphics/efl/GraphicsContext3DPrivate.cpp:182 > > + > > We are wondering, if it would make sense to move the common code down to GraphicsContext3D instead of GraphicsContext3DPrivate, and therefore make GraphicsContext3D derive from TextureMapperPlatformLayer directly. > This way we could get rid of quite some duplicated code and even improve the design, by having less many abstraction layers.
Yes, I think that's a good idea. A lot of the code can be shared if GraphicsContext3D is derived from TextureMapperPlatformLayer directly.
> > > Source/WebCore/platform/graphics/surfaces/efl/GraphicsSurfaceEfl.cpp:29 > > + > > This code mostly looks like a copy from GraphicsSurfaceGLX.cpp. It would be nice if we could share more of it. > For Qt we could also operate on a pure GLXContext (i might need to tweak the Qt implementation a little bit) but we should be able to achieve this. > So most of this code could actually remain in GraphicsSurfaceGLX and actually replace the current code. - As long as you only use GLX api. >
The efl port will only use GLX api, so it would be great if we can completely share the original GraphicsSurfaceGLX implementation.
> > Source/WebCore/platform/graphics/surfaces/efl/GraphicsSurfaceEfl.cpp:122 > > + glXMakeCurrent(m_display, m_detachedSurface, m_detachedContext); > > Here we should probably do: > m_detachedContext = 0; > > It is missing in our implementation as well. :) >
This would probably go into the common code if we're able to adjust the code as you suggested? :)
> > Source/WebCore/platform/graphics/surfaces/qt/GraphicsSurfaceGLX.h:57 > > +#if PLATFORM(QT) > > We will not need most of these #ifdefs if we manage to unify the code as stated above. > > > Source/WebCore/platform/graphics/surfaces/qt/GraphicsSurfaceGLX.h:222 > > + pGlXBindTexImageEXT = (PFNGLXBINDTEXIMAGEEXTPROC)glXGetProcAddress((GLubyte *) "glXBindTexImageEXT"); > > I think C-style casts are not allowed in WebKit code base. >
I'll fix that.
> > Source/WebCore/platform/graphics/surfaces/qt/GraphicsSurfaceQt.cpp:31 > > + > > This code should be wrapped by: > #if HAVE(GLX) > > because we Qt also uses this file on Mac and Windows. > Then again - As far as i see, all of it would go away under the premise that we can adjust the code as stated above.
Regina Chung
Comment 12
2012-10-24 00:22:32 PDT
Created
attachment 170334
[details]
Patch
WebKit Review Bot
Comment 13
2012-10-24 00:25:43 PDT
Attachment 170334
[details]
did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'Source/WebCore/ChangeLog', u..." exit_code: 1 Source/WebCore/platform/graphics/surfaces/qt/GraphicsSurfaceGLX.cpp:235: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/surfaces/qt/GraphicsSurfaceGLX.cpp:236: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 2 in 13 files If any of these errors are false positives, please file a bug against check-webkit-style.
Regina Chung
Comment 14
2012-10-24 00:51:29 PDT
Created
attachment 170337
[details]
Patch
Regina Chung
Comment 15
2012-10-24 01:32:43 PDT
I moved back everything to GraphicsSurfaceGLX and added the EFL code with #ifdefs. Most of these #ifdefs should be cleaned up when Qt also works with a pure GLXContext, as Zeno suggested.
Zeno Albisser
Comment 16
2012-10-24 03:44:30 PDT
Comment on
attachment 170337
[details]
Patch LGTM, once the patch has landed i will go through the #ifdefs on the Qt side, and remove as many as possible.
Gyuyoung Kim
Comment 17
2012-10-24 04:11:17 PDT
Comment on
attachment 170337
[details]
Patch Why you don't enable WEBGL in OptionEfl.cmake ? It looks this patch enables WEBGL feature according to bug title. By the way, are there no unskipped tests by this patch ?
Gyuyoung Kim
Comment 18
2012-10-24 04:14:13 PDT
Comment on
attachment 170337
[details]
Patch Oops. I clear r+ by kenneth. My mistake. So, I set r+ again. I would like to listen answer about my question.
Zeno Albisser
Comment 19
2012-10-24 05:33:45 PDT
(In reply to
comment #17
)
> (From update of
attachment 170337
[details]
) > Why you don't enable WEBGL in OptionEfl.cmake ? It looks this patch enables WEBGL feature according to bug title. By the way, are there no unskipped tests by this patch ?
Most likely not. Even though this might be bad practice, we currently do not have infrastructure to do read-pixel based tests for webgl content for the Qt port. And the Qt port was up to know, the only one using the GraphicsSurface.
Regina Chung
Comment 20
2012-10-25 07:52:43 PDT
I changed the bug title after seeing Gyuyoung's comments.
Gyuyoung Kim
Comment 21
2012-10-25 16:52:35 PDT
(In reply to
comment #20
)
> I changed the bug title after seeing Gyuyoung's comments.
As I asked in
comment #17
, Why you don't enable ENABLE_WEBGL macro in OptionEfl.cmake ? Will you enable it in other patches ?
Regina Chung
Comment 22
2012-10-25 18:25:58 PDT
(In reply to
comment #21
)
> (In reply to
comment #20
) > > I changed the bug title after seeing Gyuyoung's comments. > As I asked in
comment #17
, Why you don't enable ENABLE_WEBGL macro in OptionEfl.cmake ? Will you enable it in other patches ?
I thought it would be better to leave it as a build option rather than enabling webgl by default. Or should I enable I by default (actually it can only be enabled with the 3d-rendering and tiled-backing-store options)?
Yael
Comment 23
2012-10-25 18:37:15 PDT
(In reply to
comment #22
)
> (In reply to
comment #21
) > > (In reply to
comment #20
) > > > I changed the bug title after seeing Gyuyoung's comments. > > As I asked in
comment #17
, Why you don't enable ENABLE_WEBGL macro in OptionEfl.cmake ? Will you enable it in other patches ? > > I thought it would be better to leave it as a build option rather than enabling webgl by default. Or should I enable I by default (actually it can only be enabled with the 3d-rendering and tiled-backing-store options)?
I think it is better to enable the flag (when tiled-backing-store is enabled). That way it is easier for us to build with the feature turned on, test it and fix bugs that we find.
Regina Chung
Comment 24
2012-10-25 19:04:55 PDT
Created
attachment 170786
[details]
Patch
Regina Chung
Comment 25
2012-10-25 19:06:00 PDT
(In reply to
comment #23
)
> (In reply to
comment #22
) > > (In reply to
comment #21
) > > > (In reply to
comment #20
) > > > > I changed the bug title after seeing Gyuyoung's comments. > > > As I asked in
comment #17
, Why you don't enable ENABLE_WEBGL macro in OptionEfl.cmake ? Will you enable it in other patches ? > > > > I thought it would be better to leave it as a build option rather than enabling webgl by default. Or should I enable I by default (actually it can only be enabled with the 3d-rendering and tiled-backing-store options)? > I think it is better to enable the flag (when tiled-backing-store is enabled). > That way it is easier for us to build with the feature turned on, test it and fix bugs that we find.
I enabled webgl when tiled-backing-store is enabled in OptionsEfl.cmake :)
Regina Chung
Comment 26
2012-10-25 19:40:37 PDT
Created
attachment 170789
[details]
Patch
Regina Chung
Comment 27
2012-10-25 19:41:48 PDT
(In reply to
comment #26
)
> Created an attachment (id=170789) [details] > Patch
Sorry, there was a merge error in my last patch.. I fixed it in this one.
Regina Chung
Comment 28
2012-10-25 19:43:50 PDT
Created
attachment 170790
[details]
Patch
Ryuan Choi
Comment 29
2012-10-25 19:51:12 PDT
Comment on
attachment 170789
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=170789&action=review
> Source/WebCore/platform/graphics/surfaces/efl/GraphicsSurfaceEfl.cpp:23 > +#if USE(GRAPHICS_SURFACE) && PLATFORM(EFL)
I think that we don't need to check PLATFORM(EFL) in Efl file.
> Source/WebCore/platform/graphics/surfaces/efl/GraphicsSurfaceEfl.cpp:29 > +PassOwnPtr<GraphicsContext> GraphicsSurface::platformBeginPaint(const IntSize& size, char* bits, int stride)
As a style issue, please remove unused variable name. XXX(const IntSize&, char*, int) is enough
> Source/WebCore/platform/graphics/surfaces/efl/GraphicsSurfaceEfl.cpp:35 > +PassRefPtr<Image> GraphicsSurface::createReadOnlyImage(const IntRect& rect)
Ditto.
> Source/cmake/OptionsEfl.cmake:192 > + SET (ENABLE_WEBGL 1)
Could you check this once more? I am not sure whether changing cmake variable is enough. DOM.idl has WEBGL condition.
Kenneth Rohde Christiansen
Comment 30
2012-10-26 01:17:32 PDT
Comment on
attachment 170790
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=170790&action=review
> Source/WebCore/ChangeLog:9 > + Implemented GraphicsSurface for EFL to enable WebGL for WebKit2. > + Depends on GLX since Evas doesn't provide the necessary functionality.
Great, I dont see any reason to depend on Evas, so far it is only resulting in problems. Using GLX or EGL directly is fine.
> Source/WebCore/platform/graphics/surfaces/efl/GraphicsSurfaceEfl.cpp:5 > +/* > + Copyright (C) 2012 Samsung Electronics > + > + This library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Library General Public
Do we need this GraphicsSurfaceEfl file? I dont understand why we need a port specific file for this, especailly given it does nothing!
> Source/WebCore/platform/graphics/surfaces/qt/GraphicsSurfaceGLX.cpp:100 > +#elif PLATFORM(EFL)
I dont think this is EFL related, it is more like GLX pure...
> Source/WebCore/platform/graphics/surfaces/qt/GraphicsSurfaceGLX.cpp:188 > +#endif
I would change this to #elif PLATFORM(QT) m_glContext = adoptPtr(new QOpenGLContext); #endif and remove that from the ctor
> Source/WebCore/platform/graphics/surfaces/qt/GraphicsSurfaceGLX.cpp:420 > pGlGenFramebuffers = reinterpret_cast<PFNGLGENFRAMEBUFFERSPROC>(glContext->getProcAddress("glGenFramebuffers")); > pGlDeleteFramebuffers = reinterpret_cast<PFNGLDELETEFRAMEBUFFERSPROC>(glContext->getProcAddress("glDeleteFramebuffers")); > pGlFramebufferTexture2D = reinterpret_cast<PFNGLFRAMEBUFFERTEXTURE2DPROC>(glContext->getProcAddress("glFramebufferTexture2D")); > - > +#elif PLATFORM(EFL) > + pGlXBindTexImageEXT = reinterpret_cast<PFNGLXBINDTEXIMAGEEXTPROC>(glXGetProcAddress(reinterpret_cast<const GLubyte*>("glXBindTexImageEXT"))); > + pGlXReleaseTexImageEXT = reinterpret_cast<PFNGLXRELEASETEXIMAGEEXTPROC>(glXGetProcAddress(reinterpret_cast<const GLubyte*>("glXReleaseTexImageEXT")));
Can't this be shared somehow? Why is it different? can a macro or template help?
Regina Chung
Comment 31
2012-10-26 01:46:58 PDT
Created
attachment 170851
[details]
Patch
Regina Chung
Comment 32
2012-10-26 01:49:47 PDT
(In reply to
comment #29
)
> (From update of
attachment 170789
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=170789&action=review
> > Source/WebCore/platform/graphics/surfaces/efl/GraphicsSurfaceEfl.cpp:23 > > +#if USE(GRAPHICS_SURFACE) && PLATFORM(EFL) > I think that we don't need to check PLATFORM(EFL) in Efl file. > > Source/WebCore/platform/graphics/surfaces/efl/GraphicsSurfaceEfl.cpp:29 > > +PassOwnPtr<GraphicsContext> GraphicsSurface::platformBeginPaint(const IntSize& size, char* bits, int stride) > As a style issue, please remove unused variable name. > XXX(const IntSize&, char*, int) is enough > > Source/WebCore/platform/graphics/surfaces/efl/GraphicsSurfaceEfl.cpp:35 > > +PassRefPtr<Image> GraphicsSurface::createReadOnlyImage(const IntRect& rect) > Ditto. > > Source/cmake/OptionsEfl.cmake:192 > > + SET (ENABLE_WEBGL 1) > Could you check this once more? > I am not sure whether changing cmake variable is enough. > DOM.idl has WEBGL condition.
I removed the GraphicsSurfaceEfl.cpp file as Kenneth suggested and moved the 'notImplemented' functions into GraphicsSurfaceGLX.cpp. Also removed the parameter names as you mentioned.
Zeno Albisser
Comment 33
2012-10-26 01:49:50 PDT
Comment on
attachment 170790
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=170790&action=review
>> Source/WebCore/platform/graphics/surfaces/qt/GraphicsSurfaceGLX.cpp:100 >> +#elif PLATFORM(EFL) > > I dont think this is EFL related, it is more like GLX pure...
This #ifdef can go away completely, once i switched Qt to use pure GLX as well. I have a patch ready for that, but it of course depends on this patch. :)
>> Source/WebCore/platform/graphics/surfaces/qt/GraphicsSurfaceGLX.cpp:188 >> +#endif > > I would change this to > > #elif PLATFORM(QT) > m_glContext = adoptPtr(new QOpenGLContext); > #endif > > and remove that from the ctor
Same here.
>> Source/WebCore/platform/graphics/surfaces/qt/GraphicsSurfaceGLX.cpp:420 >> + pGlXReleaseTexImageEXT = reinterpret_cast<PFNGLXRELEASETEXIMAGEEXTPROC>(glXGetProcAddress(reinterpret_cast<const GLubyte*>("glXReleaseTexImageEXT"))); > > Can't this be shared somehow? Why is it different? can a macro or template help?
No need for macros. Th current Qt code path will go away. Asap.
Kenneth Rohde Christiansen
Comment 34
2012-10-26 01:51:27 PDT
Comment on
attachment 170851
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=170851&action=review
> ChangeLog:6 > + Reviewed by Gyuyoung Kim.
You are still changing it, to reset review please
> Source/WebCore/platform/graphics/surfaces/qt/GraphicsSurfaceGLX.cpp:79 > + Window* get(Display* dpy)
Weird method name. Window* createOffscreenXWindow()
Kenneth Rohde Christiansen
Comment 35
2012-10-26 01:56:49 PDT
Thanks for the update Zeno! You want this to go in as it is?
Zeno Albisser
Comment 36
2012-10-26 02:05:04 PDT
(In reply to
comment #35
)
> Thanks for the update Zeno! You want this to go in as it is?
I cant judge for EFL, but from the Qt side it is fine with me.
WebKit Review Bot
Comment 37
2012-10-26 03:22:16 PDT
Comment on
attachment 170851
[details]
Patch Clearing flags on attachment: 170851 Committed
r132601
: <
http://trac.webkit.org/changeset/132601
>
WebKit Review Bot
Comment 38
2012-10-26 03:22:24 PDT
All reviewed patches have been landed. Closing bug.
WebKit Review Bot
Comment 39
2012-10-26 03:49:55 PDT
Re-opened since this is blocked by
bug 100494
Csaba Osztrogonác
Comment 40
2012-10-26 03:57:19 PDT
(In reply to
comment #37
)
> (From update of
attachment 170851
[details]
) > Clearing flags on attachment: 170851 > > Committed
r132601
: <
http://trac.webkit.org/changeset/132601
>
Rolled out, because it broke the Qt build: /home/webkitbuildbot/slaves/release64bitWebKit2_EC2/buildslave/qt-linux-64-release-webkit2/build/Source/WebCore/platform/graphics/surfaces/qt/GraphicsSurfaceGLX.cpp: In constructor ‘WebCore::GraphicsSurfacePrivate::GraphicsSurfacePrivate(PlatformGraphicsContext3D)’: /home/webkitbuildbot/slaves/release64bitWebKit2_EC2/buildslave/qt-linux-64-release-webkit2/build/Source/WebCore/platform/graphics/surfaces/qt/GraphicsSurfaceGLX.cpp:151:27: error: no matching function for call to ‘WTF::OwnPtr<QOpenGLContext>::OwnPtr(int)’ /home/webkitbuildbot/slaves/release64bitWebKit2_EC2/buildslave/qt-linux-64-release-webkit2/build/Source/WebCore/platform/graphics/surfaces/qt/GraphicsSurfaceGLX.cpp:151:27: note: candidates are: /home/webkitbuildbot/slaves/release64bitWebKit2_EC2/buildslave/qt-linux-64-release-webkit2/build/Source/WTF/wtf/OwnPtr.h:60:9: note: WTF::OwnPtr<T>::OwnPtr(const WTF::OwnPtr<typename WTF::RemovePointer<T>::Type>&) [with T = QOpenGLContext, typename WTF::RemovePointer<T>::Type = QOpenGLContext] /home/webkitbuildbot/slaves/release64bitWebKit2_EC2/buildslave/qt-linux-64-release-webkit2/build/Source/WTF/wtf/OwnPtr.h:60:9: note: no known conversion for argument 1 from ‘int’ to ‘const WTF::OwnPtr<QOpenGLContext>&’ /home/webkitbuildbot/slaves/release64bitWebKit2_EC2/buildslave/qt-linux-64-release-webkit2/build/Source/WTF/wtf/OwnPtr.h:53:30: note: template<class U> WTF::OwnPtr::OwnPtr(const WTF::PassOwnPtr<U>&) /home/webkitbuildbot/slaves/release64bitWebKit2_EC2/buildslave/qt-linux-64-release-webkit2/build/Source/WTF/wtf/OwnPtr.h:50:9: note: WTF::OwnPtr<T>::OwnPtr(std::nullptr_t) [with T = QOpenGLContext] /home/webkitbuildbot/slaves/release64bitWebKit2_EC2/buildslave/qt-linux-64-release-webkit2/build/Source/WTF/wtf/OwnPtr.h:50:9: note: no known conversion for argument 1 from ‘int’ to ‘std::nullptr_t’ /home/webkitbuildbot/slaves/release64bitWebKit2_EC2/buildslave/qt-linux-64-release-webkit2/build/Source/WTF/wtf/OwnPtr.h:49:9: note: WTF::OwnPtr<T>::OwnPtr() [with T = QOpenGLContext] /home/webkitbuildbot/slaves/release64bitWebKit2_EC2/buildslave/qt-linux-64-release-webkit2/build/Source/WTF/wtf/OwnPtr.h:49:9: note: candidate expects 0 arguments, 1 provided
Csaba Osztrogonác
Comment 41
2012-10-26 04:53:05 PDT
Unfortunately we caught to build fail lately, because libxcomposite-dev wasn't installes on our bots (nobody complained about it and build-webkit simple disabled building GraphicsSurfaceGLX.cpp) I installed this package, we will catch similar failures by the EWS bots.
Zeno Albisser
Comment 42
2012-10-26 04:57:04 PDT
(In reply to
comment #40
)
> Rolled out, because it broke the Qt build: > > /home/webkitbuildbot/slaves/release64bitWebKit2_EC2/buildslave/qt-linux-64-release-webkit2/build/Source/WebCore/platform/graphics/surfaces/qt/GraphicsSurfaceGLX.cpp: In constructor
... This error would be fixed as well by:
https://bugs.webkit.org/show_bug.cgi?id=100492
So how do we proceed about that now? Sorry for not being online right now, i'm facing some trouble with my quassel core right now.
Simon Hausmann
Comment 43
2012-10-26 05:20:55 PDT
Committed
r132627
: <
http://trac.webkit.org/changeset/132627
>
Csaba Osztrogonác
Comment 44
2012-10-26 05:33:12 PDT
(In reply to
comment #43
)
> Committed
r132627
: <
http://trac.webkit.org/changeset/132627
>
The build is still failing: /ramdisk/qt-linux-32-release-webkit2/build/Source/WebCore/platform/graphics/surfaces/qt/GraphicsSurfaceGLX.cpp: In constructor ‘WebCore::GraphicsSurfacePrivate::GraphicsSurfacePrivate(QOpenGLContext*)’: /ramdisk/qt-linux-32-release-webkit2/build/Source/WebCore/platform/graphics/surfaces/qt/GraphicsSurfaceGLX.cpp:139: error: ‘glxGetCurrentContext’ was not declared in this scope
Simon Hausmann
Comment 45
2012-10-26 05:44:51 PDT
(In reply to
comment #44
)
> (In reply to
comment #43
) > > Committed
r132627
: <
http://trac.webkit.org/changeset/132627
> > > The build is still failing: > /ramdisk/qt-linux-32-release-webkit2/build/Source/WebCore/platform/graphics/surfaces/qt/GraphicsSurfaceGLX.cpp: In constructor ‘WebCore::GraphicsSurfacePrivate::GraphicsSurfacePrivate(QOpenGLContext*)’: > /ramdisk/qt-linux-32-release-webkit2/build/Source/WebCore/platform/graphics/surfaces/qt/GraphicsSurfaceGLX.cpp:139: error: ‘glxGetCurrentContext’ was not declared in this scope
Fixed in
http://trac.webkit.org/changeset/132634
:)
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