Improving non-square large icon in MediaNotification

Improving non-square large icon in MediaNotification

This CL improves non-square large icon in MediaNotification by:
* Giving penalty score to non-square image.
* Rescalling non-square image while keeping the aspect ratio
  (paddings added).

BUG=656028
Committed: https://crrev.com/76252384438261f0c575e22296d5776a7e7f4f47
Cr-Commit-Position: refs/heads/master@{#428691}

[Zhiqiang Zhang (Slow), 2016-10-28 19:53:20]

zqzhang@chromium.org changed reviewers:
+ avayvod@chromium.org

[whywhat, 2016-10-28 20:19:26]

lgtm

do you have example sites/images? does it look better than if we crop the middle
square and scale it?

https://codereview.chromium.org/2462703002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaImageManager.java
(right):

https://codereview.chromium.org/2462703002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaImageManager.java:37:
*   - The dominant size score lies in [0, 1] and is computed using |mMimumSize|
and |mIdealSize|
nit: s/Mimum/Minimum

[Zhiqiang Zhang (Slow), 2016-10-28 20:49:24]

On 2016/10/28 20:19:26, whywhat wrote:
> lgtm
> 
> do you have example sites/images? does it look better than if we crop the
middle
> square and scale it?

I have a test page:
https://xxyzzzq.github.io/sandbox/media-session/test-non-square-icons.html

I have no idea which one is better. I personally prefer adding padding to make
it square.
If the provider doesn't give a square one, I think they know it will look bad
and they still want to do it.
We could ask UX for this, it should be two-lines change.

[whywhat, 2016-10-29 05:18:26]

On 2016/10/28 at 20:49:24, zqzhang wrote:
> On 2016/10/28 20:19:26, whywhat wrote:
> > lgtm
> > 
> > do you have example sites/images? does it look better than if we crop the
middle
> > square and scale it?
> 
> I have a test page:
> https://xxyzzzq.github.io/sandbox/media-session/test-non-square-icons.html
> 
> I have no idea which one is better. I personally prefer adding padding to make
it square.
> If the provider doesn't give a square one, I think they know it will look bad
and they still want to do it.
> We could ask UX for this, it should be two-lines change.

nit: s/could/should? :)

[Zhiqiang Zhang (Slow), 2016-10-31 12:08:19]

https://codereview.chromium.org/2462703002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaImageManager.java
(right):

https://codereview.chromium.org/2462703002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaImageManager.java:37:
*   - The dominant size score lies in [0, 1] and is computed using |mMimumSize|
and |mIdealSize|
On 2016/10/28 20:19:25, whywhat wrote:
> nit: s/Mimum/Minimum

Done.

[Zhiqiang Zhang (Slow), 2016-10-31 12:08:26]

The CQ bit was checked by zqzhang@chromium.org

[Zhiqiang Zhang (Slow), 2016-10-31 12:08:26]

The patchset sent to the CQ was uploaded after l-g-t-m from avayvod@chromium.org
Link to the patchset: https://codereview.chromium.org/2462703002/#ps20001
(title: "nits")

[commit-bot: I haz the power, 2016-10-31 12:08:40]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-31 12:37:37]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-10-31 12:39:42]

Description was changed from

==========
Improving non-square large icon in MediaNotification

This CL improves non-square large icon in MediaNotification by:
* Giving penalty score to non-square image.
* Rescalling non-square image while keeping the aspect ratio
  (paddings added).

BUG=656028
==========

to

==========
Improving non-square large icon in MediaNotification

This CL improves non-square large icon in MediaNotification by:
* Giving penalty score to non-square image.
* Rescalling non-square image while keeping the aspect ratio
  (paddings added).

BUG=656028

Committed: https://crrev.com/76252384438261f0c575e22296d5776a7e7f4f47
Cr-Commit-Position: refs/heads/master@{#428691}
==========

[commit-bot: I haz the power, 2016-10-31 12:39:43]

Patchset 2 (id:??) landed as
https://crrev.com/76252384438261f0c575e22296d5776a7e7f4f47
Cr-Commit-Position: refs/heads/master@{#428691}