459677 - default DPI on windows mobile?

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Doug Turner (:dougt)]

Attachment: patch v.1

in the nsThebesDeviceContext,  on windows, we call GetDeviceCaps to figure out the DPI for the display.  On the htc, this value is 360 (iirc, it was ~360).  This caused the browser window to only fill about 1/4 of the screen.  We hard coded this value to be 96 and that seems to have worked for the most part, however that caused the fennec "canvas" to not display:

http://dougt.files.wordpress.com/2008/10/200810101458.jpg?w=480&h=640

When we set this value to 48, we did get the correct result:

http://people.mozilla.org/~blassey/fennec_splash.PNG

Comment 1

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 342887 [details] [diff] [review]
patch v.1

drive by comment, setting it to 96 has the same effect.

Comment 2

[Doug Turner (:dougt)]

was your image at 96 or 48?

Comment 3

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Why not just set layout.css.dpi instead of hardcoding it?

Comment 4

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

There's actually something horked here, unrelated to the dpi -- for some reason everything is 2x the size in fennec by default, even when the system DPI is 96.  Not sure why this doesn't show on maemo, but it shows up on the desktop at least... I think it's related to the default CSS hardcoding some (very large) font sizes.  Setting the dpi to 48 would cause gecko to scale everything down by 1/2 (since CSS pixels are 96dpi).  Not sure what's going on.

Comment 5

[Brad Lassey [:blassey] (use needinfo?)]

(In reply to comment #2)
> was your image at 96 or 48?

48, but I tried 96 as well and it looks the same.

(In reply to comment #3)
> Why not just set layout.css.dpi instead of hardcoding it?

if we just set that, web content looks right because we're masking mAppUnitsPerDevPixel not being set correctly, but canvas is broken.  This is (I think) because nsCanvasRenderingContext2D does a lot of calculations based on mAppUnitsPerDevPixel, which is not set correctly.

Comment 6

[Stuart Parmenter]

Comment on attachment 342887 [details] [diff] [review]
patch v.1

this patch is not what we want -- we need to understand why things are breaking.  360 should be fine here.

Comment 7

[Brad Lassey [:blassey] (use needinfo?)]

the problem is GetDeviceCaps isn't returning 360, but garbage instead.  Checking MSDN, I found this:

> Note   This API is part of the complete Windows CE OS package as provided by
> Microsoft. The functionality of a particular platform is determined by the
> original equipment manufacturer (OEM) and some devices may not support this API.

Comment 8

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: converts css pixels to dev pixels

In XulWindow::SetSize, we're using CSS pixels to set the size of the native window which (I presume) expects device pixels.  This patch converts the css pixels to app units, and app units to device pixels.  Is there a way to do that in one step?

Comment 9

[Doug Turner (:dougt)]

Comment on attachment 343866 [details] [diff] [review]
converts css pixels to dev pixels

roc is travelling, maybe bz can look at this for your blassey.

Comment 10

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

(In reply to comment #7)
> the problem is GetDeviceCaps isn't returning 360, but garbage instead. 
> Checking MSDN, I found this:
> 
> > Note   This API is part of the complete Windows CE OS package as provided by
> > Microsoft. The functionality of a particular platform is determined by the
> > original equipment manufacturer (OEM) and some devices may not support this API.

We should be able to hardcode it to some appropriate value then (360 etc), maybe a set of different values depending on what we know about the device that it's being used on..

Comment 11

[Brad Lassey [:blassey] (use needinfo?)]

(In reply to comment #10)
> (In reply to comment #7)
> > the problem is GetDeviceCaps isn't returning 360, but garbage instead. 
> > Checking MSDN, I found this:
> > 
> > > Note   This API is part of the complete Windows CE OS package as provided by
> > > Microsoft. The functionality of a particular platform is determined by the
> > > original equipment manufacturer (OEM) and some devices may not support this API.
> 
> We should be able to hardcode it to some appropriate value then (360 etc),
> maybe a set of different values depending on what we know about the device that
> it's being used on..

turns out GetDeviceCaps is returning a sensible value of 192.  The debugger says OSVal and dpi are 0, but printf'ing them shows them to be 192.

Comment 12

[Boris Zbarsky [:bzbarsky]]

The coordinate conversions look ok, but I'm not sure we want to depend on having a prescontext here.  Seems safer to just create our own device context for mWindow and then do the conversions using that.

Comment 13

[Boris Zbarsky [:bzbarsky]]

That is, basically reimplement nsPresContext::AppUnitsToDevPixels using our own device context.

Comment 14

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: removes need for nsPresContext

Comment 15

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 344431 [details] [diff] [review]
removes need for nsPresContext

Excellent.  Good catch on just getting the device context off the widget.

Comment 16

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 344431 [details] [diff] [review]
removes need for nsPresContext

Risk: Seems minimal.  Scariest operation is getting appUnitsPerDevPixel from the device context, which should always exist.


Reward: Correctly handles window sizes on high dpi screens.

Comment 17

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 344431 [details] [diff] [review]
removes need for nsPresContext

a1.9.1b2=beltzner

Comment 18

[Dave Townsend [:mossop]]

Pushed: http://hg.mozilla.org/mozilla-central/rev/0486e61caa41

Comment 19

[Daniel Brooks [:db48x]]

I think this fix caused a regression; windows such as the Add-ons manager or the Prefs dialog are twice as large as they should be when layout.css.dpi is set. The initial size specified when the window is opened is scaled as expected, but then the next time the window is opened, the size read from the localstore is also scaled. Since the size in the localstore is already the result of scaling the initial size, this results in windows that are much larger than they ought to be.

Comment 20

[Brad Lassey [:blassey] (use needinfo?)]

sounds like we need to scale to app units before storing the value.  Can you file a bug on that?

Comment 21

[:Gavin Sharp [email: [e-mail]]]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/0486e61caa41

Comment 22

[Atsushi Sakai]

(In reply to comment #20)
> sounds like we need to scale to app units before storing the value.

Please consider also Bug 469933.

Comment 23

[Boris Zbarsky [:bzbarsky]]

Did a bug ever get filed on comment 19?

Comment 24

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #23)
> Did a bug ever get filed on comment 19?

yep: bug 471729