Closed
Bug 960378
Opened 11 years ago
Closed 11 years ago
PROCESS-CRASH | /tests/content/canvas/test/webgl/test_webgl_conformance_test_suite.html | application crashed [@ mozilla::gl::ReadPixelsIntoImageSurface(mozilla::gl::GLContext*, gfxImageSurface*)] on Android 4.0 Debug
Categories
(Core :: Graphics: CanvasWebGL, defect)
Tracking
()
RESOLVED
FIXED
mozilla30
People
(Reporter: gbrown, Assigned: jgilbert)
References
Details
Attachments
(1 file, 2 obsolete files)
7.20 KB,
patch
|
jgilbert
:
review+
|
Details | Diff | Splinter Review |
Android 4.0 *Debug* tests currently only run on Cedar. We are trying to get these tests to pass so that they can be turned on on trunk.
On Cedar, on Android 4.0 Debug, mochitest-gl tests consistently fail with a crash.
https://tbpl.mozilla.org/php/getParsedLog.php?id=33063997&tree=Cedar&full=1#error0
16:34:25 INFO - 6640 INFO TEST-PASS | /tests/content/canvas/test/webgl/test_webgl_conformance_test_suite.html | [conformance/textures/texture-size-cube-maps.html] Test passed - 0, 0, 2, 1 should be red
16:34:25 INFO - 6641 INFO TEST-PASS | /tests/content/canvas/test/webgl/test_webgl_conformance_test_suite.html | [conformance/textures/texture-size-cube-maps.html] Test passed - 0, 1, 2, 1 should be yellow
16:34:25 INFO - 6642 INFO TEST-PASS | /tests/content/canvas/test/webgl/test_webgl_conformance_test_suite.html | [conformance/textures/texture-size-cube-maps.html] Test passed - 0, 0, 2, 1 should be yellow
16:34:25 INFO - 6643 INFO TEST-PASS | /tests/content/canvas/test/webgl/test_webgl_conformance_test_suite.html | [conformance/textures/texture-size-cube-maps.html] Test passed - 0, 1, 2, 1 should be magenta
16:34:25 INFO - INFO | automation.py | Application ran for: 0:08:04.362758
16:34:25 INFO - INFO | zombiecheck | Reading PID log: /tmp/tmphv3Ni5pidlog
16:34:25 INFO - /data/anr/traces.txt not found
16:34:26 INFO - mozcrash INFO | Downloading symbols from: http://ftp.mozilla.org/pub/mozilla.org/mobile/tinderbox-builds/cedar-android-debug/1389761500/fennec-29.0a1.en-US.android-arm.crashreporter-symbols.zip
16:34:41 WARNING - PROCESS-CRASH | /tests/content/canvas/test/webgl/test_webgl_conformance_test_suite.html | application crashed [@ mozilla::gl::ReadPixelsIntoImageSurface(mozilla::gl::GLContext*, gfxImageSurface*)]
16:34:41 INFO - Crash dump filename: /tmp/tmpkgwgqj/1ea9ac52-abf1-ac60-3a8303db-410e6859.dmp
16:34:41 INFO - Operating system: Android
16:34:41 INFO - 0.0.0 Linux 3.2.0+ #2 SMP PREEMPT Thu Nov 29 08:06:57 EST 2012 armv7l pandaboard/pandaboard/pandaboard:4.0.4/IMM76I/5:eng/test-keys
16:34:41 INFO - CPU: arm
16:34:41 INFO - 2 CPUs
16:34:41 INFO - Crash reason: SIGSEGV
16:34:41 INFO - Crash address: 0x0
16:34:41 INFO - Thread 13 (crashed)
16:34:41 INFO - 0 libxul.so!mozilla::gl::ReadPixelsIntoImageSurface(mozilla::gl::GLContext*, gfxImageSurface*) [GLReadTexImageHelper.cpp:9ea06b86b739 : 270 + 0x14]
16:34:41 INFO - r4 = 0x6ac1dd40 r5 = 0x6ccde800 r6 = 0x00000000 r7 = 0x639bf4b4
16:34:41 INFO - r8 = 0x00000010 r9 = 0x00008367 r10 = 0x000080e1 fp = 0x00000004
16:34:41 INFO - sp = 0x5d401c98 lr = 0x61fbc89f pc = 0x61fbce8e
16:34:41 INFO - Found by: given as instruction pointer in context
16:34:41 INFO - 1 libxul.so!mozilla::gl::ReadScreenIntoImageSurface(mozilla::gl::GLContext*, gfxImageSurface*) [GLReadTexImageHelper.cpp:9ea06b86b739 : 479 + 0x7]
16:34:41 INFO - r4 = 0x6ccde800 r5 = 0x6ac1dd40 r6 = 0x00000000 r7 = 0x6866be50
16:34:41 INFO - r8 = 0x00000010 r9 = 0x63c29d0c r10 = 0x636f9742 fp = 0x00088b88
16:34:41 INFO - sp = 0x5d401d70 pc = 0x61fbd173
16:34:41 INFO - Found by: call frame info
16:34:41 INFO - 2 libxul.so!mozilla::gl::SharedSurface_EGLImage::Fence() [SharedSurfaceEGL.cpp:9ea06b86b739 : 184 + 0x7]
16:34:41 INFO - r4 = 0x6a76a390 r5 = 0x6ac1dd40 r6 = 0x00000000 r7 = 0x6866be50
16:34:41 INFO - r8 = 0x00000010 r9 = 0x63c29d0c r10 = 0x636f9742 fp = 0x00088b88
16:34:41 INFO - sp = 0x5d401d90 pc = 0x61fc2a9f
16:34:41 INFO - Found by: call frame info
16:34:41 INFO - 3 libxul.so!mozilla::gfx::SurfaceStream_TripleBuffer::SwapProducer(mozilla::gfx::SurfaceFactory*, mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> const&) [SurfaceStream.cpp:9ea06b86b739 : 425 + 0x7]
16:34:41 INFO - r4 = 0x69732d80 r5 = 0x68868a00 r6 = 0x6a76a398 r7 = 0x6b373218
16:34:41 INFO - r8 = 0x6b373000 r9 = 0x620051d9 r10 = 0x697c0060 fp = 0x00000001
16:34:41 INFO - sp = 0x5d401de8 pc = 0x61fc4427
16:34:41 INFO - Found by: call frame info
16:34:41 INFO - 4 libxul.so!mozilla::gl::GLScreenBuffer::Swap(mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> const&) [GLScreenBuffer.cpp:9ea06b86b739 : 425 + 0x9]
16:34:41 INFO - r4 = 0x6a13db80 r5 = 0x00000000 r6 = 0x6a76a398 r7 = 0x6b373218
16:34:41 INFO - r8 = 0x6b373000 r9 = 0x620051d9 r10 = 0x697c0060 fp = 0x00000001
16:34:41 INFO - sp = 0x5d401e00 pc = 0x61fbeb99
16:34:41 INFO - Found by: call frame info
16:34:41 INFO - 5 libxul.so!mozilla::WebGLContext::PresentScreenBuffer() [WebGLContext.cpp:9ea06b86b739 : 1189 + 0x9]
16:34:41 INFO - r4 = 0x6dbefc00 r5 = 0x00000000 r6 = 0x6dbefc50 r7 = 0x6b373218
16:34:41 INFO - r8 = 0x6b373000 r9 = 0x620051d9 r10 = 0x697c0060 fp = 0x00000001
16:34:41 INFO - sp = 0x5d401e90 pc = 0x625c7e87
16:34:41 INFO - Found by: call frame info
16:34:45 INFO - 01-15 16:33:58.226 I/GeckoDump( 2227): 6641 INFO TEST-PASS | /tests/content/canvas/test/webgl/test_webgl_conformance_test_suite.html | [conformance/textures/texture-size-cube-maps.html] Test passed - 0, 1, 2, 1 should be yellow
16:34:45 INFO - 01-15 16:33:58.234 I/GeckoDump( 2227): 6642 INFO TEST-PASS | /tests/content/canvas/test/webgl/test_webgl_conformance_test_suite.html | [conformance/textures/texture-size-cube-maps.html] Test passed - 0, 0, 2, 1 should be yellow
16:34:45 INFO - 01-15 16:33:58.234 I/GeckoDump( 2227): 6643 INFO TEST-PASS | /tests/content/canvas/test/webgl/test_webgl_conformance_test_suite.html | [conformance/textures/texture-size-cube-maps.html] Test passed - 0, 1, 2, 1 should be magenta
16:34:45 INFO - 01-15 16:33:58.304 F/MOZ_Assert( 2227): Assertion failure: dest->Stride() == dest->Width() * destPixelSize, at ../../../gfx/gl/GLReadTexImageHelper.cpp:272
16:34:45 INFO - 01-15 16:33:59.382 F/libc ( 2227): Fatal signal 11 (SIGSEGV) at 0x00000000 (code=1)
16:34:45 INFO - 01-15 16:33:59.812 I/DEBUG ( 1290): *** *** *** *** *** *** *** *** *** *** *** *** *** *** *** ***
16:34:45 INFO - 01-15 16:33:59.812 I/DEBUG ( 1290): Build fingerprint: 'pandaboard/pandaboard/pandaboard:4.0.4/IMM76I/5:eng/test-keys'
16:34:45 INFO - 01-15 16:33:59.812 I/DEBUG ( 1290): pid: 2227, tid: 2247 >>> org.mozilla.fennec <<<
16:34:45 INFO - 01-15 16:33:59.812 I/DEBUG ( 1290): signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 00000000
16:34:45 INFO - 01-15 16:33:59.812 I/DEBUG ( 1290): r0 00000081 r1 63422360 r2 0000007b r3 00000000
16:34:45 INFO - 01-15 16:33:59.812 I/DEBUG ( 1290): r4 6ac1dd40 r5 6ccde800 r6 00000000 r7 639bf4b4
16:34:45 INFO - 01-15 16:33:59.812 I/DEBUG ( 1290): r8 00000010 r9 00008367 10 000080e1 fp 00000004
16:34:45 INFO - 01-15 16:33:59.812 I/DEBUG ( 1290): ip 400b4130 sp 5d401c98 lr 61fbc89f pc 61fbce8e cpsr 28000130
![]() |
Reporter | |
Comment 1•11 years ago
|
||
More examples:
https://tbpl.mozilla.org/php/getParsedLog.php?id=33063997&tree=Cedar&full=1
https://tbpl.mozilla.org/php/getParsedLog.php?id=33064019&tree=Cedar&full=1
https://tbpl.mozilla.org/php/getParsedLog.php?id=32828669&tree=Cedar&full=1
https://tbpl.mozilla.org/php/getParsedLog.php?id=32789284&tree=Cedar&full=1
Comment 2•11 years ago
|
||
CJ, we deemed this high priority to look at soon - can you get somebody to assign to this?
Flags: needinfo?(cku)
OS: Mac OS X → Android
(In reply to Milan Sreckovic [:milan] from comment #2)
> CJ, we deemed this high priority to look at soon - can you get somebody to
> assign to this?
Milan, we will have a Chinese New Year vacation between Jan 30th and Feb 4th. Which means we can start working on this fix after Feb 4th. If you think it's ok, please assign to me, depend on internal resource, I will find correct one to work on this.
Flags: needinfo?(cku)
Comment 5•11 years ago
|
||
Sometimes we'll create SourceSurface with stride 16 ( http://dxr.mozilla.org/mozilla-central/source/gfx/2d/SourceSurfaceRawData.cpp#32 ), and this kind of SourceSurface will hit assertion that causes crash. But I think although stride doesn't match to width multiply destPixelSize, We still can read it through tempSurf.
Attachment #8375254 -
Flags: review?(jgilbert)
Assignee | ||
Comment 6•11 years ago
|
||
Comment on attachment 8375254 [details] [diff] [review]
remove-assert
Review of attachment 8375254 [details] [diff] [review]:
-----------------------------------------------------------------
The code below relies on this assert.
Attachment #8375254 -
Flags: review?(jgilbert) → review-
Assignee | ||
Comment 7•11 years ago
|
||
This should allow for reading into non-tightly-packed surfaces.
Morris: Does this fix the issue?
Attachment #8375254 -
Attachment is obsolete: true
Attachment #8375282 -
Flags: review?(bjacob)
Attachment #8375282 -
Flags: feedback?(mtseng)
Assignee | ||
Comment 8•11 years ago
|
||
Comment on attachment 8375282 [details] [diff] [review]
read-overstrided-surfs
Review of attachment 8375282 [details] [diff] [review]:
-----------------------------------------------------------------
::: gfx/gl/GLReadTexImageHelper.cpp
@@ +235,5 @@
> +static int
> +CalcStride(int width, int pixelSize, int alignment)
> +{
> + MOZ_ASSERT(alignment);
> +
Oops, more WS.
Comment 9•11 years ago
|
||
Comment on attachment 8375282 [details] [diff] [review]
read-overstrided-surfs
Review of attachment 8375282 [details] [diff] [review]:
-----------------------------------------------------------------
This patch works for me.
Attachment #8375282 -
Flags: feedback?(mtseng) → feedback+
Comment 10•11 years ago
|
||
Comment on attachment 8375282 [details] [diff] [review]
read-overstrided-surfs
Review of attachment 8375282 [details] [diff] [review]:
-----------------------------------------------------------------
::: gfx/gl/GLReadTexImageHelper.cpp
@@ +240,5 @@
> + int stride = width * pixelSize;
> + if (stride % alignment) { // Extra at the end of the line?
> + int alignmentCount = stride / alignment;
> + stride = (alignmentCount+1) * alignment;
> + }
This function is not needed, if the next comment is correct.
If you decide to keep it, you could at least making it much faster by asserting that alignment is a POT and then replacing expensive / and % by bit ops.
@@ +248,5 @@
> +static int
> +GuessAlignment(int width, int pixelSize, int stride)
> +{
> + int alignment = 8; // Max GLES allows.
> + while (CalcStride(width, pixelSize, alignment) != stride) {
This function could be written more simply as:
int padding = stride - width * pixelSize;
return 1
+ 1 * ((padding % 2) == 0)
+ 2 * ((padding % 4) == 0)
+ 4 * ((padding % 8) == 0);
The % here are cheap because the RHS is POT.
Right?
(you'd probably want to assert that padding >= 0, too).
Attachment #8375282 -
Flags: review?(bjacob) → review+
Comment 11•11 years ago
|
||
... or if you prefer a straightforward implementation because this is not so performance sensitive:
int padding = stride - width * pixelSize;
return ((padding % 8) == 0) ? 8
: ((padding % 4) == 0) ? 4
: ((padding % 2) == 0) ? 2
: 1;
Comment 12•11 years ago
|
||
... in fact the latter is probably uniformly better, because compilers are smart enough, and this is definitely not that performance sensitive anyway.
Comment 13•11 years ago
|
||
Oh... nevermind my above comments, I was totally wrong :-)
Go with what you have and maybe just consider the above-mentioned optimization of assuming that alignment is POT (which it is) to avoid expensive arithmetic.
Assignee | ||
Comment 14•11 years ago
|
||
I would rather keep this obvious and 'expensive', because any time here will be dwarfed by the time the ReadPixels takes. This call should only happen about once per frame, so we'd only incur this cost on the order of a hundred times a second.
Assignee | ||
Comment 16•11 years ago
|
||
r=bjacob
Fixed whitespace.
Assignee: mtseng → jgilbert
Attachment #8375282 -
Attachment is obsolete: true
Attachment #8375870 -
Flags: review+
Assignee | ||
Comment 17•11 years ago
|
||
Comment 18•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in
before you can comment on or make changes to this bug.
Description
•