After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 342529 - gdk should set resolution on PangoCairoFontmap, not PangoCairoContext
gdk should set resolution on PangoCairoFontmap, not PangoCairoContext
Status: RESOLVED FIXED
Product: pango
Classification: Platform
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: pango-maint
pango-maint
: 314114 (view as bug list)
Depends on: 342525
Blocks:
 
 
Reported: 2006-05-21 23:17 UTC by Behdad Esfahbod
Modified: 2006-05-23 17:21 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
initial patch (3.70 KB, patch)
2006-05-22 00:45 UTC, Behdad Esfahbod
none Details | Review
committed patch (4.61 KB, patch)
2006-05-22 04:12 UTC, Behdad Esfahbod
rejected Details | Review
Committed patch to Pango (12.39 KB, patch)
2006-05-22 20:13 UTC, Behdad Esfahbod
committed Details | Review

Description Behdad Esfahbod 2006-05-21 23:17:12 UTC
+++ This bug was initially created as a clone of Bug #342525 +++


In gdk/gdkpango.c:gdk_pango_context_get_for_screen() sets dpi on PangoCairoContext, but font metrics use the fontmap dpi, not context.  Yes, this is a known bug in Pango, but one that cannot be easily fixed at this point.  This results in approximate digit width for a Gtk widget to not match the actual rendering for example.  (see original bug report for test)  The solution is to make gdk create one cairo fontmap per screen and set dpi on that.
Comment 1 Tor Lillqvist 2006-05-21 23:32:55 UTC
*** Bug 314114 has been marked as a duplicate of this bug. ***
Comment 2 Behdad Esfahbod 2006-05-22 00:45:43 UTC
Created attachment 65973 [details] [review]
initial patch

Ok, this fixes the issue by allocating one fontmap per screen.  This means duplicated caches across screens, but I don't think that's a major problem.

What I'm not sure however is whether changing the screen resolution that causes the fontmap resolution to be changed breaks anything.  Previously the PangoContext objects created for screen had their resolution locked, but now it will be changed behind their back.  However, when screen resolution changes, everything needs a redraw anyway.

Can use some docs update around gdk_screen_[gs]et_resolution().  And while there, are those really added for 2.10?  The Since: says so.
Comment 3 Matthias Clasen 2006-05-22 02:12:25 UTC
Looks good. 

gdk_screen_set_resolution was added as a _libgtk_only
function in 2.8.1. It was made public in 2.10.
Comment 4 Behdad Esfahbod 2006-05-22 04:12:51 UTC
Created attachment 65976 [details] [review]
committed patch

Committed to branch and HEAD.  No new API.
Comment 5 Behdad Esfahbod 2006-05-22 04:14:02 UTC
2006-05-21  Behdad Esfahbod  <behdad@gnome.org>

        * gdk/gdkinternals.h:
        * gdk/gdkscreen.c (gdk_screen_class_init), (gdk_screen_finalize),
        (update_fontmap_resolution), (gdk_screen_set_resolution):  Add new
        function _gdk_screen_get_font_map() and have one fontmap per screen,
        with the correct resolution set on it.

        * gdk/gdkpango.c (gdk_pango_context_get_for_screen): Use
        _gdk_screen_get_font_map() instead of setting resolution on the
        PangoCairoContext.  (#342529)

Comment 6 Owen Taylor 2006-05-22 14:58:05 UTC
[ Peanut Gallery comment ... that is, I haven't gone back and reread
  the code and tried to come up with an alternate change; I'm just throwing 
  in a random opinion ]

> In gdk/gdkpango.c:gdk_pango_context_get_for_screen() sets dpi on 
> PangoCairoContext, but font metrics use the fontmap dpi, not context

Why does the DPI *matter* for the metrics? Metrics only care about the
chosen size of the font.... the absolute size, in Pango terms. 

It appears to me that the originally issue in bug 342525 would have
been best fixed by setting an absolute size on the "measurement layout"
and forgetting about trying to propagate the DPI.

There's the separate question of pango_face_list_sizes(), which doesn't
have a context available, so that's something that this change would
fix, but that's a real corner case, and I wouldn't add a bunch of 
inefficiency just for that; an API extension like 
pango_face_list_absolute_sizes() or pango_face_list_sizes_for_resolution()
would work around that issue.
Comment 7 Behdad Esfahbod 2006-05-22 15:12:17 UTC
Yes, the reason is the font setting on the measurement layout.  I'm actually working on fixing it where you suggest, so I'll rollback this when I manage to do that.
Comment 8 Behdad Esfahbod 2006-05-22 19:18:24 UTC
Ok, reverted, trying to fix in Pango.
Comment 9 Behdad Esfahbod 2006-05-22 20:11:28 UTC
What remains is:

Win32 needs to return point size in pango_win32_font_describe()
ATSUI needs to implement pango_atsui_font_describe_absolute()


2006-05-22  Behdad Esfahbod  <behdad@gnome.org>

        * pango/pango-font.h, pango/fonts.c: New function
        pango_font_describe_with_absolute_size().

        * pango/pangocairo-font.c (_pango_cairo_font_get_hex_box_info):
        * pango/pangocairo-win32font.c (create_metrics_for_context):
        * pango/pangofc-font.c (pango_fc_font_class_init),
        (pango_fc_font_describe_absolute),
        (pango_fc_font_create_metrics_for_context):
        * pango/pangowin32.c (pango_win32_font_class_init),
        (pango_win32_font_get_metrics), (pango_win32_font_describe),
        (pango_win32_font_describe_absolute): Implement and use
        PangoFontClass->describe_absolute.

Comment 10 Behdad Esfahbod 2006-05-22 20:13:28 UTC
Created attachment 66012 [details] [review]
Committed patch to Pango

This adds a new virtual method to PangoFontClass called describe_absolute, and a public function pango_font_describe_with_absolute_size().

The metric logic is changed to use that for Fc backend, and the original bug is indeed fixed.
Comment 11 Behdad Esfahbod 2006-05-23 17:21:35 UTC
Remains ATSUI that Pango will print a warning for.

2006-05-23  Tor Lillqvist  <tml@novell.com>

        * pango/pangowin32.c (pango_win32_font_class_init): Initialize the
        describe_absolute method pointer correctly.
        (pango_win32_font_describe): Scale size to points which is what
        pango_font_description_set_size() wants. There has been several
        bugs opened around this issue, with more or less misleading
        guesses. See for instance #314114. Thanks to Behdad for finally
        noticing the real problem here.