Closed
Bug 1181290
Opened 9 years ago
Closed 9 years ago
[Aries][Gallery] Images draw in segments from right to left displaying placeholder color briefly and flashing
Categories
(Firefox OS Graveyard :: Gaia::Gallery, defect, P3)
Tracking
(blocking-b2g:2.5+, b2g-master verified)
People
(Reporter: onelson, Assigned: pdahiya)
References
Details
(Whiteboard: [2.5-Daily-Testing], [Spark])
Attachments
(2 files)
Description:
When a user is accessing images in large view from their gallery (by tapping), they will observe that the images have a splintered load-in and display a placeholder color while the image settles. This does not occur when the user is swiping between images, only when they are accessing from thumbnail view of gallery's contents.
The load itself appears similar to the swipe behavior observed in the Camera app preview pictures: bug 1178510
Repro Steps:
1) Update an Aries to 20150707033152
2) Open the Camera app
3) Take 5 pictures
4) Return to homescreen, open gallery
5) Tap a picture to view large
6) Observe draw
Actual:
Draw appears from right to left and flashes; placeholder color is observed on segments when flashing
Expected:
Image draws at once; no flashing
Environmental Variables:
----------------------
Device: Aries 2.5
Build ID: 20150707033152
Gaia: e6506d68e01489b57616f8b74e8773f938ce62b3
Gecko: e271ef4c49ae
Gonk: 2916e2368074b5383c80bf5a0fba3fc83ba310bd
Version: 42.0a1 (2.5)
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:42.0) Gecko/42.0 Firefox/42.0
**********************
Issue DOES NOT REPRO on master for flame devices
Device: Flame 2.5
BuildID: 20150707010204
Gaia: e6506d68e01489b57616f8b74e8773f938ce62b3
Gecko: e7e69cc8c07b
Gonk: a4f6f31d1fe213ac935ca8ede7d05e47324101a4
Version: 42.0a1 (2.5)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:42.0) Gecko/42.0 Firefox/42.0
----------------------
Repro frequency: 5/5
See attached:
video- https://youtu.be/KUgFM5mDwDI
logcat
Reporter | ||
Updated•9 years ago
|
Flags: needinfo?(pbylenga)
Comment 1•9 years ago
|
||
NI Component owner to take a look, is this a dupe of bug 1178510?
blocking-b2g: --- → 2.5?
QA Whiteboard: [QAnalyst-Triage+]
Flags: needinfo?(pbylenga) → needinfo?(npark)
Comment 2•9 years ago
|
||
hmm, I think it might be a graphics issue. kats, could you take a look at this one?
And I think bug 1178510 is definitely a dupe of this one.
Flags: needinfo?(npark) → needinfo?(bugmail.mozilla)
Comment 4•9 years ago
|
||
It might be a graphics issue but my preliminary investigation didn't point to any of the usual suspects. The image isn't being animated in; it's just plopped into place and seems to draw in chunks. Seth, could this be a result of decode-on-draw or something?
Flags: needinfo?(bugmail.mozilla) → needinfo?(seth)
Comment 5•9 years ago
|
||
My best guess is that this is just a result of the image not being decoded already. Nothing to do with decode-on-draw, necessarily. We won't keep non-visible images in memory, regardless of decode-on-draw.
Probably this is a result of bug 1163215, which made us use <img> elements instead of CSS background images for the Gallery app. <img> elements draw in progressively instead of "popping" in once they're completely decoded, like CSS background images do.
We switched to using <img> elements because currently memory used by CSS background images cannot be freed when the images scroll out of view, resulting in severe memory-related issues in the Gallery app. I want to fix that issue at the platform level for 2.5, but it will take a while; it's a pretty massive change.
I'm going to speculatively add blocking bugs.
Comment 8•9 years ago
|
||
Seth: could this be happening because we're still using the #-moz-sample-size media fragment and not just relying on gecko to downsample the image for us?
If so, then fixing 1140619 might fix this.
Otherwise, the issue is that the camera app takes ridiculously large 20mp images, but does not include a screen-sized EXIF preview. In these circumstances, we have to decode the full image, and that involves processing megabytes of data. So of course it is slow.
I'd suggest that the best solution here would be to change the default photo resolution in the camera app so that it takes smaller photos by default. Andrew: is this something you're willing to consider?
If we have to, we could revert the gallery back to the old days when we created preview images for large photos that did not have suitable EXIF previews. Given that gecko can downsample on decode, we shouldn't have to do that, but I suppose we could put it back in if needed.
More likely, though, would be to play some kind of CSS trick and fade the photo in to try to disguise the image decoding delay.
Flags: needinfo?(seth)
Flags: needinfo?(aosmond)
Comment 9•9 years ago
|
||
Clearing the regression keyword. I'm pretty sure that no code has actually broken here: this bug appears because we've quadrupled the size of the photos we take.
Keywords: regression
Updated•9 years ago
|
Priority: -- → P3
Updated•9 years ago
|
Comment 10•9 years ago
|
||
I have been hoping that this would be resolved (or mitigated sufficiently) by the work in bug 1206206. But it now looks like that will not be landing in 2.5.
I believe that there is not a bug here, but that is just how much time it takes to decode and display the giant images from Aries. The camera on that device does not produce usable EXIF preview images, so we have to decode the full image.
In order to change this, we'd have to modify the Gallery metadata parser to create a screen-sized preview image for each photo the user takes. We used to do this in the past, and I think we still do it for large PNG images. So the change won't be too difficult. It is a tradeoff: it will make scanning slower, and take up more space on the user's sd card, but viewing photos in the gallery will be more responsive.
Assignee: nobody → dflanagan
Assignee | ||
Comment 11•9 years ago
|
||
As discussed over IRC, assigning myself to look into this issue.
Assignee | ||
Updated•9 years ago
|
Assignee: dflanagan → pdahiya
Comment 12•9 years ago
|
||
Assignee | ||
Comment 13•9 years ago
|
||
Comment on attachment 8669959 [details]
[gaia] punamdahiya:Bug1181290 > mozilla-b2g:master
Hi David
Attaching patch that creates preview for large jpeg files. I have kept the file size check to 2.5 MB as down-sampling jpegs > 2.5 MB for preview shows flash when viewed in fullscreen. Please review. Thanks!
Attachment #8669959 -
Flags: review?(dflanagan)
Comment 14•9 years ago
|
||
Comment on attachment 8669959 [details]
[gaia] punamdahiya:Bug1181290 > mozilla-b2g:master
Please base the size test on the actual image size, since we already know it. The file size is usually a good proxy for jpeg image size, but it can vary a lot depending on the subject of a photo. We've got the actual pixel size of the image already, so let's use that.
Attachment #8669959 -
Flags: review?(dflanagan) → review-
Assignee | ||
Comment 15•9 years ago
|
||
(In reply to David Flanagan [:djf] from comment #14)
> Comment on attachment 8669959 [details]
> [gaia] punamdahiya:Bug1181290 > mozilla-b2g:master
>
> Please base the size test on the actual image size, since we already know
> it. The file size is usually a good proxy for jpeg image size, but it can
> vary a lot depending on the subject of a photo. We've got the actual pixel
> size of the image already, so let's use that.
makes sense, updated patch to use image size in pixels. Please review.
Assignee | ||
Updated•9 years ago
|
Attachment #8669959 -
Flags: review- → review?(dflanagan)
Comment 16•9 years ago
|
||
Comment on attachment 8669959 [details]
[gaia] punamdahiya:Bug1181290 > mozilla-b2g:master
Looks good to me
Attachment #8669959 -
Flags: review?(dflanagan) → review+
Assignee | ||
Comment 17•9 years ago
|
||
While testing this fix for camera Bug 1188286, here are my findings
a) Camera preview doesn't use preview images stored in sdcard. Reason being metadataparser is kicked in when gallery app is opened. Camera preview uses media_frame to check for valid preview and if no valid preview is available (Line 272) uses downsampled image as preview.
https://github.com/mozilla-b2g/gaia/blob/master/shared/js/media/media_frame.js#L231
https://github.com/mozilla-b2g/gaia/blob/master/shared/js/media/media_frame.js#L272
So flashing is still seen while viewing image in camera preview.
b) On debugging, we do have a valid preview generated by Aries camera however it's not bigenough and has width and height of 160 x120 because of which media_frame fails to use it and executes flow inside nopreview();
https://github.com/mozilla-b2g/gaia/blob/master/shared/js/media/media_frame.js#L220
Andrew, David: Is there any way to customize this preview size so that camera app generate valid sizes preview.
I see we have config inside camera app, is this something that can be configured by device.
https://github.com/mozilla-b2g/gaia/blob/master/apps/camera/js/config/config.js#L14
Thanks!
Flags: needinfo?(dflanagan)
Assignee | ||
Updated•9 years ago
|
Flags: needinfo?(aosmond)
Assignee | ||
Updated•9 years ago
|
Flags: needinfo?(aosmond)
Assignee | ||
Comment 18•9 years ago
|
||
Synched up with Andrew on IRC and 160x 120 is the largest size preview Aries can produce. Landing attached patch that fixes this issue for gallery and moving the discussion to resolve this issue for camera in bug 1188286.
Flags: needinfo?(aosmond)
Assignee | ||
Updated•9 years ago
|
Flags: needinfo?(dflanagan)
Assignee | ||
Comment 19•9 years ago
|
||
Patch landed on master
https://github.com/mozilla-b2g/gaia/commit/cbf099c0858cc03172df87977c17f26f12e69fe2
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Comment 20•9 years ago
|
||
According to the STR of Comment 0, this bug has been verified as pass on latest Aries KK v2.5.
Actual results: Image draws at once and no flashing.
See attachment: verified_Aries KK v2.5.3gp
Reproduce rate: 0/6
Device: Aries KK v2.5 (Pass)
Build ID 20151010002512
Gaia Revision 74b0d4b17f39d238a7997800bd9363d3c60f20c3
Gaia Date 2015-10-09 19:27:39
Gecko Revision https://hg.mozilla.org/mozilla-central/rev/d1bb0de19476541cd517ab14017e7fedbd9f13e3
Gecko Version 44.0a1
Device Name aries
Firmware(Release) 4.4.2
Firmware(Incremental) eng.worker.20151009.234251
Firmware Date Fri Oct 9 23:42:59 UTC 2015
Bootloader s1
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage+], [MGSEI-Triage+]
Updated•9 years ago
|
Flags: needinfo?(seth)
You need to log in
before you can comment on or make changes to this bug.
Description
•