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

RESOLVED FIXED in mozilla1.9.1b1

Status

()

Core
Graphics
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: karlt, Assigned: karlt)

Tracking

Trunk
mozilla1.9.1b1
x86
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

9 years ago
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.
(Assignee)

Comment 1

9 years ago
Created attachment 338015 [details] [diff] [review]
patch

This also removes a dependency on pangocairo.
Attachment #338015 - Flags: review?(roc)
(Assignee)

Updated

9 years ago
Blocks: 454730
Attachment #338015 - Flags: review?(roc) → review+
Do we depend on pangocairo at all now? If we don't, can we discard some configure and build stuff?
(Assignee)

Comment 3

9 years ago
(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.
(Assignee)

Comment 4

9 years ago
http://hg.mozilla.org/mozilla-central/rev/7b8f76d7f3d9
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
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).
(That said, in terms of calculating actual screen resolution, the GDK call is probably better.  But we really want the pango DPI in nsSystemFontsGTK.)
(Assignee)

Comment 7

9 years ago
(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.
(Assignee)

Updated

9 years ago
Target Milestone: --- → mozilla1.9.1b1
You need to log in before you can comment on or make changes to this bug.