1103258 - MOZ_ASSERT(aRect.width != 0 && aRect.height != 0, "Trying to create a render target of invalid size");

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Gregor Wagner [:gwagner]]

On flame with debug gecko on current trunk.
I see this during monkey testing with the Enter 2 Pin Screen:

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 1133.1190]
0xb47a910e in mozilla::layers::CompositorOGL::CreateRenderTarget (this=0xb13db200, aRect=..., aInit=mozilla::layers::INIT_MODE_CLEAR) at ../../../gfx/layers/opengl/CompositorOGL.cpp:455
455	  MOZ_ASSERT(aRect.width != 0 && aRect.height != 0, "Trying to create a render target of invalid size");
(gdb) bt
#0  0xb47a910e in mozilla::layers::CompositorOGL::CreateRenderTarget (this=0xb13db200, aRect=..., aInit=mozilla::layers::INIT_MODE_CLEAR) at ../../../gfx/layers/opengl/CompositorOGL.cpp:455
#1  0xb47865e6 in mozilla::layers::CreateOrRecycleTarget<mozilla::layers::ContainerLayerComposite> (aContainer=aContainer@entry=0xa8a1c400, aManager=aManager@entry=0xab92dac0)
    at ../../../gfx/layers/composite/ContainerLayerComposite.cpp:315
#2  0xb478e0aa in mozilla::layers::ContainerPrepare<mozilla::layers::ContainerLayerComposite> (aContainer=0xa8a1c400, aManager=0xab92dac0, aClipRect=...) at ../../../gfx/layers/composite/ContainerLayerComposite.cpp:203
#3  0xb478e164 in mozilla::layers::ContainerPrepare<mozilla::layers::ContainerLayerComposite> (aContainer=0xa85c3800, aManager=0xab92dac0, aClipRect=...) at ../../../gfx/layers/composite/ContainerLayerComposite.cpp:177
#4  0xb478e164 in mozilla::layers::ContainerPrepare<mozilla::layers::ContainerLayerComposite> (aContainer=0xa85bdc00, aManager=0xab92dac0, aClipRect=...) at ../../../gfx/layers/composite/ContainerLayerComposite.cpp:177
#5  0xb478e164 in mozilla::layers::ContainerPrepare<mozilla::layers::ContainerLayerComposite> (aContainer=0xa8596800, aManager=0xab92dac0, aClipRect=...) at ../../../gfx/layers/composite/ContainerLayerComposite.cpp:177
#6  0xb478e164 in mozilla::layers::ContainerPrepare<mozilla::layers::ContainerLayerComposite> (aContainer=0xaec13000, aManager=0xab92dac0, aClipRect=...) at ../../../gfx/layers/composite/ContainerLayerComposite.cpp:177
#7  0xb478e164 in mozilla::layers::ContainerPrepare<mozilla::layers::ContainerLayerComposite> (aContainer=0xaea98c00, aManager=0xab92dac0, aClipRect=...) at ../../../gfx/layers/composite/ContainerLayerComposite.cpp:177
#8  0xb478b508 in mozilla::layers::LayerManagerComposite::Render (this=this@entry=0xab92dac0) at ../../../gfx/layers/composite/LayerManagerComposite.cpp:722
#9  0xb478b8f8 in EndTransaction (aFlags=<optimized out>, this=0xab92dac0, aCallback=<optimized out>, aCallbackData=<optimized out>) at ../../../gfx/layers/composite/LayerManagerComposite.cpp:309
#10 mozilla::layers::LayerManagerComposite::EndTransaction (this=0xab92dac0, aCallback=<optimized out>, aCallbackData=<optimized out>, aFlags=<optimized out>)
    at ../../../gfx/layers/composite/LayerManagerComposite.cpp:261
#11 0xb478b9de in mozilla::layers::LayerManagerComposite::EndEmptyTransaction (this=0xab92dac0, aFlags=aFlags@entry=mozilla::layers::LayerManager::END_DEFAULT)
    at ../../../gfx/layers/composite/LayerManagerComposite.cpp:256
#12 0xb47a2242 in mozilla::layers::CompositorParent::CompositeToTarget (this=0xad12a800, aTarget=0x0, aRect=<optimized out>) at ../../../gfx/layers/ipc/CompositorParent.cpp:903
#13 0xb4790d0c in DispatchToMethod<mozilla::layers::CompositorParent, void (mozilla::layers::CompositorParent::*)(mozilla::TimeStamp), mozilla::TimeStamp> (arg=..., method=
    (void (mozilla::layers::CompositorParent::*)(mozilla::layers::CompositorParent * const, mozilla::TimeStamp)) 0xb47a2391 <mozilla::layers::CompositorParent::CompositeCallback(mozilla::TimeStamp)>, 
    obj=<optimized out>) at ../../../ipc/chromium/src/base/tuple.h:393
#14 RunnableMethod<mozilla::layers::CompositorParent, void (mozilla::layers::CompositorParent::*)(mozilla::TimeStamp), Tuple1<mozilla::TimeStamp> >::Run (this=<optimized out>)
    at ../../../ipc/chromium/src/base/task.h:307
#15 0xb443af9c in MessageLoop::RunTask (this=0xaf7ffcc0, task=0xa8ea6250) at ../../../ipc/chromium/src/base/message_loop.cc:361
#16 0xb443e14a in MessageLoop::DeferOrRunPendingTask (this=this@entry=0xaf7ffcc0, pending_task=...) at ../../../ipc/chromium/src/base/message_loop.cc:369
#17 0xb443e452 in DoDelayedWork (next_delayed_work_time=0xafde6a30, this=0xaf7ffcc0) at ../../../ipc/chromium/src/base/message_loop.cc:474
#18 MessageLoop::DoDelayedWork (this=0xaf7ffcc0, next_delayed_work_time=0xafde6a30) at ../../../ipc/chromium/src/base/message_loop.cc:457
#19 0xb443b092 in base::MessagePumpDefault::Run (this=0xafde6a20, delegate=0xaf7ffcc0) at ../../../ipc/chromium/src/base/message_pump_default.cc:39
#20 0xb443c0e4 in MessageLoop::RunInternal (this=this@entry=0xaf7ffcc0) at ../../../ipc/chromium/src/base/message_loop.cc:233
#21 0xb443c0fe in RunHandler (this=0xaf7ffcc0) at ../../../ipc/chromium/src/base/message_loop.cc:226
#22 MessageLoop::Run (this=this@entry=0xaf7ffcc0) at ../../../ipc/chromium/src/base/message_loop.cc:200
#23 0xb4441e1c in base::Thread::ThreadMain (this=0xb1368f70) at ../../../ipc/chromium/src/base/thread.cc:170
#24 0xb4434d74 in ThreadFunc (closure=<optimized out>) at ../../../ipc/chromium/src/base/platform_thread_posix.cc:39
#25 0xb6e8822c in __thread_entry (func=0xb4434d6d <ThreadFunc(void*)>, arg=0xb1368f70, tls=0xaf7ffdd0) at bionic/libc/bionic/pthread_create.cpp:105
#26 0xb6e883c4 in pthread_create (thread_out=0xb1368f78, attr=<optimized out>, start_routine=0xb4434d6d <ThreadFunc(void*)>, arg=0x78) at bionic/libc/bionic/pthread_create.cpp:224

Comment 1

[Gregor Wagner [:gwagner]]

(gdb) p aRect
$1 = (const 
    mozilla::gfx::IntRect &) @0xaf7ff5a8: {<mozilla::gfx::BaseRect<int, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits>, mozilla::gfx::IntPointTyped<mozilla::gfx::UnknownUnits>, mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits>, mozilla::gfx::IntMarginTyped<mozilla::gfx::UnknownUnits> >> = {x = 352, y = 1, width = 0, height = 0}, <mozilla::gfx::UnknownUnits> = {<No data fields>}, <No data fields>}
(gdb) p this
$2 = (mozilla::layers::CompositorOGL * const) 0xb13db200
(gdb) p *this
$3 = {<mozilla::layers::Compositor> = {_vptr.Compositor = 0xb64d59e0 <vtable for mozilla::layers::CompositorOGL+8>, mRefCnt = {static isThreadSafe = false, mValue = 167}, _mOwningThread = {mThread = 0xaf904400}, 
    static ForceMakeCurrent = 1, mCompositorID = 2, mDiagnosticTypes = mozilla::layers::NO_DIAGNOSTIC, mParent = 0x0, mPixelsPerFrame = 409920, mPixelsFilled = 0, mScreenRotation = mozilla::ROTATION_0, mTarget = {
      mPtr = 0x0}, mTargetBounds = {<mozilla::gfx::BaseRect<int, nsIntRect, nsIntPoint, nsIntSize, nsIntMargin>> = {x = 0, y = 0, width = 0, height = 0}, <No data fields>}, static sBackend = 
    mozilla::layers::LAYERS_OPENGL}, mPrograms = {
    _M_t = {<std::priv::_Rb_tree_base<std::pair<mozilla::layers::ShaderConfigOGL const, mozilla::layers::ShaderProgramOGL*>, std::allocator<std::pair<mozilla::layers::ShaderConfigOGL const, mozilla::layers::ShaderProgramOGL*> > >> = {
        _M_header = {<std::allocator<std::priv::_Rb_tree_node<std::pair<mozilla::layers::ShaderConfigOGL const, mozilla::layers::ShaderProgramOGL*> > >> = {<std::__stlport_class<std::allocator<std::priv::_Rb_tree_node<std::pair<mozilla::layers::ShaderConfigOGL const, mozilla::layers::ShaderProgramOGL*> > > >> = {<No data fields>}, <No data fields>}, _M_data = {_M_color = false, _M_parent = 0xa8af6080, _M_left = 0xacef6a80, 
            _M_right = 0xac8f3980}}}, _M_node_count = 12, 
      _M_key_compare = {<std::binary_function<mozilla::layers::ShaderConfigOGL, mozilla::layers::ShaderConfigOGL, bool>> = {<No data fields>}, <std::__stlport_class<std::less<mozilla::layers::ShaderConfigOGL> >> = {<No data fields>}, <No data fields>}}}, mWidget = 0xafcad4d0, mWidgetSize = {<mozilla::gfx::BaseSize<int, nsIntSize>> = {width = 480, height = 854}, <No data fields>}, mGLContext = {mRawPtr = 0xafc95000}, 
  mBlitTextureImageHelper = {
    mTuple = {<mozilla::detail::PairHelper<mozilla::layers::GLBlitTextureImageHelper*, mozilla::DefaultDelete<mozilla::layers::GLBlitTextureImageHelper>, (mozilla::detail::StorageType)1, (mozilla::detail::StorageType)0>> = {<mozilla::DefaultDelete<mozilla::layers::GLBlitTextureImageHelper>> = {<No data fields>}, mFirstA = 0x0}, <No data fields>}}, mProjMatrix = {_11 = 0.00416666688, _12 = 0, _13 = 0, _14 = 0, _21 = -0, 
    _22 = -0.00234192028, _23 = 0, _24 = 0, _31 = 0, _32 = 0, _33 = 0, _34 = 0, _41 = -1, _42 = 1, _43 = 0, _44 = 1}, mSurfaceSize = {<mozilla::gfx::BaseSize<int, nsIntSize>> = {width = 480, 
      height = 854}, <No data fields>}, mRenderOffset = {<mozilla::gfx::BasePoint<float, mozilla::gfx::PointTyped<mozilla::ScreenPixel>, mozilla::gfx::CoordTyped<mozilla::ScreenPixel> >> = {x = 0, 
      y = 0}, <mozilla::ScreenPixel> = {<No data fields>}, <No data fields>}, mFBOTextureTarget = 3553, mCurrentRenderTarget = {mPtr = 0xa8ef54c0}, mWindowRenderTarget = 0xa8ef54c0, mQuadVBO = 1, mHasBGRA = true, 
  mUseExternalSurfaceSize = false, mFrameInProgress = true, mTexturePool = {mPtr = 0xa85ddce0}, mContextStateTracker = {<mozilla::ContextStateTracker> = {
      mCompletedSections = {<nsTArray_Impl<mozilla::ContextStateTracker::ContextState, nsTArrayInfallibleAllocator>> = {<nsTArray_base<nsTArrayInfallibleAllocator, nsTArray_CopyWithMemutils>> = {
            mHdr = 0xb66d6c30 <nsTArrayHeader::sEmptyHdr>}, <nsTArray_TypedBase<mozilla::ContextStateTracker::ContextState, nsTArray_Impl<mozilla::ContextStateTracker::ContextState, nsTArrayInfallibleAllocator> >> = {<nsTArray_SafeElementAtHelper<mozilla::ContextStateTracker::ContextState, nsTArray_Impl<mozilla::ContextStateTracker::ContextState, nsTArrayInfallibleAllocator> >> = {<No data fields>}, <No data fields>}, 
          static NoIndex = <optimized out>}, <No data fields>}, 
      mSectionStack = {<nsTArray_Impl<mozilla::ContextStateTracker::ContextState, nsTArrayInfallibleAllocator>> = {<nsTArray_base<nsTArrayInfallibleAllocator, nsTArray_CopyWithMemutils>> = {
            mHdr = 0xb66d6c30 <nsTArrayHeader::sEmptyHdr>}, <nsTArray_TypedBase<mozilla::ContextStateTracker::ContextState, nsTArray_Impl<mozilla::ContextStateTracker::ContextState, nsTArrayInfallibleAllocator> >> = {<nsTArray_SafeElementAtHelper<mozilla::ContextStateTracker::ContextState, nsTArray_Impl<mozilla::ContextStateTracker::ContextState, nsTArrayInfallibleAllocator> >> = {<No data fields>}, <No data fields>}, 
          static NoIndex = <optimized out>}, <No data fields>}}, <No data fields>}, mDestroyed = false, mHeight = 854, mReleaseFenceHandle = {mFence = {m_ptr = 0xadda7468}}, mCurrentProgram = 0xb0bbb800}

Comment 2

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

BenWa, with your recent change, would we expect to get an empty rect in http://dxr.mozilla.org/mozilla-central/source/gfx/layers/composite/ContainerLayerComposite.cpp#408, and should we check if it's empty before passing it down to CreateRenderTarget?

Comment 3

[Benoit Girard (:BenWa)]

My change shouldn't have changed how the surface sizes are calculated.

This could be a fallout of something like culling. We could just skip the intermediate surface if the size is zero. This is assuming that the size calculation is correct.

Comment 4

[Benoit Girard (:BenWa)]

Attachment: simplified test case

100% reproducible for me:

This has a layer that uses an intermediate surface for group opacity. I then position another active layer to cull it. This makes us try to create an empty render target because it's been culled.

Comment 5

[Benoit Girard (:BenWa)]

Attachment: Part 0: Add a reftest

Comment 6

[Benoit Girard (:BenWa)]

Attachment: patch

Comment 8

[Ethan Lin[:ethlin]]

Attachment: Remove container check at ContainerPrepare

As our discussion last week, I confirmed with kats and found that we don't need [1] for ScrollInfo layer. So we could just remove the line to solve this problem. That was added at bug 890279. 


[1] http://dxr.mozilla.org/mozilla-central/source/gfx/layers/composite/ContainerLayerComposite.cpp#271

Comment 9

[Benoit Girard (:BenWa)]

Comment on attachment 8536303 [details] [diff] [review]
Remove container check at ContainerPrepare

Review of attachment 8536303 [details] [diff] [review]:
-----------------------------------------------------------------

r+ since we don't need an more eyes on this.

Comment 10

[Benoit Girard (:BenWa)]

Actually :ethan I told you on IRC that I would land this here but it doesn't belong with this bug. I'd rather we file a separate bug for that patch and land it there to not tangle unrelated patches. I don't think your patch belongs with this original issue.

Comment 11

[Benoit Girard (:BenWa)]

https://tbpl.mozilla.org/?tree=Try&rev=e268877985ff

Comment 12

[Benoit Girard (:BenWa)]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/bca5954a31d2
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/64e9d08ae14b

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Both backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/aae910aa3688 for b2g reftest-3 failures:

https://treeherder.mozilla.org/ui/logviewer.html#?job_id=4696959&repo=mozilla-inbound

Comment 14

[Ethan Lin[:ethlin]]

BenWa, my patch can also solve the ASSERT problem. Here is the tryserver result:
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=ea5bf6ddc0e3

I think I also need to debug the async-scrolling test.

Comment 15

[Benoit Girard (:BenWa)]

Well both of our patch do essentially the same thing so it's probably the same problem. Ethan do you want to look at it or should I?

Comment 16

[Ethan Lin[:ethlin]]

BenWa, I want to look at this problem, so I'll take the bug and commit with your test case.

Comment 17

[Ethan Lin[:ethlin]]

Attachment: v2 - check the visible size before render target

Move the check of visible region to the back of recursive preparation. The try server result:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ee659fba70e7

Comment 18

[Ethan Lin[:ethlin]]

Please land the attachment 8540692 [details] [diff] [review] and attachment 8535141 [details] [diff] [review] to mozilla-central.
Try server result: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ee659fba70e7

Comment 19

[Carsten Book [:Tomcat]]

Hi,

this patch failed to apply:

applying patch
patching file layout/reftests/bugs/reftest.list
Hunk #1 FAILED at 1839
1 out of 1 hunks FAILED -- saving rejects to file layout/reftests/bugs/reftest.list.rej

Could you take a look? Thanks

Comment 20

[Ethan Lin[:ethlin]]

Attachment: Add a reftest. r=jmuizelaar

Regenerate the reftest patch from the latest code.

Comment 21

[Ethan Lin[:ethlin]]

Please land the attachment 8540692 [details] [diff] [review] and attachment 8544030 [details] [diff] [review] to mozilla-central.

Comment 22

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/64988a48dea9
https://hg.mozilla.org/integration/mozilla-inbound/rev/9a84c3fd8d99

Comment 23

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/64988a48dea9
https://hg.mozilla.org/mozilla-central/rev/9a84c3fd8d99

Comment 24

[Benoit Girard (:BenWa)]

Thanks :Ethan!