884706 - Update GetDefaultScaleInternal() again to enable FxOS for devices beyond retina

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Tim Guan-tin Chien [:timdream] (please needinfo)]

+++ This bug was initially created as a clone of Bug #845182 +++

So I made a wrong assumption about dppx: The function in bug #878029 does not return any value more than 2, but as of 2013, we already have some ddpx=3 porting target out there.

https://github.com/ResponsiveImagesCG/picture-element/wiki/Display-technologies:-beyond-Retina

It would be a really easy patch enabling people to port FxOS to these device by adopt the the |floor()| part on our Android backend here.

http://hg.mozilla.org/mozilla-central/file/e5b2e2ee91e0/content/base/src/nsContentUtils.cpp#l4868

Comment 1

[Mason Chang]

Attachment: updateDefaultScale.patch

Hi Tim,

I'm new to this and thought I'd tackle this bug. Thanks for your comment, it seems like a simple fix. I have attached a patch that's a first stab at this based on your comments. It seems like a simple enough patch. I built the B2G Desktop environment on OS X 10.8.2 and it compiled and ran. I don't have a phone to try it on though. Is there anything else I should try?

Thanks!

Comment 2

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Mason,

Your code was left unattended because you have not set anyone for review. Please ensure there is always someone being marked review?, feedback?, needinfo? to ensure the bug don't get left out (like what I do to you now).

Comment 3

[Michael Wu [:mwu]]

Tim, this patch makes it so that dpi between 200 and 300 are 1.5x scaled, rather than between 200 and 280. Is that ok with you? It makes things match the linked Android logic, at least, but we should change the code for the 1.5x scaling part to make things obvious.

Comment 4

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Michael Wu [:mwu] from comment #3)
> Tim, this patch makes it so that dpi between 200 and 300 are 1.5x scaled,
> rather than between 200 and 280. Is that ok with you? It makes things match
> the linked Android logic, at least, but we should change the code for the
> 1.5x scaling part to make things obvious.

Agree. It is unclear to me if we would have a device within that dpi range (280-300) though.

Comment 5

[Michael Wu [:mwu]]

Comment on attachment 783547 [details] [diff] [review]
updateDefaultScale.patch

Ok. Please fix the 1.5x scaling case and I'll rubberstamp this.

Comment 6

[Mason Chang [Inactive] [:mchang]]

Attachment: updateDefaultScale.patch

Updated to reflect the same dpi as Android backend.

Comment 7

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8335495 [details] [diff] [review]
updateDefaultScale.patch

># HG changeset patch
># Parent 597287004ff59cd132e0b74fb1a19b18b61a3e86
>
>diff -r 597287004ff5 widget/gonk/nsWindow.cpp
>--- a/widget/gonk/nsWindow.cpp	Wed Nov 20 15:16:32 2013 +0100
>+++ b/widget/gonk/nsWindow.cpp	Wed Nov 20 12:06:10 2013 -0800
>     }
>-    return 2.0; // xhdpi devices.
>+    return floor(dpi / 150.0); // xhdpi devices.
s/xhdpi devices/xhdpi devices and beyond/

Comment 8

[Mason Chang [Inactive] [:mchang]]

Attachment: updateDefaultScale.patch

Carrying over r+ from mwu. Modified comment as stated in Comment 7.

Successful try server:
https://tbpl.mozilla.org/?tree=Try&rev=e8c4085694ca

Comment 9

[(Out Sep 17-23) Ryan VanderMeulen]

https://hg.mozilla.org/integration/b2g-inbound/rev/4a006106c65c

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/4a006106c65c