598970 - GL/EGL qt rendering is broken completely after landing 596784

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Oleg Romashin (:romaxa)]

Image rendering pipeline with Qt build, show only black screen:
MOZ_ACCELERATED=1 MOZ_QT_GRAPHICSSYSTEM=raster ./fennec -graphicssystem raster
 -software -url about:license

X-Rendering pipeline crashes with backtrace:
MOZ_ACCELERATED=1 MOZ_QT_GRAPHICSSYSTEM=native ./fennec -graphicssystem native
 -software -url about:license

#0  0x4004677c in raise (sig=11) at ../nptl/sysdeps/unix/sysv/linux/pt-raise.c:45
#1  0x402db34a in nsProfileLock::FatalSignalHandler (signo=11, info=0xbee9fce8, context=0xbee9fd68) at nsProfileLock.cpp:213
#2  <signal handler called>
#3  0xc07e883c in ?? ()
#4  0x40c17f96 in mozilla::gl::GLContext::fDeleteFramebuffers (this=0x47973400)
    at gfx/thebes/GLContext.h:1207
#5  mozilla::gl::GLContext::DeleteOffscreenFBO (this=0x47973400)
    at gfx/thebes/GLContext.cpp:759
#6  0x40c181cc in mozilla::gl::GLContext::MarkDestroyed (this=0x47973400)
    at gfx/thebes/GLContext.cpp:814
#7  0x40c26696 in ~GLContextEGL (this=0x1, __in_chrg=<value optimized out>)
    at gfx/thebes/GLContextProviderEGL.cpp:526
#8  0x40c2661c in mozilla::gl::GLContext::Release (this=0x4797f800, __in_chrg=<value optimized out>)
    at gfx/thebes/GLContext.h:352
#9  nsRefPtr<mozilla::gl::GLContextEGL>::assign_assuming_AddRef (this=0x4797f800, __in_chrg=<value optimized out>)
    at ../../dist/include/nsAutoPtr.h:957
#10 nsRefPtr<mozilla::gl::GLContextEGL>::assign_with_AddRef (this=0x4797f800, __in_chrg=<value optimized out>)
---Type <return> to continue, or q <return> to quit---
    at ../../dist/include/nsAutoPtr.h:941
#11 nsRefPtr<mozilla::gl::GLContextEGL>::operator= (this=0x4797f800, __in_chrg=<value optimized out>)
    at ../../dist/include/nsAutoPtr.h:1025
#12 ~TextureImageEGL (this=0x4797f800, __in_chrg=<value optimized out>)
    at gfx/thebes/GLContextProviderEGL.cpp:861
#13 0x40c36526 in mozilla::gl::TextureImage::Release (this=0x4759cc80, aType=<value optimized out>, 
    aSize=<value optimized out>) at ../../dist/include/GLContext.h:142
#14 nsRefPtr<mozilla::gl::TextureImage>::assign_assuming_AddRef (this=0x4759cc80, aType=<value optimized out>, 
    aSize=<value optimized out>) at ../../dist/include/nsAutoPtr.h:957
#15 operator=<mozilla::gl::TextureImage> (this=0x4759cc80, aType=<value optimized out>, aSize=<value optimized out>)
    at ../../dist/include/nsAutoPtr.h:1034
#16 mozilla::layers::SurfaceBufferOGL::CreateBuffer (this=0x4759cc80, aType=<value optimized out>, 
    aSize=<value optimized out>) at gfx/layers/opengl/ThebesLayerOGL.cpp:214
#17 0x40c2d4dc in mozilla::layers::ThebesLayerBuffer::BeginPaint (this=0x4759cc90, aLayer=<value optimized out>, 
    aContentType=gfxASurface::CONTENT_COLOR, aXResolution=) at gfx/layers/ThebesLayerBuffer.cpp:259
#18 0x40c364da in mozilla::layers::SurfaceBufferOGL::BeginPaint (this=<value optimized out>, aContentType=3229517884)
    at gfx/layers/opengl/ThebesLayerOGL.cpp:205
#19 0x40c363ae in mozilla::layers::ThebesLayerOGL::RenderLayer (this=0x47915300, aPreviousFrameBuffer=70001, aOffset=...)
    at gfx/layers/opengl/ThebesLayerOGL.cpp:497
#20 0x40c2ee58 in mozilla::layers::ContainerLayerOGL::RenderLayer (this=0x47ff26a0, aPreviousFrameBuffer=70001, aOffset=...)
    at gfx/layers/opengl/ContainerLayerOGL.cpp:205
#21 0x40c2ee58 in mozilla::layers::ContainerLayerOGL::RenderLayer (this=0x471df740, aPreviousFrameBuffer=70001, aOffset=...)
    at gfx/layers/opengl/ContainerLayerOGL.cpp:205
#22 0x40c31f54 in mozilla::layers::LayerManagerOGL::Render (this=0x47586b30)
---Type <return> to continue, or q <return> to quit---n
    at gfx/layers/opengl/LayerManagerOGL.cpp:572
#23 0x40c322d6 in mozilla::layers::LayerManagerOGL::EndTransaction (this=0x47586b30, aCallback=0x47973498, aCallbackData=0x0)
    at gfx/layers/opengl/LayerManagerOGL.cpp:396
#24 0x403e6152 in nsDisplayList::PaintForFrame (this=<value optimized out>, aBuilder=0xbeea0628, aCtx=<value optimized out>, 
    aForFrame=<value optimized out>, aFlags=1)
    at layout/base/nsDisplayList.cpp:452
#25 0x403e62b8 in nsDisplayList::PaintRoot (this=0x1, aBuilder=0x47973498, aCtx=0x0, aFlags=<value optimized out>)
    at layout/base/nsDisplayList.cpp:360
#26 0x403f1826 in nsLayoutUtils::PaintFrame (aRenderingContext=0x0, aFrame=0x465f16d8, aDirtyRegion=..., 
    aBackstop=4293914607, aFlags=4) at layout/base/nsLayoutUtils.cpp:1429
#27 0x403fb078 in PresShell::Paint (this=0x4499a5e0, aDisplayRoot=0x45f016a0, aViewToPaint=0x45f016a0, 
    aWidgetToPaint=0x4659b200, aDirtyRegion=..., aIntDirtyRegion=..., aPaintDefaultBackground=0, aWillSendDidPaint=0)
    at layout/base/nsPresShell.cpp:6089
#28 0x406685b4 in nsViewManager::RenderViews (this=<value optimized out>, aView=0x45f016a0, aWidget=0x4659b200, 
    aRegion=<value optimized out>, aIntRegion=..., aPaintDefaultBackground=0, aWillSendDidPaint=0)
    at view/src/nsViewManager.cpp:447
#29 0x40668664 in nsViewManager::Refresh (this=0x465ee3d0, aView=0x45f016a0, aWidget=0x4659b200, aRegion=..., aUpdateFlags=1)
---Type <return> to continue, or q <return> to quit--- 
    at view/src/nsViewManager.cpp:413
#30 0x40669816 in nsViewManager::DispatchEvent (this=0x465ee3d0, aEvent=0xbeea0ce0, aView=0x45f016a0, 
    aStatus=<value optimized out>) at view/src/nsViewManager.cpp:913
#31 0x406662b4 in HandleEvent (aEvent=0xbeea0ce0) at view/src/nsView.cpp:161
#32 0x40a627be in nsWindow::DispatchEvent (this=<value optimized out>, aEvent=0x47973498, aStatus=@0xbeea0c44)
    at widget/src/qt/nsWindow.cpp:2580
#33 0x40a674b0 in nsWindow::DispatchEvent (this=0x1, aEvent=0x47973498)
    at widget/src/qt/nsWindow.h:221
#34 0x40a63c52 in nsWindow::DoPaint (this=0x4659b200, aPainter=0xbeea21f4, aOption=<value optimized out>)
    at widget/src/qt/nsWindow.cpp:965
#35 0x40a6c192 in MozQWidget::paint (this=<value optimized out>, aPainter=0x47973498, aOption=0x0, aWidget=0xc07e883c)
    at widget/src/qt/mozqwidget.cpp:97
#36 0x41b672b8 in _q_paintItem (item=<value optimized out>, painter=0xbeea21f4, option=0x458e3668, widget=0x4654de20, 
    useWindowOpacity=<value optimized out>, painterStateProtection=true) at graphicsview/qgraphicsscene.cpp:4318
#37 0x41b6de60 in QGraphicsScenePrivate::drawItemHelper (this=0x458e3540, item=0x465ef608, painter=0xbeea21f4, 
    option=0x458e3668, widget=0x4654de20, painterStateProtection=true) at graphicsview/qgraphicsscene.cpp:4387
#38 0x41b719c4 in QGraphicsScenePrivate::draw (this=0x458e3540, item=0x465ef608, painter=0xbeea21f4, viewTra

Comment 1

[Oleg Romashin (:romaxa)]

Ok I found that we are creating GLContext for pixmap surface, but not calling ResizeOffscreenFBO....

and all our members mOffscreenFBO, mOffscreenDepthRB,  just not created == 0

also DeleteOffscreenFBO(), and MarkDestroyed(), not checking that variables for NULL value.

Comment 2

[Oleg Romashin (:romaxa)]

Attachment: Fix egl crash with native (X) rendering

GL context still destroying very often, but at least we are not crashing on passing null objects to GL driver

Comment 3

[Oleg Romashin (:romaxa)]

Attachment: Generic EGL cleanup

Comment 4

[Oleg Romashin (:romaxa)]

Attachment: Disable opt surface for Qt Raster (image based backend)

Comment 5

[Oleg Romashin (:romaxa)]

Attachment: Add basic image path, used in all other implementation (for testing)

With basic image we don't have repeated GL context destroying, also we have almost good scrolling.
But content artifacts after scrolling still visible, even with basic image

Comment 6

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 479484 [details] [diff] [review]
Fix egl crash with native (X) rendering

this should be reported upstream (mesa or img) -- glDeleteFramebuffers explicitly says:

> glDeleteFramebuffers silently ignores 0's and names that do not correspond to existing framebuffer objects.

Same with glDeleteProgram.

I don't want to workaround buggy GL implementations without at least making sure a bug is on file for those implementations.

Comment 7

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 479512 [details] [diff] [review]
Generic EGL cleanup

Looks mostly ok, but two issues:

> already_AddRefed<GLContextEGL>
> GLContextEGL::CreateEGLPixmapOffscreenContext(const gfxIntSize& aSize,
>                                               const ContextFormat& aFormat)
> {
>     // XXX -- write me.
>     // This needs to find a FBConfig/Visual that matches aFormat, and allocate
>     // a gfxXlibSurface of the appropriat format, and then create a context
>     // for it.
>@@ -1297,62 +1376,33 @@ GLContextEGL::CreateEGLPixmapOffscreenCo
>     gfxASurface *thebesSurface = nsnull;
>     EGLNativePixmapType pixmap = 0;
> 
> #ifdef MOZ_X11
>     nsRefPtr<gfxXlibSurface> xsurface =
>         gfxXlibSurface::Create(DefaultScreenOfDisplay(DefaultXDisplay()),
>                                gfxXlibSurface::FindRenderFormat(DefaultXDisplay(),
>                                                                 gfxASurface::ImageFormatRGB24),
>-                               gfxIntSize(16, 16));
>+                               gfxIntSize(aSize.width, aSize.height));

No, this really should be 16x16 -- we're just using this pixmap as a dummy surface, and later on we'll create a FBO to do offscreen rendering in.

>+    config = FindConfigForThebesXSurface(thebesSurface,
>+                                         &surface);

Doesn't this need to be inside MOZ_X11 ifdefs?  Though perhaps the entire function needs to be, X11 is the only place where we'll try to create pixmap contexts.

Comment 8

[Oleg Romashin (:romaxa)]

Attachment: v2, EGL Cleanup, Updated to comment 7

Comment 9

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 479694 [details] [diff] [review]
v2, EGL Cleanup, Updated to comment 7

You need to keep the |if (!config) return nsnull;| bit -- outside of the #if, after getting it for the X surface; double check that config is initialized to null at the top as well.  r+ with that.

Comment 10

[Oleg Romashin (:romaxa)]

Attachment: EGL cleanup v3, comment 9 double check, FOR PUSH

Comment 11

[Oleg Romashin (:romaxa)]

Comment on attachment 479716 [details] [diff] [review]
EGL cleanup v3, comment 9 double check, FOR PUSH

Moving r+ from attachment.cgi\?id\=479694

Comment 12

[Oleg Romashin (:romaxa)]

Comment on attachment 479484 [details] [diff] [review]
Fix egl crash with native (X) rendering

Ok, Meego graphics guys has fixed the problem.

Comment 13

[Oleg Romashin (:romaxa)]

I think this is definitely blocking 
"Enable GLES for Maemo bug"

Comment 14

[Oleg Romashin (:romaxa)]

Comment on attachment 479484 [details] [diff] [review]
Fix egl crash with native (X) rendering

Obsolete, fixed in maemo drivers

Comment 15

[Oleg Romashin (:romaxa)]

without these 2 patches, we are not able even to start working on EGL qt backend..., must have this before fixing artifacts and other EGL/GL related problems

Comment 16

[Oleg Romashin (:romaxa)]

http://hg.mozilla.org/mozilla-central/rev/523a116d617f