947029 - [B2G][Gallery][Small images]Extremely small images display in gallery with a larger than expected thumbnail.

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: Gaia::Gallery, defect)

Description

[Ross Kuhlman ([e-mail])]

Attachment: Comparison of Image in viewer and image in gallery grid.

The Gallery app displays thumbnails arranged in a grid. If the source image is smaller than the thumbnail, the image will be upscaled to completely fill its alotted thumbnail. This can be misleading for users, who are led to believe that an image is larger than it actually is. There should be a cap to how much a small image gets upscaled. Maybe 200%

Repro Steps:
Prerequisite: Have a very small picture available on the Device. (Ex: 1x1 pixel)

1) Updated Buri to Build ID: 20131204004003
2) Launch the Gallery app.
3) Observe the small picture in the gallery grid.
4) Tap on the image to view it.

Actual:
The image in the Gallery grid is far larger than the actual image.

Expected:
There is a limit to how much an image can be upscaled. Suggested cap: 200%

Environmental Variables
Device: Buri v 1.2 COM RIL
Build ID: 20131204004003
Gecko: http://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/758f3fb32dda
Gaia: 8d762f3376318fd6be390432db750ae4904c9ab6
Platform Version: 26.0
RIL Version: 01.02.00.019.102 
Firmware Version: V1.2_20131115

Notes:
Repro frequency: 100%
See attached screenshots.

Images also display with upscaling when recieved by text messages.

Comment 1

[Ross Kuhlman ([e-mail])]

Attachment: SmallImagesInMessageApp.png

Issue also occurs in 1.1

Environmental Variables
Device: Leo v 1.1 Moz RIL
Build ID: 20131122041201
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/c699a8e7bde9
Gaia: b7610870ec71495685557744bfbcbce357032251
Platform Version: 18.1 
Firmware Version: lge_default

Comment 2

[David Flanagan [:djf]]

This seems very much like a corner case, but I guess I agree that these images should not be upscaled.  I think this has come up before and at some point we displayed small images at their natural size.  So possibly this is a regression.

I forget what CSS is used to display the thumbnails. It might be that this just needs a CSS tweak and then you can use tiny images unmodified as their own thumbnail.

On the other hand, the assumption that thumbnail images are square goes pretty deep in the gallery code, so I suspect that for tiny images like these, you may have to create a *larger* thumbnail that is black with the tiny image centered in the middle.  Transparent black might work as the background, but we've had some scrolling performance issues with transparent before, so hardcoding an opaque background is probably better.

The code for creating thumbnails in in gallery/js/MetadataParser.js, I think.

Comment 3

[Russ Nicoletti [:russn]]

I can confirm the gallery js uses the original image if the image is smaller than the thumbnail size: https://github.com/mozilla-b2g/gaia/blob/master/apps/gallery/js/MetadataParser.js#L297-298

Comment 4

[Russ Nicoletti [:russn]]

Attachment: https://github.com/mozilla-b2g/gaia/pull/15487

https://github.com/mozilla-b2g/gaia/pull/15487

Comment 5

[Russ Nicoletti [:russn]]

Hi David, Can you review my pull request in comment #4? Thanks.

Comment 6

[David Flanagan [:djf]]

Comment on attachment 8361956 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/15487

I'd like you to tackle this one differently, Russ.  See my comments on github.

Comment 7

[David Flanagan [:djf]]

Also, note that we have a one-commit per pull request rule, so you'll need to squash your commits. (Let me know if you don't know how to do this.  I use 'git rebase -i master'

Comment 8

[Russ Nicoletti [:russn]]

Attachment: https://github.com/mozilla-b2g/gaia/pull/16340

Hi David, the updated solution doesn't persist any new data in the db. I've also added unit tests. I've pushed the new changes to a different branch than the old changes so you want see any trace of the old changes.

Comment 9

[David Flanagan [:djf]]

Comment on attachment 8376726 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/16340

Russ,

This is really great. I love how clean the patch becomes with the code moved to gallery.js.  But I'd like you to take it further and move that thumbnail size related code into a module of its own.  See my comments on github for a sketch of how you could do this.

Comment 10

[Russ Nicoletti [:russn]]

Attachment: New pull request

Hi David, I've moved the thumbnail size logic to a separate object.

Comment 11

[David Flanagan [:djf]]

Comment on attachment 8381762 [details] [review]
New pull request

This is another good patch that got buried in my review queue. It is now a year old and I assume it is bitrotted, so I'm clearing the review request.

But it does seem like a good refactoring, and it has tests, so we ought to go ahead and get this landed. Russ: if you'd like to revive it I promise to review it this time. Or, since you're working on the video player component, this might be a good one to pass on to Punam. One immediate comment is that this patch adds the new files to the jshint xfail list, and I'd prefer that you didn't do that. Instead make the new files pass jshint.

Comment 12

[Russ Nicoletti [:russn]]

Punam, if you'd like to take this, that's fine with me. If not, that's also fine, I will take it up in the next few days.

Comment 13

[BMO Automation]

Firefox OS is not being worked on