454718 - Change the return value type of gfxPlatformGtk::DPI() from PRUint32 to double

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Karl Tomlinson (:karlt)]

gfxPlatformGtk::DPI() is used as a float and calculated as a double but returns
an int.  There is one compiler warning:

/home/karl/moz/dev/gfx/thebes/src/gfxPlatformGtk.cpp: In static member function 'static void gfxPlatformGtk::InitDPI()':
/home/karl/moz/dev/gfx/thebes/src/gfxPlatformGtk.cpp:542: warning: converting to 'PRInt32' from 'double'

gfxPSSurface::Get/SetDPI() use doubles.

Comment 1

[Karl Tomlinson (:karlt)]

Attachment: patch

This also removes a dependency on pangocairo.

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Do we depend on pangocairo at all now? If we don't, can we discard some configure and build stuff?

Comment 3

[Karl Tomlinson (:karlt)]

(In reply to comment #2)
> Do we depend on pangocairo at all now?

Just one dependency left.

> can we discard some configure and build stuff?

Soon.  See bug 454730.

Comment 4

[Karl Tomlinson (:karlt)]

http://hg.mozilla.org/mozilla-central/rev/7b8f76d7f3d9

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

So we really wanted that GetDPI call to match the DPI that pango uses (which can differ from what GTK uses given some configuration settings ,IIRC), since sometimes we're sometimes getting font sizes from pango in points (in particular, in nsSystemFontsGTK2::GetSystemFontInfo).  (It looks like the gfxPangoFonts code doesn't have a branch for that case anymore, though, since we've dropped support for older versions of pango.)

It seems like this patch will do the wrong thing when the GDK DPI (which comes straight from X, I think) and the pango DPI (which is influenced by various preferences) disagree, and pango_font_description_get_size_is_absolute returns false or we're dealing with a version of pango old enough not to have that function (although I'd think we can remove the code for the latter case from nsSystemFontsGTK2 if we already have from gfxPangoFonts).

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(That said, in terms of calculating actual screen resolution, the GDK call is probably better.  But we really want the pango DPI in nsSystemFontsGTK.)

Comment 7

[Karl Tomlinson (:karlt)]

(In reply to comment #6)
> But we really want the pango DPI in nsSystemFontsGTK.

I recognize the need to get the right dpi, but the situation is not quite as
you describe.

(In reply to comment #5)
> So we really wanted that GetDPI call to match the DPI that pango uses (which
> can differ from what GTK uses given some configuration settings ,IIRC), since
> sometimes we're sometimes getting font sizes from pango in points (in
> particular, in nsSystemFontsGTK2::GetSystemFontInfo).  (It looks like the
> gfxPangoFonts code doesn't have a branch for that case anymore, though, since
> we've dropped support for older versions of pango.)
> 
> It seems like this patch will do the wrong thing when the GDK DPI (which comes
> straight from X, I think) and the pango DPI (which is influenced by various
> preferences) disagree, and pango_font_description_get_size_is_absolute returns
> false...

The "gtk-font-name" setting with the font size often in points is actually a
GTK setting, so it is the GTK dpi that we want (so using GDK seems
reasonable).  The GDK dpi includes user settings.

pango_font_description_from_string does no pt/px conversions.  It only records
whether the size is absolute or not.  I've checked this in Pango 1.14.10 and
1.20.5.

When GTK uses Pango to render its fonts, it tells Pango what resolution to
use.  This is certainly the case for GTK versions that use pangocairo, which
is all the versions that we support.

Back when GTK used pangoxft, it wouldn't have been possible to tell pangoxft
which resolution to use.  pangoxft/Xft use X Resources (Xft.dpi) to convert pt
to px, and I don't know whether GTK did the conversion to px before passing
the size to Pango.

In gfxPangoFonts we always create fonts with pixel sizes, so the DPI is not
relevant there (except for user fontconfig settings).

> ... or we're dealing with a version of pango old enough not to have that
> function (although I'd think we can remove the code for the latter case from
> nsSystemFontsGTK2 if we already have from gfxPangoFonts).

Yes, pango_font_description_get_size_is_absolute has been in pango since 1.8.  We officially only support 1.14 or newer, so we can assume that function is
available.