Closed Bug 951241 Opened 9 years ago Closed 9 years ago

[B2G][Gallery][Image Edit] Image preview in edit mode shows orange and blue horizontal lines across middle

Categories

(Core :: Graphics, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

()

RESOLVED FIXED
1.3 C2/1.4 S2(17jan)
blocking-b2g 1.3+
Tracking Status
b2g-v1.3 --- fixed

People

(Reporter: bzumwalt, Assigned: johnhu)

Details

(Keywords: regression)

Attachments

(5 files)

Attached image Screenshot 1
Description:
When editing an image in the Gallery taken with the Camera app, the image preview shows an orange or blue horizontal line across the middle of the picture. This issue does not occur in image files from external sources or screenshots. It also does not persist in the final edited image if saved.


Repro Steps:
1) Updated Buri to Build ID: 20131217004001
2) Open Camera app and tap camera icon to take picture
3) Press Home button to return to Home Screen
4) Open Gallery app
5) Tap on picture taken in step 2
6) Select edit icon

Actual:
Orange or blue horizontal line appears on image preview in Gallery's edit mode.

Expected:
Image preview in edit mode displays no visual abnormalities.

Environmental Variables
Device: Buri v 1.3 Mozilla RIL
Build ID: 20131217004001
Gecko: http://hg.mozilla.org/releases/mozilla-aurora/rev/1f7db4cc788e
Gaia: dca0a3dcf062ce3e422a9c56d141c14543c816fb
Platform Version: 28.0a2
Firmware Version: V1.2_US_20131115


Notes:
Repro frequency: 3/3, 100%
See attached: screenshots
Attached image Screenshot 2
Does this reproduce on 1.2?
Keywords: qawanted
Environment:
--------------------------------------------
Gaia      724d36716bcbbc5cee1af18b94cc328c289d0ccc
Gecko     http://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/931864adcf68
BuildID   20131223004001
Version   26.0


Follow these steps to reproduce:
--------------------------------------------
1. Goto Camera and take a picture
2. Press Home button to return to Home Screen
3. Open Gallery app
4. Tap on picture taken in step 1
5. Select edit icon, and choose "cut"
6. Select "3:2", and then save the picture
7. Goto Gallery
8. Tap on the picture saved in step 6
Keywords: qawanted
Does this reproduce on 1.1?
Keywords: qawanted
This does not reproduce on latest 1.1

Device: Buri v1.1 Mozilla RIL
BuildID: 20140102041202
Gaia: 6ff3a607f873320d00cb036fa76117f6fadd010f
Gecko: bdac595a4e46
Version: 18.0
Firmware Version: V1.2_20131115
QA Contact: astole
Keywords: qawanted
This is bad. This makes the gallery app look like a broken image viewer, as we're showing a broken color line across the image.

Too bad this was caught after close of 1.2 - this wouldn't be acceptable to ship with. We really need to get this fixed for 1.3.
blocking-b2g: --- → 1.3?
Also happening on Buri 1.4

Gaia=711d8bf4efde6b1aaaf1f61bd422312c17435810
Gecko=http://hg.mozilla.org/mozilla-central/rev/cf80c0d4f46e
Buri
BuildID=20140105040201
Version=29.0a1
ro.build.version.incremental=eng.archermind.20131114.105818
ro.build.date=Thu=Nov=14=10:58:33=CST=2013
agree with jsmith, looks pretty bad UI experience...marking this a blocker
John/djf - can you guys check whats going on
blocking-b2g: 1.3? → 1.3+
Flags: needinfo?(johu)
Flags: needinfo?(dflanagan)
Assignee: nobody → johu
Flags: needinfo?(dflanagan)
This bug is related to graphic backend. I had made a test case app which is based on template app. This test app does:
1. a 2MP image into canvas
2. use toBlob to create a jpeg blob
3. use img element to show this jpeg blob

In step 1, I use an image captured by Buri camera which is 1536x2048 and try to scale it into a canvas 228x305.

The followings are my investigations:

1. When we change the height of canvas of step 1 from 305 to 306 or 304, this issue disappears. It is only related to height. When we change the width, it doesn't make any effect.
2. If we just show the canvas to screen without step 2/3, we don't have this issue.
3. After discussion with graphics team, we found it is related to graphic backend. When we switch skia to cario, this issue disappears:
     pref("gfx.canvas.azure.backends", "skia"); // change to "cario"
4. When we turned off the accelerated to false, this issue disappear:
     pref("gfx.canvas.azure.accelerated", true); // change to false

The regression window is still important to find out what's going on since the switch of skia and cario had happened long time ago, at September in my memory.

I will cc graphics team in to follow up this bug and unassign myself.
Flags: needinfo?(johu)
Peter,

I don't know who is the correct person to handle this bug from graphics team. Please help us to find someone to follow this bug.
Assignee: johu → nobody
Flags: needinfo?(pchang)
Component: Gaia::Gallery → Graphics
Product: Firefox OS → Core
(In reply to John Hu [:johnhu] from comment #10)
> Peter,
> 
> I don't know who is the correct person to handle this bug from graphics
> team. Please help us to find someone to follow this bug.

Switching needinfo to Milan - he could help find the right person to look into this.
Flags: needinfo?(pchang) → needinfo?(milan)
James, can you take a look?
Assignee: nobody → snorp
Flags: needinfo?(milan)
Flags: needinfo?(snorp)
Yup, looking now
Flags: needinfo?(snorp)
(In reply to Milan Sreckovic [:milan] from comment #12)
> James, can you take a look?
Sorry for late response, I'm available to check it now.

James, let me know if you are busy working on other issues.
It's very late/early in Taipei, but James is having trouble with the up to date build, so if you can take a look at this in parallel, it would be great.  Just comment in the bug when you discover things.
I can't reproduce with master on a hamachi device (that's the same as buri?)
Yes, same device - there are enough differences between master and 1.3 that we need to check on 1.3 explicitly...
Working fine for me on 1.3/aurora on hamachi as well...
Adding qawanted for a retest on the latest 1.3.
Keywords: qawanted
This issue reproduced for me on the 01/09/14 1.3 build

Gaia   22bc6be5b76cdc6d4e9667ff070979041a20ce2f
SourceStamp 2c8f8683bd0d
BuildID 20140109004002
Version 28.0a2
Keywords: qawanted
I could reproduce this issue when skiaGL enabled on my leo device(master branch).
And I couldn't reproduce it when skiaGL disabled.
The following are detail callstacks from attachment 8356489 [details].

It didn't go through drawTiledBitmap(), still need to figure out why the blue line stayed at the middle of photo.

#0  GrInOrderDrawBuffer::drawRect (this=0x435af000, rect=..., matrix=<value optimized out>, localRect=0xbee209f4, localMatrix=0x0) at /Users/peter/code/b2g_gecko/gfx/skia/src/gpu/GrInOrderDrawBuffer.cpp:105
#1  0x4154ecec in GrContext::drawRectToRect (this=<value optimized out>, paint=<value optimized out>, dstRect=..., localRect=..., dstMatrix=0xbee20c88, localMatrix=0x0) at /Users/peter/code/b2g_gecko/gfx/skia/src/gpu/GrContext.cpp:899
#2  0x4155b68c in SkGpuDevice::internalDrawBitmap (this=0x435f70a0, bitmap=..., srcRect=<value optimized out>, m=<value optimized out>, params=..., grPaint=0xbee20a60) at /Users/peter/code/b2g_gecko/gfx/skia/src/gpu/SkGpuDevice.cpp:1442
#3  0x4155bc60 in SkGpuDevice::drawBitmapCommon (this=0x435f70a0, draw=..., bitmap=..., srcRectPtr=0xbee20cb0, m=..., paint=...) at /Users/peter/code/b2g_gecko/gfx/skia/src/gpu/SkGpuDevice.cpp:1241
#4  0x4155bd44 in SkGpuDevice::drawBitmapRect (this=0x435f70a0, draw=..., bitmap=..., src=0xbee20e8c, dst=..., paint=...) at /Users/peter/code/b2g_gecko/gfx/skia/src/gpu/SkGpuDevice.cpp:1580
#5  0x4151e9d6 in SkCanvas::internalDrawBitmapRect (this=0x435c79c0, bitmap=..., src=0xbee20e8c, dst=..., paint=0xbee20e38) at /Users/peter/code/b2g_gecko/gfx/skia/src/core/SkCanvas.cpp:1674
#6  0x4151ec5a in SkCanvas::drawBitmapRectToRect (this=<value optimized out>, bitmap=<value optimized out>, src=0x0, dst=..., paint=0xbee20e38) at /Users/peter/code/b2g_gecko/gfx/skia/src/core/SkCanvas.cpp:1683
#7  0x41490da0 in mozilla::gfx::DrawTargetSkia::DrawSurface (this=0x440639a0, aSurface=0x43588b50, aDest=..., aSource=..., aSurfOptions=..., aOptions=...) at /Users/peter/code/b2g_gecko/gfx/2d/DrawTargetSkia.cpp:375
#8  0x40e89464 in mozilla::dom::CanvasRenderingContext2D::DrawImage (this=0x403a2400, image=..., sx=0, sy=0, sw=1536, sh=2048, dx=0, dy=0, dw=228, dh=305, optional_argc=6
So the actual preview in edit mode is WebGL, right? The 2D canvas is only for scaling? I would expect the fix for bug 939962 to fix this as well.
(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) from comment #23)
> So the actual preview in edit mode is WebGL, right? The 2D canvas is only
> for scaling? I would expect the fix for bug 939962 to fix this as well.

I think the preview under edit mode is using Canvas2D. And the WebGL is used for post-processing. Agree that we could leverage bug 939962 to fix this problem.

John, how do you think?
Flags: needinfo?(johu)
(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) from comment #23)
> So the actual preview in edit mode is WebGL, right? The 2D canvas is only
> for scaling? I would expect the fix for bug 939962 to fix this as well.

James, Do you mean that we just use software version instead of gpu version 2d canvas for gallery editor preview?
But we still don't know why gpu version 2d canvas has the blue horizontal line.
(In reply to Jerry Shih[:jerry] from comment #25)
> (In reply to James Willcox (:snorp) (jwillcox@mozilla.com) from comment #23)
> > So the actual preview in edit mode is WebGL, right? The 2D canvas is only
> > for scaling? I would expect the fix for bug 939962 to fix this as well.
> 
> James, Do you mean that we just use software version instead of gpu version
> 2d canvas for gallery editor preview?

Right.

> But we still don't know why gpu version 2d canvas has the blue horizontal
> line.

Yeah, it's probably something in the Skia bitmap tiling code. In the near term it's going to be best just to fallback to software, because we don't have anyone who knows that code.
(In reply to peter chang[:pchang][:peter] from comment #24)
> I think the preview under edit mode is using Canvas2D. And the WebGL is used
> for post-processing. Agree that we could leverage bug 939962 to fix this
> problem.
> 
> John, how do you think?

Yes, it is true. I had applied the same change to the generateNewPreview function[1] and the blue line was gone. But I don't know why about this....

What's the magic happened here?

[1] https://github.com/mozilla-b2g/gaia/blob/e771f17cb11b87999eb632f795690fb4faa9a68d/apps/gallery/js/ImageEditor.js#L550
Flags: needinfo?(johu)
According to James's comment at comment 23 and comment 26, there is a bug in canvas 2d of gpu version. We can use { willReadFrequently: true } to prevent that. So, I play the same patch of bug 939962 to generateNewPreview function.
Attachment #8359025 - Flags: review?(dflanagan)
If we have a workaround for this issue, by switching to software, let's make sure we put that fix into 1.3, but have a bug open against a later release to figure out the SkiaGL issue.  So, there should be on bug, with the Gaia workaround, and "fixed" soon, and one bug against SkiaGL, left open until we figure out what the issue is.  I don't have a preference whether this particular bug ends up being the one that gets fixed with these changes or ends up being the SkiaGL one.
(In reply to Milan Sreckovic [:milan] from comment #29)
> If we have a workaround for this issue, by switching to software, let's make
> sure we put that fix into 1.3, but have a bug open against a later release
> to figure out the SkiaGL issue.  So, there should be on bug, with the Gaia
> workaround, and "fixed" soon, and one bug against SkiaGL, left open until we
> figure out what the issue is.  I don't have a preference whether this
> particular bug ends up being the one that gets fixed with these changes or
> ends up being the SkiaGL one.

I think we should keep this bug focused on the blocking factor for 1.3 here. If there's a real fix still needed here, then we should open a followup to target that.
Comment on attachment 8359025 [details] [review]
apply the patch of bug 939962 to preview generation

Looks good to me.
Attachment #8359025 - Flags: review?(dflanagan) → review+
James,

Will you assign this bug back to John so he can land his workaround and open a new bug for the underlying skia issue?
Flags: needinfo?(snorp)
Assignee: snorp → johu
Flags: needinfo?(snorp)
Follow up Skia bug is 960276
Thanks James,

I will add this bug number as the comment.
merged to master:
https://github.com/mozilla-b2g/gaia/commit/c99867fe9ba161535e764ba6582ea13a7c25e324
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.3 C2/1.4 S2(17jan)
John, can you please assist with the uplift to v1.3? :)
Flags: needinfo?(jhford)
[v1.3 66977a9] Merge pull request #15235 from huchengtw-moz/gallery/Bug_951241-blue-line
Flags: needinfo?(jhford)
Issue is occurring in latest 1.3 and Master 1.4, but only when viewing certain Facebook contacts in Contacts app. When viewing Facebook contact image a horizontal blue line is visible on picture. Should this new development reopen this bug, or should a new bug be filed?

Environmental Variables:
Device: Buri v1.4 Master Mozilla RIL
BuildID: 20140128040224
Gaia: 6586d2b8b43c6997be5cf5895bbdf3dd20490725
Gecko: 4da3e21a0e5f
Version: 29.0a1
Firmware Version: V1.2-device.cfg


Environmental Variables:
Device: Buri v1.3 Mozilla RIL
BuildID: 20140128004000
Gaia: 3c5119507a985c59cfc264ed23821679b138486d
Gecko: 4936f09590a0
Version: 29.0a1
Firmware Version: V1.2-device.cfg
While certainly related, the steps to reproduce is different enough that I'd file a separate bug.  Jason, what do you think?
IMHO, the main cause of this bug is about Skia which is filed as bug 960276.

This patch is only a "workaround patch" for gallery app. If this is really annoying, we should file a bug to track all apps who uses canvas to resize and crop a image to apply the workaround or put our efforts to bug 960276.
Brogan - Can you file a separate bug for comment 38? We'll look into implementing the workaround then in the contacts app as well.
Flags: needinfo?(bzumwalt)
Filed new bug: https://bugzilla.mozilla.org/show_bug.cgi?id=965394
Flags: needinfo?(bzumwalt)
You need to log in before you can comment on or make changes to this bug.