In InitDPS, we try to get the monitor's DPS using pango_cairo_context_get_resolution. This method, on the device, returns -1 and we set the default DPI to 96. The n810 has a DPI of 225. I am not sure if this is a terrible thing. it looks like DPI is only used for system fonts?

Should not be in core graveyard

We need to find out what sDPI is getting set to in http://mxr.mozilla.org/mozilla-central/source/gfx/thebes/src/gfxPlatformGtk.cpp#528 on the n810. We'd like for that to get set correctly automatically. Either way, we basically need to ensure that layout.css.devPixelsPerPx is 1 and layout.css.dpi is the correct screen DPI

this hardcodes the DPI on the n810 using a preference. In a follow up patch, we can test the hardware version and do something different.

Comment on attachment 390934 [details] [diff] [review] set DPI based on pref for fonts * Do you want to use a #ifdef MOZ_PLATFORM_HILDON here? * You have some branding "dsstore" changes in here too

mfinkle, no, i think that change should be free of ifdefs, and I don't even know what a dsstore is. that part will not land.

vote for not blocking1.0

(In reply to comment #7) > vote for not blocking1.0 Ok, this should block

Comment on attachment 390934 [details] [diff] [review] set DPI based on pref for fonts This won't work for n810 / n900, which have different DPIs. We can't set a pref at build time.

This patch uses a component_version file on the n810/n900 to determine at runtime the device. It then sets a hard coded DPI or falls back to the baseline. The code _does_ determine the device, but returning the DPI breaks Fennec. The app doesn't even load. Still investigating.

i would rather you have this code somewhere else and have it just set the DPI pref correctly. Otherwise, you are also going to have to do a similar thing here: http://mxr.mozilla.org/mozilla-central/source/gfx/src/thebes/nsThebesDeviceContext.cpp#673 Any every new place that tests for this preference.

This patch uses a file shipped on the Nokia devices to determine the DPI. It caches the value so the file is only read and parsed once.

This code badly needs to be refactored so that we call just down into gfxPlatform to get the DPI and the platform-specific code can be hosted there. Are you up for that? It should be pretty quick and easy. If you post a patch for Windows and GTK I can finish off the Mac part.

This patch reorganizes the DPI code to live in gfxPlatform. It also adds a "ViewingRatio" which is created based on the DiagonalSize of the screen (in millimeters). The patch doesn't do anything with ViewingRatio yet - I need some direction in that area. I the patch builds and runs fine on my N900 (linux ARM). I pushed it to the try servers too and will report the results.

WIP patch of Fennec CSS changes needed for the corrected DPI on the N900 (N810) I have a little more work to do and will post some screenshots too. Obviously depends on the platform patch

Fixed some Windows breakage and tweaked the ViewingRatio estimates a bit

+PRInt32 gCachedDPI = -1; +PRInt32 gCachedDiagonal = -1; Why do we need these since they're cached in sDPI/sDiagonalSize anyway? + static PRInt32 sDPI; + static PRInt32 sDiagonalSize; Can you document what these mean? sDiagonalSize is the size of the (virtual) desktop in mm I guess? GetViewingRatio needs to be documented to say what it means. + // Small handhelds and phones (14") + if (sDiagonalSize < 127) That's 5" not 14". What do those parenthetical numbers mean? They're all off. + if (sDiagonalSize < 600) + return 1.0; + + // Large screens (In a room ~10ft) + if (sDiagonalSize < 1000) + return 4.0; I think lots of people (certainly at Mozilla) have screens more than 23" along the diagonal that they use from less than 10 feet away. It seems to me that GetViewingRatio as implemented requires a lot of guesswork. Maybe the best thing to do for now is to hardcode the value in a pref for Fennec and not use sDiagonal at all?

(In reply to comment #17) > +PRInt32 gCachedDPI = -1; > +PRInt32 gCachedDiagonal = -1; > > Why do we need these since they're cached in sDPI/sDiagonalSize anyway? We don't - removed > + static PRInt32 sDPI; > + static PRInt32 sDiagonalSize; > > Can you document what these mean? sDiagonalSize is the size of the (virtual) > desktop in mm I guess? > > GetViewingRatio needs to be documented to say what it means. I removed sDiagonalSize and GetViewingRatio from this patch > + // Small handhelds and phones (14") > + if (sDiagonalSize < 127) > > That's 5" not 14". What do those parenthetical numbers mean? They're all off. The parenthetical numbers were the viewing distance, not the display size, but this method has been removed. > It seems to me that GetViewingRatio as implemented requires a lot of guesswork. > Maybe the best thing to do for now is to hardcode the value in a pref for > Fennec and not use sDiagonal at all? In fact, I'd like to take viewing ratio to a new bug and leave this bug focused on DPI: Getting the right DPI on Maemo and moving the code for DPI into gfx platform. Yes, I chickened out, but I need to land DPI for Maemo quickly :( Patch coming up

This patch removes the viewing ratio code and fixes other review comments

Comment on attachment 401927 [details] [diff] [review] patch 4 (platform) Moved to Core:Graphics and requesting approval for 1.9.2

pushed platform patch to trunk: http://hg.mozilla.org/mozilla-central/rev/6d1c8ac530ad

pushed to 1.9.2: http://hg.mozilla.org/releases/mozilla-1.9.2/rev/ea32b873533c

This is an updated Fennec CSS patch, needed to handle the new true DPI on the n900/n810.

pushed the fennec CSS patch: https://hg.mozilla.org/mobile-browser/rev/6b89b88cca17

Comment on attachment 401927 [details] [diff] [review] patch 4 (platform) I'm not sure why but my Windows debug shared build didn't like this change - layers.dll was trying to link to sDPI (I fixed the problem locally by moving GetDPI to gfxPlatform.cpp).