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 418585 - GtkFileChooserDefault sizing code is not DPI independent
GtkFileChooserDefault sizing code is not DPI independent
Status: RESOLVED OBSOLETE
Product: gtk+
Classification: Platform
Component: Widget: GtkFileChooser
unspecified
Other Linux
: High major
: ---
Assigned To: Federico Mena Quintero
Federico Mena Quintero
Depends on:
Blocks:
 
 
Reported: 2007-03-15 14:23 UTC by Marco Pesenti Gritti
Modified: 2013-06-14 16:56 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gtk+-418585-file-chooser-sizing.diff (1.31 KB, patch)
2007-03-15 19:11 UTC, Federico Mena Quintero
committed Details | Review

Description Marco Pesenti Gritti 2007-03-15 14:23:45 UTC
Snippet of the code:

font_size = pango_font_description_get_size (widget->style->font_desc);
font_size = PANGO_PIXELS (font_size);

default_width = font_size * NUM_CHARS;
default_height = font_size * NUM_LINES;

Despite the name it seem like PANGO_PIXELS it just converting the font size from pango units to points. So the dialog size is proportional to the font size in points.

The result is that on screens with an high DPI (201 on the OLPC for example) the dialog size is way too small.
Comment 1 Dan Williams 2007-03-15 14:32:52 UTC
The problem is that the code is treating a variable in _points_ (font_size) like it's actually in pixels.

PANGO_PIXELS() doesn't actually do any points -> pixels conversion, it simply converts the value from pango units to regular device units (getting rid of the PANGO_SCALE factor), but that's _still_ in points.  Bad name I guess.

So after PANGO_SCALE, the file chooser default size code needs to convert from points to pixels, taking the display's DPI into account.
Comment 2 Federico Mena Quintero 2007-03-15 17:15:21 UTC
Wow, *excellent* catch.  I had no idea that this was causing the ugly sizing we have.  I'll cook a patch.
Comment 3 Federico Mena Quintero 2007-03-15 19:11:02 UTC
Created attachment 84671 [details] [review]
gtk+-418585-file-chooser-sizing.diff
Comment 4 Federico Mena Quintero 2007-03-16 00:59:19 UTC
Thanks a lot for catching this silly bug :)  I've committed the patch to gtk-2-10 and trunk.

2007-03-15  Federico Mena Quintero  <federico@novell.com>

        * gtk/gtkfilechooserdefault.c (find_good_size_from_style):
        PANGO_PIXELS() gives us device units, which are *points* in
        pangocairo's parlance, but we want actual pixels.  So, get the
        screen's resolution to compute the actual number of pixels.
        Fixes bug #418585.
Comment 5 Behdad Esfahbod 2007-03-16 01:21:55 UTC
Notes:

  * PANGO_PIXELS() is agnostic to the units in use.  They may be points or pixels.  Depends on the input.

  * pango_font_description_get_size() may return value in points or pixels.  You can now by calling pango_font_description_get_size_is_absolute().

  * Instead of dealing with screen directly, I suggest you create a layout with the text that is a single space character, and use pango_layout_get_pixel_extents() on it.
Comment 6 Federico Mena Quintero 2007-03-17 01:49:49 UTC
(In reply to comment #5)

>   * pango_font_description_get_size() may return value in points or pixels. 
> You can now by calling pango_font_description_get_size_is_absolute().
> 
>   * Instead of dealing with screen directly, I suggest you create a layout with
> the text that is a single space character, and use
> pango_layout_get_pixel_extents() on it.

So many coordinate systems...

For the file chooser's purposes, I need to answer this question:

- How many pixels tall is a typical line of text in a widget's normal style?

Then I can use that info to say, "oh, I want to have at least N lines in the file list, so the file list should be at least N*num_pixels tall".

I have the PangoFontDescription from widget->style.  How do I get a suitable height in pixels?

Assume I have a realized widget.  Is the following good?

PangoFontMetrics *metrics;

metrics = pango_context_get_metrics (gtk_widget_get_pango_context (widget),
                                     widget->style->font_desc,
                                     gtk_get_default_language ());

height_in_pixels = PANGO_PIXELS (pango_font_metrics_get_ascent (metrics) + pango_font_metrics_get_descent (metrics));

Comment 7 Marco Pesenti Gritti 2007-03-19 18:56:19 UTC
Federico, I think the changes you checked in increase the effective dialog size (even on a 96 dpi screen). Should NUM_LINES and NUM_CHARS be tweaked to retain the old size?

Something like:

NUM_CHARS: 60 * 72 / 96 = 45
NUM_LINES: 45 * 72 / 96 = 34
Comment 8 Federico Mena Quintero 2007-03-21 02:55:54 UTC
Hmmm, I'm not sure.  With the patch as-is, I get a very nice size on my screen :)

Can you do some "svn blame" to see if the NUM_* macros got changed in the past to try to make the dialog bigger?  I don't remember the history of that code anymore.
Comment 9 Marco Pesenti Gritti 2007-03-21 10:04:10 UTC
It was increased slightly here...

2007-02-26  Matthias Clasen <mclasen@redhat.com>

        Merge from trunk:

        Apply a patch by Carlos Garnacho to fix several problems
        with filechooser size handling (#325477, #151169, 143213,
        #153785)

        * gtk/gtkfilechooserdefault.c: Increase NUM_LINES slightly.
        (browse_widgets_create): Don't force
        the paned position to 200.
        (find_good_size_from_style):
        Take the size of the extra widget into account.

        * gtk/gtkfilechooserdialog.c (file_chooser_widget_update_hints):
        Accept a minimal width parameter.  Update all callers.



On the OLPC it looks way too big, readapting the values gives perfect sizing. On what screen size are you testing? Even on a stock GNOME desktop at 1024x768 it feels too big to me. At 800x600 it will basically take all the screen, so, if we decide to keep the current values, we will have to improve the logic to take screen size in consideration.
Comment 10 Federico Mena Quintero 2007-03-23 02:28:29 UTC
(In reply to comment #9)

> On the OLPC it looks way too big, readapting the values gives perfect sizing.
> On what screen size are you testing? Even on a stock GNOME desktop at 1024x768
> it feels too big to me. At 800x600 it will basically take all the screen, so,
> if we decide to keep the current values, we will have to improve the logic to
> take screen size in consideration.

I'm testing this on two machines, one 1280x1024 and one 1400x1050, both at 122 DPI.

A long time ago the sizing code just tried to use 75% of the screen in both the vertical and horizontal axes, but that got changed afterwards to the current state.  Maybe we need to stick a "limit to 75%" test in there... if you come up with such a patch that is suitable for OLPC as well, could you please attach it to another bug?  I'll be sure to review it quickly.  Thanks! :)
Comment 11 Federico Mena Quintero 2007-03-23 02:29:02 UTC
Bleh, never mind - just stick the patch in this bug.  I didn't realize it was still unresolved.
Comment 12 Timothy Arceri 2013-06-13 12:38:05 UTC
Is this bug still valid? Looks like an initial fix was made. But Marco never provided a patch for the tweak he wanted.
Comment 13 Federico Mena Quintero 2013-06-14 16:56:04 UTC
This is a brief and incomplete history of the sizing code:

1. GtkTreeView wasn't able to come up with a good default size by itself, so the file chooser decided to use "N characters by M lines" by default.

2. The computation was broken, as evidenced by this bug.

3. The computation got fixed, I think.

4. The file chooser knows that it will get the computation wrong for some people's liking, and that's why it tries to remember the size when the user resizes the window.

5. That code to remember the size was broken at some point, and then it got fixed.  To the best of my knowledge, it works correctly these days.

6. I have no idea if the *current* GtkTreeView, with GTK+'s natural sizing and everything, would be able to compute a good default size by itself these days.

My rationale for the sizing code was always, "I want a file list that will show a good number of lines, and a good number of horizontal characters, so it's as useful as doing ls -l on the console".  Or something like that.  What is unacceptable is getting a file chooser that displays only two or three lines, or that is too narrow to show typical filenames ("Letter for my boss.odp", not hackerish "gtkhsv.c") :)

And due to (5), the rationale is "if you set the size once to something you like, it should be preserved, so that you don't have a reason to complain afterwards" ;)

For the time being, I'll mark this bug as obsolete.