922368 - Child Process assert: mGL->IsExtensionSupported(GLContext::EXT_framebuffer_blit) || mGL->IsExtensionSupported(GLContext::ANGLE_framebuffer_blit))

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[Gregor Wagner [:gwagner]]

With --enable-debug build on nexus 4 with 
changeset:   149256:70398298d610
tag:         tip
user:        Gaia Pushbot <[e-mail]>
date:        Mon Sep 30 12:10:53 2013 -0700

STR:
Open Message app
Click on attachement -> Gallery
Choose Picture

Program received signal SIGSEGV, Segmentation fault.
0xb5b52bb6 in AssureBlitted (this=0xb176aa00)
    at ../../../gfx/gl/GLScreenBuffer.cpp:339
339	        MOZ_ASSERT(mGL->IsExtensionSupported(GLContext::EXT_framebuffer_blit) ||
(gdb) l
334	        GLuint drawFB = DrawFB();
335	        GLuint readFB = ReadFB();
336	
337	        MOZ_ASSERT(drawFB != 0);
338	        MOZ_ASSERT(drawFB != readFB);
339	        MOZ_ASSERT(mGL->IsExtensionSupported(GLContext::EXT_framebuffer_blit) ||
340	                   mGL->IsExtensionSupported(GLContext::ANGLE_framebuffer_blit));
341	        MOZ_ASSERT(mDraw->Size() == mRead->Size());
342	
343	        ScopedBindFramebuffer boundFB(mGL);
(gdb) bt
#0  0xb5b52bb6 in AssureBlitted (this=0xb176aa00)
    at ../../../gfx/gl/GLScreenBuffer.cpp:339
#1  mozilla::gl::GLScreenBuffer::AssureBlitted (this=0xb176aa00)
    at ../../../gfx/gl/GLScreenBuffer.cpp:328
#2  0xb5b5321a in mozilla::gl::GLScreenBuffer::PublishFrame (this=0xb176aa00, 
    size=...) at ../../../gfx/gl/GLScreenBuffer.cpp:442
#3  0xb518a114 in mozilla::WebGLContext::PresentScreenBuffer (this=0xb3eca090)
    at ../../../../content/canvas/src/WebGLContext.cpp:1143
#4  0xb518a152 in mozilla::WebGLContextUserData::PreTransactionCallback (
    data=<optimized out>)
    at ../../../../content/canvas/src/WebGLContext.cpp:818
#5  0xb5b0d552 in FirePreTransactionCallback (this=0xb3404d80)
    at ../../../gfx/layers/Layers.h:1781
#6  mozilla::layers::ClientCanvasLayer::RenderLayer (this=0xb3404d80)
    at ../../../gfx/layers/client/ClientCanvasLayer.cpp:119
#7  0xb5b0dfa6 in mozilla::layers::ClientContainerLayer::RenderLayer (
    this=<optimized out>)
    at ../../../gfx/layers/client/ClientContainerLayer.h:81
#8  0xb5b0e6f4 in mozilla::layers::ClientLayerManager::EndTransactionInternal (
    this=0xb31bd200, aCallback=
    0xb4f23849 <mozilla::FrameLayerBuilder::DrawThebesLayer(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, void*)>, 
    aCallbackData=<optimized out>)
---Type <return> to continue, or q <return> to quit---
    at ../../../gfx/layers/client/ClientLayerManager.cpp:185
#9  0xb5b0ee9e in mozilla::layers::ClientLayerManager::EndTransaction (
    this=0xb31bd200, 
    aCallback=0xb4f23849 <mozilla::FrameLayerBuilder::DrawThebesLayer(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, void*)>, aCallbackData=0xbe8ac718, 
    aFlags=mozilla::layers::LayerManager::END_NO_COMPOSITE)
    at ../../../gfx/layers/client/ClientLayerManager.cpp:208
#10 0xb4f548f8 in nsDisplayList::PaintForFrame (this=0xbe8ac66c, 
    aBuilder=0xbe8ac718, aCtx=<optimized out>, aForFrame=<optimized out>, 
    aFlags=13) at ../../../layout/base/nsDisplayList.cpp:1270
#11 0xb4f54b10 in nsDisplayList::PaintRoot (this=0xbe8ac66c, 
    aBuilder=0xbe8ac718, aCtx=0x0, aFlags=13)
    at ../../../layout/base/nsDisplayList.cpp:1117
#12 0xb4f6a430 in nsLayoutUtils::PaintFrame (aRenderingContext=0x0, aFrame=
    0xb2e8c298, aDirtyRegion=<optimized out>, aBackstop=0, aFlags=772)
    at ../../../layout/base/nsLayoutUtils.cpp:2150
#13 0xb4f7f4b4 in PresShell::Paint (this=0xb2e41ad0, 
    aViewToPaint=<optimized out>, aDirtyRegion=..., aFlags=<optimized out>)
    at ../../../layout/base/nsPresShell.cpp:5660
#14 0xb52f8904 in ProcessPendingUpdatesForView (aView=0xb2a3dec0, 
    this=0xb2aa8be0, aFlushDirtyRegion=<optimized out>)
    at ../../../view/src/nsViewManager.cpp:411
---Type <return> to continue, or q <return> to quit---
#15 nsViewManager::ProcessPendingUpdatesForView (this=0xb2aa8be0, 
    aView=0xb2a3dec0, aFlushDirtyRegion=<optimized out>)
    at ../../../view/src/nsViewManager.cpp:362
#16 0xb4f88330 in nsRefreshDriver::Tick (this=0xb3e927c0, 
    aNowEpoch=<optimized out>, aNowTime=...)
    at ../../../layout/base/nsRefreshDriver.cpp:1204
#17 0xb4f88808 in mozilla::RefreshDriverTimer::Tick (this=0xb2a32580)
    at ../../../layout/base/nsRefreshDriver.cpp:158
#18 0xb5aa553e in nsTimerImpl::Fire (this=0xb2a0b4c0)
    at ../../../xpcom/threads/nsTimerImpl.cpp:546
#19 0xb5aa56c8 in nsTimerEvent::Run (this=0xb2a1e050)
    at ../../../xpcom/threads/nsTimerImpl.cpp:630
#20 0xb5aa2072 in nsThread::ProcessNextEvent (this=0xb3e02320,

Comment 1

[Gregor Wagner [:gwagner]]

Milan, can you help here? This is still crashing and it is 100% reproducible.

Comment 2

[Milan Sreckovic [:milan] (needinfo for best results)]

Nexus 4 only?  We're trying to get the 1.2 issues out of the way first, but Jeff, is there a quick answer here?  I mean, if the extensions are necessary and the above is an assertion trigger, and Nexus 4 doesn't support them (somehow), then I'm not sure what we can do.  If the above is not an assertion trigger but we're crashing because mGL is NULL (can that ever happen?), that's a different story.

Comment 3

[Kelsey Gilbert [:jgilbert]]

Yep, this is my bad. Fix coming later today.

Comment 4

[Kelsey Gilbert [:jgilbert]]

This should be be happening on all android devices. (Thanks, no-debug-android-tests!)

Comment 5

[Gregor Wagner [:gwagner]]

(In reply to Milan Sreckovic [:milan] from comment #2)
> Nexus 4 only?  We're trying to get the 1.2 issues out of the way first, but
> Jeff, is there a quick answer here?  I mean, if the extensions are necessary
> and the above is an assertion trigger, and Nexus 4 doesn't support them
> (somehow), then I'm not sure what we can do.  If the above is not an
> assertion trigger but we're crashing because mGL is NULL (can that ever
> happen?), that's a different story.

The nexus4 is the only phone I have that is good enough to run with gdb and --enable-debug. So I don't know if it's nexus 4 or 1.3 only :(
I am ok if someone looks at it and says it's not a problem on 1.3 or it's nexus 4 only but ignoring it because I only test on nexus 4 seems risky.

Comment 6

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

Nexus 4 definitely doesn't support this extension -- see https://gfxbench.com/device.jsp?benchmark=gfx27&D=Google+Nexus+4&testgroup=gl .  This likely means that Adreno 320 doesn't have EXT_framebuffer_blit at all.  Dunno about other Android devices.

Comment 7

[Kelsey Gilbert [:jgilbert]]

(In reply to Vladimir Vukicevic [:vlad] [:vladv] from comment #6)
> Nexus 4 definitely doesn't support this extension -- see
> https://gfxbench.com/device.
> jsp?benchmark=gfx27&D=Google+Nexus+4&testgroup=gl .  This likely means that
> Adreno 320 doesn't have EXT_framebuffer_blit at all.  Dunno about other
> Android devices.

Very few have support for multisampled renderbuffers, so we should be debug-crashing on most phones.

This is just me adding a function with the assumption that all our asserts would let us know if we did something wrong. It worked, we know now, but we should have known at the time of the push.

Comment 8

[Kelsey Gilbert [:jgilbert]]

For clarification, we don't support antialiased webgl on mobile devices yes, so we definitely shouldn't be hitting this path. I'll have a patch in an hour.

Comment 9

[Kelsey Gilbert [:jgilbert]]

Actually, this is totally something else, as it predates the fix that added the new AssureBlitted.

Comment 10

[Kelsey Gilbert [:jgilbert]]

Anyways, I have a nexus 4, so I'll just test this.

Comment 11

[Kelsey Gilbert [:jgilbert]]

So I have good news that is also bad news.
This is because the N4 gives us a GLES3 GLContext, for which framebuffer_blit and friends are core.
This means we just need to fix the old asserts to check the Feature for MSFBs instead of doing extensions itself.

Good news the second: This should mean that antialiasing should work finally on GLES3 devices, such as the N4.
This is also bad news: One reason we haven't worked on enabling AA on mobile before is that it's not cheap for the already-taxed hardware, and also it doesn't matter as much, given the pixel densities on mobile devices.

Since aa defaults to `true` for webgl contexts, this means that many demos will suddenly run slower on these devices. One thing we've discussed in the past is forbidding AA contexts on mobile (within spec) or defaulting to `false` on mobile. (non-spec)

I think we should try to respec this such that the default for `antialias` can be device dependent. This way, things continue at the same speed as before, but apps which deliberately opt into antialiasing would get better quality.

If we just fix AA for mobile, I anticipate a number of bugs about FF being suddenly slower, or slower than X other browser on a number of devices. This is probably best handled in another bug. I created bug 925530 for this.

Comment 12

[Kelsey Gilbert [:jgilbert]]

This was reported against Gonk, but I'm reproing it on android.

Comment 13

[Kelsey Gilbert [:jgilbert]]

Attachment: patch: Check for features instead of framebuffer_blit extensions

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to Jeff Gilbert [:jgilbert] from comment #11)
> I think we should try to respec this such that the default for `antialias`
> can be device dependent. This way, things continue at the same speed as
> before, but apps which deliberately opt into antialiasing would get better
> quality.

FWIW that sounds like a great approach to me.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/b45bcc12e8a1

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b45bcc12e8a1

Comment 18

[Jason Smith [:jsmith]]

Already in 1.3, clearing nom.