Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Listview lines not correctly showing the number of elements expected #1190

Closed
steckhelena opened this issue Sep 12, 2020 · 18 comments
Closed
Labels
Milestone

Comments

@steckhelena
Copy link

steckhelena commented Sep 12, 2020

Version

Version: 1.6.0

Configuration

https://gist.github.com/steckhelena/ba5c5f18216f7bf94453ff3080986e8e

Launch Command

echo "Headphone;Speakers" | rofi -sep ";" -dmenu -p "Select audio device:" -i -theme AudioSelMenu

Steps to reproduce

  • Set, in the theme file, the following: (minimal config to reproduce it)
* {
    font: "Cantarell 18";
}

window {
    children:   [ vertibox ];
}

vertibox {
    orientation: vertical;
    children:   [ prompt, listview ];
}

listview {
    lines: 2;
}
  • Set any font in the global properties section(as shown above)
  • If the font is set you will see a menu with one of the elements hidden, and only being able to be select with arrow keys up/down
  • If the font property OR the prompt child is set the bug can be reproduced

What behaviour you see

  • Rofi does not show two lines as expected, and instead just show one line which, with the cycle option set to true, I can select the other option by pressing up or down keys.

What behaviour you expect to see

  • I expect to see both input lines without having to scroll between them because one is hidden.

Additional details:

I started experiencing this yesterday after updating rofi to the latest version, it worked as expected before that(at least I think it was the expected behavior, if I am wrong about it, please feel free to correct me)

Thanks for the amazing software! Any other information needed, or if I have some wrong config I will be following the issue.

@DaveDavenport
Copy link
Collaborator

Does it also happen without the icons?

@steckhelena
Copy link
Author

steckhelena commented Sep 12, 2020

Yes, first thing I tried

@DaveDavenport
Copy link
Collaborator

Interresting it works for me.

What I had happened before is that pango indicated height (using ascent en descent query on font) is smaller then the actual font height when rendered. This screws up rofi as the sizing goes off.

@DaveDavenport
Copy link
Collaborator

DaveDavenport commented Sep 12, 2020

matching dpi and using above example:

rofi-2020-09-12-2126-00000

@DaveDavenport
Copy link
Collaborator

DaveDavenport commented Sep 12, 2020

what if you set 'fixed-height" to false (on listview) and lines to > 2? (or dynamic on listview to true)

@steckhelena
Copy link
Author

steckhelena commented Sep 12, 2020

If I set fixed-height to false, whatever number I put on lines it still has the same problem. With dynamic and >2 lines it shows all the entries, but it still has an unwanted space at the bottom. With dynamic and 2 lines it still has the same problem.

@steckhelena
Copy link
Author

steckhelena commented Sep 12, 2020

I found that with lines >2 it and no other properties, it shows both lines as expected, but an unwanted space at the bottom (as expected xD)

@DaveDavenport
Copy link
Collaborator

Thanks.

It seems for some reason the font height calculation on your system is different then on mine.
What distro are you running?

@steckhelena
Copy link
Author

steckhelena commented Sep 12, 2020

Btw, for me, the above example will be as follows:
rofi-2020-09-12-1647-00000

@steckhelena
Copy link
Author

I am running Arch with i3

@DaveDavenport
Copy link
Collaborator

For some reason a simple thing as figuring out what the max height of a font line is difficult :(. This is a problem that keeps popping up.

Thanks, I will make a vm with archlinux and try it out, (weird issues like this seems to happen on arch more often with it 'bleeding edge' )

@DaveDavenport
Copy link
Collaborator

DaveDavenport commented Sep 12, 2020

.... no it is font sizing issue.

I get 31 pixels when rendered, but 30 pixels when asked how high font is..

@DaveDavenport
Copy link
Collaborator

workaround might be to set : expand: false; on listview.

@steckhelena
Copy link
Author

That was fast, the workaround works on the minimal config I posted above:
rofi-2020-09-12-1739-00000

But sadly it won't work if I set spacing on listview or padding on element, or margin on window xD

@DaveDavenport
Copy link
Collaborator

Yeah, the problem is that with pango I get (sometimes) a 1 pixel difference in estimated height and actual height.. annoying ..
I wonder if this is a result of the new rendering backend.

@DaveDavenport
Copy link
Collaborator

DaveDavenport commented Sep 12, 2020

Can you try the following patch?

diff --git a/include/widgets/textbox.h b/include/widgets/textbox.h
index 229e34d9..cb9be121 100644
--- a/include/widgets/textbox.h
+++ b/include/widgets/textbox.h
@@ -43,6 +43,17 @@
  *
  * @{
  */
+
+/** Cache to hold font descriptions. This it to avoid having to lookup each time. */
+typedef struct TBFontConfig
+{
+    /** Font description */
+    PangoFontDescription *pfd;
+    /** Font metrics */
+    PangoFontMetrics     *metrics;
+    /** height */
+    double               height;
+}TBFontConfig;
 /**
  * Internal structure of a textbox widget.
  * TODO make this internal to textbox
@@ -66,7 +77,8 @@ typedef struct
     double             yalign;
     double             xalign;
 
-    PangoFontMetrics   *metrics;
+    TBFontConfig       *tbfc;
+
     PangoEllipsizeMode emode;
     //
     const char         *theme_name;
diff --git a/source/widgets/textbox.c b/source/widgets/textbox.c
index 3f5d0633..d22b5913 100644
--- a/source/widgets/textbox.c
+++ b/source/widgets/textbox.c
@@ -55,16 +55,8 @@ static PangoContext     *p_context = NULL;
 /** The pango font metrics */
 static PangoFontMetrics *p_metrics = NULL;
 
-/** Cache to hold font descriptions. This it to avoid having to lookup each time. */
-typedef struct TBFontConfig
-{
-    /** Font description */
-    PangoFontDescription *pfd;
-    /** Font metrics */
-    PangoFontMetrics     *metrics;
-    /** height */
-    double               height;
-}TBFontConfig;
+/* Default tbfc */
+TBFontConfig *tbfc_default = NULL;
 
 /** HashMap of previously parsed font descriptions. */
 static GHashTable *tbfc_cache = NULL;
@@ -133,7 +125,7 @@ static WidgetTriggerActionResult textbox_editable_trigger_action ( widget *wid,
 
 static void textbox_initialize_font ( textbox *tb )
 {
-    tb->metrics = p_metrics;
+    tb->tbfc = tbfc_default;
     const char * font = rofi_theme_get_string ( WIDGET ( tb ), "font", NULL );
     if ( font ) {
         TBFontConfig *tbfc = g_hash_table_lookup ( tbfc_cache, font );
@@ -142,7 +134,13 @@ static void textbox_initialize_font ( textbox *tb )
             tbfc->pfd = pango_font_description_from_string ( font );
             if ( helper_validate_font ( tbfc->pfd, font ) ) {
                 tbfc->metrics = pango_context_get_metrics ( p_context, tbfc->pfd, NULL );
-                tbfc->height  = pango_font_metrics_get_ascent ( tbfc->metrics ) + pango_font_metrics_get_descent ( tbfc->metrics );
+
+                PangoLayout *layout = pango_layout_new(p_context );
+                pango_layout_set_text(layout,"aAjb", -1);
+                PangoRectangle rect;
+                pango_layout_get_pixel_extents(layout, NULL, &rect );
+                tbfc->height  = rect.y + rect.height;
+                g_object_unref ( layout);
 
                 // Cast away consts. (*yuck*) because table_insert does not know it is const.
                 g_hash_table_insert ( tbfc_cache, (char *) font, tbfc );
@@ -156,7 +154,7 @@ static void textbox_initialize_font ( textbox *tb )
         if ( tbfc ) {
             // Update for used font.
             pango_layout_set_font_description ( tb->layout, tbfc->pfd );
-            tb->metrics = tbfc->metrics;
+            tb->tbfc = tbfc;
         }
     }
 }
@@ -419,7 +417,7 @@ static void textbox_draw ( widget *wid, cairo_t *draw )
     // Skip the side MARGIN on the X axis.
     int x = widget_padding_get_left ( WIDGET ( tb ) );
     int top = widget_padding_get_top ( WIDGET ( tb ) );
-    int y = ( pango_font_metrics_get_ascent ( tb->metrics ) - pango_layout_get_baseline ( tb->layout ) ) / PANGO_SCALE;
+    int y = ( pango_font_metrics_get_ascent ( tb->tbfc->metrics ) - pango_layout_get_baseline ( tb->layout ) ) / PANGO_SCALE;
     int line_width = 0, line_height = 0;
     // Get actual width.
     pango_layout_get_pixel_size ( tb->layout, &line_width, &line_height );
@@ -828,7 +826,15 @@ void textbox_set_pango_context ( const char *font, PangoContext *p )
     p_metrics = pango_context_get_metrics ( p_context, NULL, NULL );
     TBFontConfig *tbfc = g_malloc0 ( sizeof ( TBFontConfig ) );
     tbfc->metrics = p_metrics;
-    tbfc->height  = pango_font_metrics_get_ascent ( tbfc->metrics ) + pango_font_metrics_get_descent ( tbfc->metrics );
+
+    PangoLayout *layout = pango_layout_new( p_context );
+    pango_layout_set_text(layout,"aAjb", -1);
+    PangoRectangle rect;
+    pango_layout_get_pixel_extents(layout, NULL, &rect );
+    tbfc->height  = rect.y + rect.height;
+    g_object_unref ( layout);
+    tbfc_default = tbfc;
+
     g_hash_table_insert ( tbfc_cache, (gpointer *) ( font ? font : default_font_name ), tbfc );
 }
 
@@ -866,15 +872,15 @@ int textbox_get_height ( const textbox *tb )
 
 int textbox_get_font_height ( const textbox *tb )
 {
-    int height;
-    pango_layout_get_pixel_size ( tb->layout, NULL, &height );
-    return height;
+    PangoRectangle rect;
+    pango_layout_get_pixel_extents ( tb->layout, NULL, &rect );
+    return rect.height+ rect.y;
 }
 
 int textbox_get_font_width ( const textbox *tb )
 {
     PangoRectangle rect;
-    pango_layout_get_pixel_extents ( tb->layout, NULL, &rect );
+    pango_layout_get_pixel_extents ( tb->layout, NULL, &rect);
     return rect.width + rect.x;
 }
 
@@ -883,8 +889,7 @@ static double char_height = -1;
 double textbox_get_estimated_char_height ( void )
 {
     if ( char_height < 0 ) {
-        int height = pango_font_metrics_get_ascent ( p_metrics ) + pango_font_metrics_get_descent ( p_metrics );
-        char_height = ( height ) / (double) PANGO_SCALE;
+        char_height = tbfc_default->height;
     }
     return char_height;
 }
@@ -913,8 +918,8 @@ double textbox_get_estimated_ch ( void )
 
 int textbox_get_estimated_height ( const textbox *tb, int eh )
 {
-    int height = pango_font_metrics_get_ascent ( tb->metrics ) + pango_font_metrics_get_descent ( tb->metrics );
-    return ceil ( ( eh * height ) / (double) PANGO_SCALE ) + widget_padding_get_padding_height ( WIDGET ( tb ) );
+    int height = tb->tbfc->height;
+    return ceil ( ( eh * height ) ) + widget_padding_get_padding_height ( WIDGET ( tb ) );
 }
 int textbox_get_desired_width ( widget *wid )
 {

@steckhelena
Copy link
Author

With the above patch, it works! Here's the result with my config(with prompt child back in it):

rofi-2020-09-12-2019-00000

@DaveDavenport DaveDavenport added this to the 1.6.1 milestone Sep 13, 2020
@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants