Last Comment Bug 759653 - BasicLayers.cpp ABORTs in DEBUG mode on B2G
: BasicLayers.cpp ABORTs in DEBUG mode on B2G
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: unspecified
: ARM Gonk (Firefox OS)
: -- normal (vote)
: mozilla16
Assigned To: Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
:
:
Mentors:
Depends on:
Blocks: 743182
  Show dependency treegraph
 
Reported: 2012-05-29 22:28 PDT by Dave Hylands [:dhylands]
Modified: 2012-06-12 03:04 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
gdb backtrace from the ABORT (6.95 KB, text/plain)
2012-05-29 22:33 PDT, Dave Hylands [:dhylands]
no flags Details
Use RGB24 for offscreen surfaces on android-esque platforms (1.11 KB, patch)
2012-06-11 20:01 PDT, Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
mwu.code: review+
Details | Diff | Splinter Review

Description Dave Hylands [:dhylands] 2012-05-29 22:28:14 PDT
All DEBUG builds after the following commit from our mozilla-central git repository:

7fe94a9 Sat May 26 12:38:17 2012 +0800 Kan-Ru Chen Bug 743182 - Use gfxPlatform::OptimalFormatForContent everywhere. r=joedrew

cause the following ABORT to occur in B2G:

###!!! ABORT: Bad! Did we create a buffer twice without painting?: '!mIsNewBuffer', file /home/work/B2G/gecko/gfx/layers/basic/BasicLayers.cpp, line 2531
Comment 1 Dave Hylands [:dhylands] 2012-05-29 22:33:34 PDT
Created attachment 628230 [details]
gdb backtrace from the ABORT
Comment 2 Dave Hylands [:dhylands] 2012-05-29 22:42:51 PDT
And oh yeah - the ABORT is only seen when OMTC is enabled.
Comment 3 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-05-29 22:45:13 PDT
This abort is a very serious logic error: it means our double-buffering code became discombobulated and probably would have not have synchronized front->back buffers correctly.  If that happens we'll get horrendous rendering artifacts.

Very interested to know how we're getting into this state.
Comment 4 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-05-29 22:46:13 PDT
It would be instructive to try re-applying the patch from bug 743182, but with the gfx/layers hunks removed.
Comment 5 Dave Hylands [:dhylands] 2012-05-29 22:49:04 PDT
I also confirmed that a non-debug build of the same commit fixes bug 744238 (so no artifacts seen when sliding up the lock screen when OMTC is enabled).
Comment 6 Dave Hylands [:dhylands] 2012-05-29 23:14:52 PDT
Removing gfx/layers/basic/BasicLayers.cpp and gfx/layers/ipc/ShadowLayers.cpp from the patchset, and adding back in the gfxASurface::FormatFromContent function cause the ABORT to not occur (and the lock screen artifact still shows)
Comment 7 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-05-29 23:19:34 PDT
That is very very odd.
Comment 8 Dave Hylands [:dhylands] 2012-05-30 01:03:58 PDT
In the working code case, the image format that ShadowLayerForwarder::AllocBuffer creates is ImageFormatRGB16_565.

In the case that aborts, the image format that ShadowLayerForwarder::AllocBuffer creates is ImageFormatARGB32.

And if I go back to the version of the code that doesn't ABORT and modify OptimalFormatFor to return ImageFormatARGB32 then I also get the ABORT.
Comment 9 Dave Hylands [:dhylands] 2012-05-30 01:05:24 PDT
And all of this testing was being done on a Samsung Galaxy S II running the ICS variant of gonk.
Comment 10 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-06-11 18:09:19 PDT
dhylands, do you have a patch for the ImageFormat problem?
Comment 11 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-06-11 18:54:34 PDT
Actually, I think this bug only affects sgs2, so it's not a top priority.  But if you already have a patch we should land it.
Comment 12 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-06-11 20:01:35 PDT
Created attachment 632115 [details] [diff] [review]
Use RGB24 for offscreen surfaces on android-esque platforms
Comment 13 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-06-11 20:44:59 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/a9479d148ce5
Comment 14 Graeme McCutcheon [:graememcc] 2012-06-12 03:04:08 PDT
https://hg.mozilla.org/mozilla-central/rev/a9479d148ce5

Note You need to log in before you can comment on or make changes to this bug.