The bug originated from begining native painting mode of QPainter after enabling scissor test, because begining native painting mode disables scissor test.
Created attachment 146691 [details] patch v.1
Comment on attachment 146691 [details] patch v.1 View in context: https://bugs.webkit.org/attachment.cgi?id=146691&action=review > Source/WebCore/platform/graphics/texmap/TextureMapperGL.cpp:-270 > glDisable(GL_DEPTH_TEST); > glEnable(GL_SCISSOR_TEST); > -#if PLATFORM(QT) > - if (m_context) { > - QPainter* painter = m_context->platformContext(); > - painter->save(); > - painter->beginNativePainting(); > - } > -#endif Shouldn't the new code be called before the other GL calls?
Comment on attachment 146691 [details] patch v.1 View in context: https://bugs.webkit.org/attachment.cgi?id=146691&action=review >> Source/WebCore/platform/graphics/texmap/TextureMapperGL.cpp:-270 >> -#endif > > Shouldn't the new code be called before the other GL calls? I think there is no problem to put initializeOpenGLShims() before PLATFORM(QT) code, because initializeOpenGLShims() does not change any GL states and QPainter does not need something like OpenGLShims. Moreover, I think it is more obvious to save gl states for QPainter before starting Texmap routines like TextureMapperGL::endPainting restores gl states for QPainter after all Texmap tasks are finished.
Oops, I made typo (In reply to comment #3) > (From update of attachment 146691 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=146691&action=review > > >> Source/WebCore/platform/graphics/texmap/TextureMapperGL.cpp:-270 > >> -#endif > > > > Shouldn't the new code be called before the other GL calls? > > I think there is no problem to put initializeOpenGLShims() before PLATFORM(QT) code, because initializeOpenGLShims() does not change any GL states and QPainter does not need something like OpenGLShims. before -> after > Moreover, I think it is more obvious to save gl states for QPainter before starting Texmap routines like TextureMapperGL::endPainting restores gl states for QPainter after all Texmap tasks are finished.
(In reply to comment #2) > (From update of attachment 146691 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=146691&action=review > > > Source/WebCore/platform/graphics/texmap/TextureMapperGL.cpp:-270 > > glDisable(GL_DEPTH_TEST); > > glEnable(GL_SCISSOR_TEST); > > -#if PLATFORM(QT) > > - if (m_context) { > > - QPainter* painter = m_context->platformContext(); > > - painter->save(); > > - painter->beginNativePainting(); > > - } > > -#endif > > Shouldn't the new code be called before the other GL calls? Would you like to explain more detail. I think I don't understand what you mean.
(In reply to comment #5) > (In reply to comment #2) > > (From update of attachment 146691 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=146691&action=review > > > > > Source/WebCore/platform/graphics/texmap/TextureMapperGL.cpp:-270 > > > glDisable(GL_DEPTH_TEST); > > > glEnable(GL_SCISSOR_TEST); > > > -#if PLATFORM(QT) > > > - if (m_context) { > > > - QPainter* painter = m_context->platformContext(); > > > - painter->save(); > > > - painter->beginNativePainting(); > > > - } > > > -#endif > > > > Shouldn't the new code be called before the other GL calls? > > Would you like to explain more detail. I think I don't understand what you mean. i'm saying it should look more like this: > > > -#if PLATFORM(QT) > > > - if (m_context) { > > > - QPainter* painter = m_context->platformContext(); > > > - painter->save(); > > > - painter->beginNativePainting(); > > > - } > > > -#endif > > > glDisable(GL_DEPTH_TEST); > > > glEnable(GL_SCISSOR_TEST); > >
Created attachment 147022 [details] patch v.2
Comment on attachment 147022 [details] patch v.2 View in context: https://bugs.webkit.org/attachment.cgi?id=147022&action=review > Source/WebCore/platform/graphics/texmap/TextureMapperGL.cpp:270 > data().previousDepthState = glIsEnabled(GL_DEPTH_TEST); I'm afraid that it is right to save previous states before begin NativePainting, because restoring previous states is before ending NativePainting in TextureMapperGL::endPainting(). It is asymmetric between beginPainting and endPainting.
Comment on attachment 147022 [details] patch v.2 OK, I misread the patch. LGtm.
Comment on attachment 147022 [details] patch v.2 Clearing flags on attachment: 147022 Committed r120066: <http://trac.webkit.org/changeset/120066>
All reviewed patches have been landed. Closing bug.