QFATAL : tst_QQuickWebView::show() Received signal 11 FAIL! : tst_QQuickWebView::show() Received a fatal error. This crash hides 2 other test: PASS : tst_QQuickWebView::showWebView() PASS : tst_QQuickWebView::cleanupTestCase()
+info: it crashes in xvfb, but passes with X forward.
(In reply to comment #1) > +info: it crashes in xvfb, but passes with X forward. I meant xvfb-run -a --server-args="-screen 0 1024x768x24" Tools/Scripts/run-qtwebkit-tests --output-file=qt-unit-tests.html --do-not-open-results --timeout=120 WebKitBuild/Release/Source/WebKit2/UIProcess/API/qt/tests/
Created attachment 119092 [details] "Normal" X server glxinfo run log.
Created attachment 119093 [details] Xvfb server glxinfo run log.
(In reply to comment #2) > (In reply to comment #1) > > +info: it crashes in xvfb, but passes with X forward. > > I meant xvfb-run -a --server-args="-screen 0 1024x768x24" Tools/Scripts/run-qtwebkit-tests --output-file=qt-unit-tests.html --do-not-open-results --timeout=120 WebKitBuild/Release/Source/WebKit2/UIProcess/API/qt/tests/ It seems Xvfb just doesn't support shaders. Check attached logs. Could you run xvfb-run -a --server-args="-screen 0 1024x768x24" glxinfo on build bot and attach log here? Thanks.
Created attachment 119184 [details] xvfb glxinfo run log from the bot
(In reply to comment #6) > Created an attachment (id=119184) [details] > xvfb glxinfo run log from the bot 1. xvfb GL support on ubuntu depends on mesa, which is missing mesa GLX if HW nvidia or ATI drivers installed. The only way to run this correctly is to remove HW drivers and use mesa GLX implementation. 2. Deletion of test window in API test also cause destruction of GL context making all global GL objects in TextureMapperGL invalid. Also, current implementation will not work with multiple windows (because they will have multiple canvases and multiple GL contexts). Here is QOpenGLContest destruction call stack: 0 QOpenGLContext::~QOpenGLContext qopenglcontext.cpp 193 0xb660e919 1 QOpenGLContext::~QOpenGLContext qopenglcontext.cpp 194 0xb660e983 2 QQuickTrivialWindowManager::hide qquickwindowmanager.cpp 1123 0xb7447aab 3 QQuickTrivialWindowManager::canvasDestroyed qquickwindowmanager.cpp 1130 0xb7447ae5 4 QQuickCanvas::~QQuickCanvas qquickcanvas.cpp 635 0xb7362a00 5 QQuickView::~QQuickView qquickview.cpp 178 0xb742119f 6 TestWindow::~TestWindow testwindow.h 29 0x8051f27 7 TestWindow::~TestWindow testwindow.h 29 0x8051f77 8 QScopedPointerDeleter<TestWindow>::cleanup qscopedpointer.h 62 0x8051fa2 9 QScopedPointer<TestWindow, QScopedPointerDeleter<TestWindow> >::reset qscopedpointer.h 149 0x80517c9 10 tst_QQuickWebView::cleanup tst_qquickwebview.cpp 73 0x804ea90 11 tst_QQuickWebView::qt_static_metacall tst_qquickwebview.moc 74 0x8050a24 12 QMetaMethod::invoke qmetaobject.cpp 1634 0xb630b991 13 QMetaMethod::invoke qmetaobject.h 120 0xb6a4167f 14 QTest::invokeMethod qtestcase.cpp 998 0xb6a3b927 15 QTest::qInvokeTestMethodDataEntry qtestcase.cpp 1505 0xb6a3dafa 16 QTest::qInvokeTestMethod qtestcase.cpp 1605 0xb6a3e217 17 QTest::qInvokeTestMethods qtestcase.cpp 1759 0xb6a3e9b2 18 QTest::qExec qtestcase.cpp 1972 0xb6a3ee77 19 main tst_qquickwebview.cpp 287 0x8050957
Created attachment 121596 [details] Patch Ossy, please, could you take a look at this?
Comment on attachment 121596 [details] Patch Attachment 121596 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/11113272
Created attachment 121605 [details] Use QGLContext compatible with previous Qt version
Comment on attachment 121605 [details] Use QGLContext compatible with previous Qt version View in context: https://bugs.webkit.org/attachment.cgi?id=121605&action=review > Source/WebCore/platform/graphics/opengl/TextureMapperGL.cpp:43 > +#include <GL/glx.h> This needs to be inside OS(UNIX) or OS(LINUX) or something like that. > Source/WebCore/platform/graphics/opengl/TextureMapperGL.cpp:217 > + GLContextDataMap::const_iterator mapEnd = glContextDataMap().end(); Double space > Source/WebCore/platform/graphics/opengl/TextureMapperGL.cpp:219 > + for (i = glContextDataMap().begin(); i != mapEnd; ++i) Curly braces
Can't we make it simpler, by making this GL data non-static, and not try to share it between TextureMapperGL instances in the same GL context? It seems a bit superfluous
Created attachment 121755 [details] Added multiWindow API test and other patch fixes.
Comment on attachment 121755 [details] Added multiWindow API test and other patch fixes. View in context: https://bugs.webkit.org/attachment.cgi?id=121755&action=review On the right tracks, but meeds some improvements... > Source/WebCore/platform/graphics/opengl/TextureMapperGL.cpp:127 > +#else if OS(LINUX/UNIX) ??? > Source/WebCore/platform/graphics/opengl/TextureMapperGL.cpp:201 > + GLuint shaders[4]; Why 4? > Source/WebCore/platform/graphics/opengl/TextureMapperGL.cpp:230 > + GLContextDataMap::iterator i; it instead of i > Source/WebKit2/UIProcess/API/qt/tests/qquickwebview/tst_qquickwebview.cpp:276 > + showWebView(); The right test for this is not multiple top-level windows, but rather multiple web-views in the same scene.
(In reply to comment #14) > (From update of attachment 121755 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=121755&action=review > > On the right tracks, but meeds some improvements... > > > Source/WebCore/platform/graphics/opengl/TextureMapperGL.cpp:127 > > +#else > > if OS(LINUX/UNIX) ??? Without any default it will fail build in any case. I can make some default GLContext/getCurrentGLContext returning incrementing integer this way disabling any sharing. Would it work? > > Source/WebCore/platform/graphics/opengl/TextureMapperGL.cpp:201 > > + GLuint shaders[4]; > > Why 4? Should be 2. I had some problems and was guessing around that there are no other shaders or something and forgot to change back. Or may be it would be better just to save shader id's? > > Source/WebCore/platform/graphics/opengl/TextureMapperGL.cpp:230 > > + GLContextDataMap::iterator i; > > it instead of i Ok > > Source/WebKit2/UIProcess/API/qt/tests/qquickwebview/tst_qquickwebview.cpp:276 > > + showWebView(); > > The right test for this is not multiple top-level windows, but rather multiple web-views in the same scene. Actually, in this case it will be single GL context. But we are missing multi-view test also. What you prefer - add multiple views to this test or created separate test with multiple views on one window?
> > if OS(LINUX/UNIX) ??? > > Without any default it will fail build in any case. I can make some default GLContext/getCurrentGLContext returning incrementing integer this way disabling any sharing. Would it work? Yes, I don't see why not. > I had some problems and was guessing around that there are no other shaders or something and forgot to change back. Or may be it would be better just to save shader id's? Yes, probably. That code seems unnecessarily complex. > > > Source/WebKit2/UIProcess/API/qt/tests/qquickwebview/tst_qquickwebview.cpp:276 > > > + showWebView(); > > > > The right test for this is not multiple top-level windows, but rather multiple web-views in the same scene. > > Actually, in this case it will be single GL context. > But we are missing multi-view test also. > What you prefer - add multiple views to this test or created separate test with multiple views on one window? Have two tests together with this patch, multipleWebViewsInSingleWindow and multipleWebViewWindows.
(In reply to comment #16) > > > if OS(LINUX/UNIX) ??? > > btw UNIX/LINUX doesn't necessarily mean GLX... we have to account for EGL as well.
Comment on attachment 121755 [details] Added multiWindow API test and other patch fixes. View in context: https://bugs.webkit.org/attachment.cgi?id=121755&action=review >>> Source/WebCore/platform/graphics/opengl/TextureMapperGL.cpp:127 >>> +#else >> >> if OS(LINUX/UNIX) ??? > > Without any default it will fail build in any case. I can make some default GLContext/getCurrentGLContext returning incrementing integer this way disabling any sharing. Would it work? Won't this fail for systems using EGL instead of GLX?
(In reply to comment #18) > Won't this fail for systems using EGL instead of GLX? Oops. Didn't see Noam's comment. Sorry!
Created attachment 121857 [details] Updated patch
Comment on attachment 121857 [details] Updated patch View in context: https://bugs.webkit.org/attachment.cgi?id=121857&action=review Some nitpicks, almost there. > Source/WebCore/platform/graphics/opengl/TextureMapperGL.cpp:249 > + GLContextDataMap::const_iterator mapEnd = glContextDataMap().end(); Use end instead of mapEnd > Source/WebCore/platform/graphics/opengl/TextureMapperGL.cpp:408 > void TextureMapperGL::initializeShaders() I think we should move this function into SharedData constructor, would save us lots of code.
Comment on attachment 121857 [details] Updated patch View in context: https://bugs.webkit.org/attachment.cgi?id=121857&action=review > Source/WebCore/platform/graphics/opengl/TextureMapperGL.cpp:160 > + static PassRefPtr<SharedGLData> curentSharedGLData() curent -> current
Created attachment 121907 [details] Updated patch
Comment on attachment 121907 [details] Updated patch Rejecting attachment 121907 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: kit-commit-queue/Source/WebKit/chromium/third_party/v8-i18n --revision 7 --non-interactive --force --accept theirs-conflict --ignore-externals' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' 45>At revision 7. ________ running '/usr/bin/python tools/clang/scripts/update.py --mac-only' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' ________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' Updating webkit projects from gyp files... Full output: http://queues.webkit.org/results/11141659
Created attachment 121921 [details] Fix changelog mess.
Comment on attachment 121921 [details] Fix changelog mess. Clearing flags on attachment: 121921 Committed r104651: <http://trac.webkit.org/changeset/104651>
All reviewed patches have been landed. Closing bug.
Comment on attachment 121921 [details] Fix changelog mess. View in context: https://bugs.webkit.org/attachment.cgi?id=121921&action=review > Source/WebCore/platform/graphics/opengl/TextureMapperGL.cpp:122 > + static GLContext getCurrentGLContext() > + { > + return eglGetCurrentContext(); > + } The use of functions from libEGL also requires changes to the .pro files to link in libEGL. This is missing at the moment and prevent builds on platforms where EGL is used.
(In reply to comment #28) > (From update of attachment 121921 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=121921&action=review > > > Source/WebCore/platform/graphics/opengl/TextureMapperGL.cpp:122 > > + static GLContext getCurrentGLContext() > > + { > > + return eglGetCurrentContext(); > > + } > > The use of functions from libEGL also requires changes to the .pro files to link in libEGL. This is missing at the moment and prevent builds on platforms where EGL is used. Filed bug #76268.