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

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Brogan Zumwalt [Inactive]]

Attachment: 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

Comment 1

[Brogan Zumwalt [Inactive]]

Attachment: Screenshot 2

Comment 2

[Jason Smith [:jsmith]]

Does this reproduce on 1.2?

Comment 3

[Bingqing Li]

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

Comment 4

[Jason Smith [:jsmith]]

Does this reproduce on 1.1?

Comment 5

[Andrew Stole [:astole]]

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

Comment 6

[Jason Smith [:jsmith]]

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.

Comment 7

[Eric Chang [:ericcc] [:echang]]

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

Comment 8

[Hema Koka [:hema]]

agree with jsmith, looks pretty bad UI experience...marking this a blocker
John/djf - can you guys check whats going on

Comment 9

[John Hu [:johnhu][:johu][:醬糊小弟]]

Attachment: test case based on template app.

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.

Comment 10

[John Hu [:johnhu][: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.

Comment 11

[Jason Smith [:jsmith]]

(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.

Comment 12

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

James, can you take a look?

Comment 13

[James Willcox (:snorp) ([e-mail]) (he/him)]

Yup, looking now

Comment 14

[Peter Chang[:pchang]]

(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.

Comment 15

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

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.

Comment 16

[James Willcox (:snorp) ([e-mail]) (he/him)]

I can't reproduce with master on a hamachi device (that's the same as buri?)

Comment 17

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

Yes, same device - there are enough differences between master and 1.3 that we need to check on 1.3 explicitly...

Comment 18

[James Willcox (:snorp) ([e-mail]) (he/him)]

Working fine for me on 1.3/aurora on hamachi as well...

Comment 19

[Jason Smith [:jsmith]]

Adding qawanted for a retest on the latest 1.3.

Comment 20

[Matthew Vaughan]

This issue reproduced for me on the 01/09/14 1.3 build

Gaia   22bc6be5b76cdc6d4e9667ff070979041a20ce2f
SourceStamp 2c8f8683bd0d
BuildID 20140109004002
Version 28.0a2

Comment 21

[Peter Chang[:pchang]]

I could reproduce this issue when skiaGL enabled on my leo device(master branch).
And I couldn't reproduce it when skiaGL disabled.

Comment 22

[Peter Chang[:pchang]]

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

Comment 23

[James Willcox (:snorp) ([e-mail]) (he/him)]

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.

Comment 24

[Peter Chang[:pchang]]

(In reply to James Willcox (:snorp) ([e-mail]) 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?

Comment 25

[Jerry Shih[:jerry] (UTC+8) (inactive)]

(In reply to James Willcox (:snorp) ([e-mail]) 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.

Comment 26

[James Willcox (:snorp) ([e-mail]) (he/him)]

(In reply to Jerry Shih[:jerry] from comment #25)
> (In reply to James Willcox (:snorp) ([e-mail]) 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.

Comment 27

[John Hu [:johnhu][:johu][:醬糊小弟]]

(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

Comment 28

[John Hu [:johnhu][:johu][:醬糊小弟]]

Attachment: apply the patch of bug 939962 to preview generation

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.

Comment 29

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

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.

Comment 30

[Jason Smith [:jsmith]]

(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 31

[David Flanagan [:djf]]

Comment on attachment 8359025 [details] [review]
apply the patch of bug 939962 to preview generation

Looks good to me.

Comment 32

[David Flanagan [:djf]]

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?

Comment 33

[James Willcox (:snorp) ([e-mail]) (he/him)]

Follow up Skia bug is 960276

Comment 34

[John Hu [:johnhu][:johu][:醬糊小弟]]

Thanks James,

I will add this bug number as the comment.

Comment 35

[John Hu [:johnhu][:johu][:醬糊小弟]]

merged to master:
https://github.com/mozilla-b2g/gaia/commit/c99867fe9ba161535e764ba6582ea13a7c25e324

Comment 36

[Ryan VanderMeulen [:RyanVM]]

John, can you please assist with the uplift to v1.3? :)

Comment 37

[John Ford [:jhford] CET/CEST Berlin Time]

[v1.3 66977a9] Merge pull request #15235 from huchengtw-moz/gallery/Bug_951241-blue-line

Comment 38

[Brogan Zumwalt [Inactive]]

Attachment: Screenshot - ThreeExamples

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

Comment 39

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

While certainly related, the steps to reproduce is different enough that I'd file a separate bug.  Jason, what do you think?

Comment 40

[John Hu [:johnhu][:johu][:醬糊小弟]]

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.

Comment 41

[Jason Smith [:jsmith]]

Brogan - Can you file a separate bug for comment 38? We'll look into implementing the workaround then in the contacts app as well.

Comment 42

[Brogan Zumwalt [Inactive]]

Filed new bug: https://bugzilla.mozilla.org/show_bug.cgi?id=965394