Patryk Adamczyk [:patryk] UX Reporter
	Description • 10 years ago

The thumbnail scaling looks terrible, all the contacts look jagged, as if they were scaled using "nearest neighbour" algorithm. The thumbnails should be nice a smooth.

Also in the contact details the images are super fuzzy, I feel we can use a better algorithm to scale those up.

	Stephany Wilkes
	Comment 1 • 10 years ago

This was never assigned to anyone or marked for 2.0. Unblocking the 2.0 refresh and marking for 2.1.

	Jose Manuel Cantera Assignee
	Comment 2 • 10 years ago

The proposed patch 

A/ makes configurable the jpeg quality, size and format of the thumbnails 
B/ changes the image-rendering CSS property to auto as suggested at https://developer.mozilla.org/en-US/docs/Web/CSS/image-rendering
C/ uses the image utils module to perform the image scaling, instead of duplicating code

Patryk, visually-wise, can you give us feedback on whether this patch improves the situation?

Francisco, codewise what do you think about this patch? I believe it makes the app more flexible wrt thumbnails?

	Patryk Adamczyk [:patryk] UX Reporter
	Comment 3 • 10 years ago

Hey Jose, can you please attach a screenshot, I am having some trouble loading the build.

	Francisco Jordano [:arcturus] [:francisco]
	Comment 5 • 10 years ago

Comment on attachment 8511037 [details]
25495.html

Hi,

I see two points:
- We are adding the dependency with a huge library (we also will need to ask for review of the owner/maintainer). Fortunately image_thumbnail.js is being loaded async already, but wondering how this will affect to loading the form or any other view using this.
- We are coupling the FTU to the config file in contacts, and that doesnt seems right to me. Perhaps we should use default values and allow each app to load specific values per their own config.

A part from that, looking promising, can see the difference :)

	Jose Manuel Cantera Assignee
	Comment 6 • 10 years ago

(In reply to Francisco Jordano [:arcturus] [:francisco] from comment #5)
> Comment on attachment 8511037 [details]
> 25495.html
> 
> Hi,
> 
> I see two points:
> - We are adding the dependency with a huge library (we also will need to ask
> for review of the owner/maintainer).

oh yes

 Fortunately image_thumbnail.js is being
> loaded async already, but wondering how this will affect to loading the form
> or any other view using this.

But this wil be loaded only if the contact has a photo and once the user clicks on the save button

> - We are coupling the FTU to the config file in contacts, and that doesnt
> seems right to me. Perhaps we should use default values and allow each app
> to load specific values per their own config.

That can certainly be done, but actually the config file in contacts is already being use by the FTU. Actually it is copied to the same package. 


> 
> A part from that, looking promising, can see the difference :)

me too. Let's wait for Patryk verdict and then make this production-ready.

thanks!

	Patryk Adamczyk [:patryk] UX Reporter
	Comment 7 • 10 years ago

Comment on attachment 8513369 [details]
with the patch proposed patch applied

Thanks guys, this looks MUCH better!

	Jose Manuel Cantera Assignee
	Comment 8 • 10 years ago

Comment on attachment 8511037 [details]
25495.html

David could you have a look, particularly at the changes made to the shared/js/image_utils.js library? now it is allowed to specify the encondingOptions of the saved image. thanks!

	Francisco Jordano [:arcturus] [:francisco]
	Comment 9 • 10 years ago

Comment on attachment 8511037 [details]
25495.html

LGTM, just left a couple of nits in the code.

	David Flanagan [:djf]
	Comment 10 • 10 years ago

Comment on attachment 8511037 [details]
25495.html

r+ for the shared/js/image_utils.js change if you add a couple of lines of documentation and modify one line of the test. See my github comment.

	Francisco Jordano [:arcturus] [:francisco]
	Comment 11 • 10 years ago

Jose can you append a screenshot with the before and after results?

	Francisco Jordano [:arcturus] [:francisco]
	Comment 12 • 10 years ago

(In reply to Francisco Jordano [:arcturus] [:francisco] from comment #11)
> Jose can you append a screenshot with the before and after results?

Uppps, no worries, just saw the result on the second attachment.

	Jose Manuel Cantera Assignee
	Comment 13 • 10 years ago

https://github.com/mozilla-b2g/gaia/commit/0ac4557a5241f7532bf89d0ef3dec3b138f30c03

	Jose Manuel Cantera Assignee
	Comment 14 • 10 years ago

Comment on attachment 8511037 [details]
25495.html

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Contacts Thumbnails feature
[User impact] if declined: High. Images will be perceived as jagged
[Testing completed]: Yes
[Risk to taking this patch] (and alternatives if risky): Low risk patch. Unit tests provided
[String changes made]: none

	Ryan VanderMeulen [:RyanVM]
	Comment 15 • 10 years ago

v2.1: https://github.com/mozilla-b2g/gaia/commit/0ab53fc50af44a040f62e306a962fef1d52e7cd1

	Shally Li（leave from Mozilla）
	Comment 16 • 9 years ago

This bug has been verified as "pass" on latest nightly build of Flame v2.2 by the STR in Comment 0.

Actual results: Both the contact thumbnail and contact detail photo are nice and smooth.
See attachment: verified_Flame_v2.2.png
Reproduce rate: 0/10


Device: Flame v2.2 (Verified) 
Build ID               20150713002503
Gaia Revision          84d0c76370dcd3d25813b00de55194730884355b
Gaia Date              2015-07-09 13:09:14
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/8d59402ba85a
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150713.041147
Firmware Date          Mon Jul 13 04:11:59 EDT 2015
Bootloader             L1TC000118D0

	Shally Li（leave from Mozilla）
	Comment 18 • 9 years ago

Hi Jose,

    This bug has been verified as "fail" on latest nightly Flame v2.1. The contact thumbnail scaling looks jagged. Could you help to confirm this problem? 

Thank you very much.

-----------------------------------------------------------
Repro STR:
1.Import some contacts with photo from Memory card, or create a new contact with photo.
2.Observe the contact thumbnail.

See attachments: Verify_Flame_2.1&2.2.png and logcat_0541.txt
Reproduce rate: 10/10 (v2.1)


Device: Flame v2.1 build(Fail)
Build ID               20150713001203
Gaia Revision          d13826b20b4a45e3f5cd4b25a30a737d8be7f1b9
Gaia Date              2015-07-02 23:36:46
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/92049b3c4bb5
Gecko Version          34.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150713.035945
Firmware Date          Mon Jul 13 03:59:57 EDT 2015
Bootloader             L1TC000118D0

	Jose Manuel Cantera Assignee
	Comment 20 • 9 years ago

Hello Sally,

I have no idea as this patch landed in v2.1 as well. 

Redirecting to Francisco

thanks!

	Francisco Jordano [:arcturus] [:francisco]
	Comment 21 • 9 years ago

Yes it landed :)