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

gtkui: Hide info area text containing emoji properly. Closes: #1491 #171

Merged
merged 3 commits into from
Oct 21, 2024

Conversation

radioactiveman
Copy link
Member

@radioactiveman radioactiveman commented Oct 16, 2024

Fading out text with emoji characters can lead to them still being displayed above the text of a new track.
Ignoring zero alpha channels avoids this.

Reproduced on Ubuntu 24.04. Arch Linux requires "noto-fonts-emoji".

@radioactiveman
Copy link
Member Author

@jlindgren90: Do you think this is a valid fix? I'm still not sure how and why it works. My first guess was alpha channels should never be negative, but clamping the value to 0..1 does not help.

The other commit only optimizes the font handling code. It's not required to fix #1491.

@jlindgren90
Copy link
Member

I'd like to understand why draw_text is called with a < 0 in the first place. That shouldn't happen.

@radioactiveman
Copy link
Member Author

It's because of using floating point math. I think comparing against 0 in ui_infoarea_do_fade() is not correct. Using an epsilon avoids -0.1 values.

The early return in draw_text() is still needed though to fix the emoji issue. But we can use an epsilon here as well.

diff --git a/src/gtkui/ui_infoarea.cc b/src/gtkui/ui_infoarea.cc
index c59f35a33..31a090031 100644
--- a/src/gtkui/ui_infoarea.cc
+++ b/src/gtkui/ui_infoarea.cc
@@ -17,6 +17,7 @@
  * the use of this software.
  */
 
+#include <float.h>
 #include <math.h>
 #include <string.h>
 
@@ -140,7 +141,7 @@ G_GNUC_END_IGNORE_DEPRECATIONS
 static void draw_text (GtkWidget * widget, cairo_t * cr, int x, int y, int
  width, float r, float g, float b, float a, int font_size, const char * text)
 {
-    if (a < 0)
+    if (a <= FLT_EPSILON)
         return;
 
     cairo_move_to (cr, x, y);
@@ -285,7 +286,7 @@ static void ui_infoarea_do_fade (void *)
         done = false;
     }
 
-    if (area->last_alpha > 0)
+    if (area->last_alpha > FLT_EPSILON)
     {
         area->last_alpha -= 0.1;
         done = false;

@jlindgren90
Copy link
Member

How about this? jlindgren90@f754d97

@radioactiveman
Copy link
Member Author

radioactiveman commented Oct 18, 2024

LGTM and it's also more like the code for qtui. Minor suggestion: Use ALPHA_STEPS also in TO_ALPHA().

However It does not fix the issue. Can you reproduce it?
Such special characters can be found for example here: https://emojipedia.org
The issue is that the alpha channel has no effect on them. Even with a zero value they are still visible.

Edit: Screenshot and sample code:

Screenshot

#include <gtk/gtk.h>

static void draw_text (GtkWidget *widget, cairo_t *cr, int x, int y,
                       float r, float g, float b, float a, const char *text)
{
    //if (a <= 0)
    //    return;

    cairo_move_to (cr, x, y);
    cairo_set_source_rgba (cr, r, g, b, a);

    PangoLayout * layout = gtk_widget_create_pango_layout (widget, text);
    pango_cairo_show_layout (cr, layout);
    g_object_unref (layout);
}

static gboolean draw_cb (GtkWidget *widget, cairo_t *cr)
{
    draw_text (widget, cr, 10, 10, 0, 0, 0, 1.0, "Alpha 1.0: 😄");
    draw_text (widget, cr, 10, 30, 0, 0, 0, 0.8, "Alpha 0.8: 😄");
    draw_text (widget, cr, 10, 50, 0, 0, 0, 0.6, "Alpha 0.6: 😄");
    draw_text (widget, cr, 10, 70, 0, 0, 0, 0.4, "Alpha 0.4: 😄");
    draw_text (widget, cr, 10, 90, 0, 0, 0, 0.2, "Alpha 0.2: 😄");
    draw_text (widget, cr, 10, 110, 0, 0, 0, 0.0, "Alpha 0.0: 😄");

    return TRUE;
}

int main (int argc, char *argv[])
{
    GtkWidget *window, *drawing_area;

    gtk_init (&argc, &argv);

    window = gtk_window_new (GTK_WINDOW_TOPLEVEL);
    gtk_widget_set_size_request (window, 240, 140);

    drawing_area = gtk_drawing_area_new ();
    gtk_container_add (GTK_CONTAINER (window), drawing_area);

    g_signal_connect (drawing_area, "draw", G_CALLBACK (draw_cb), NULL);
    g_signal_connect (window, "destroy", G_CALLBACK (gtk_main_quit), NULL);

    gtk_widget_show_all (window);

    gtk_main ();

    return 0;
}

@jlindgren90
Copy link
Member

Interesting, let's add a zero check as a workaround like you suggested then.

jlindgren90 and others added 2 commits October 21, 2024 18:03
Setting the font size and text directly is cleaner and more efficient.
@radioactiveman
Copy link
Member Author

Updated accordingly and added a comment. I included also your patch.

Fading out text with emoji characters can lead to them still
being displayed above the text of a new track. Ignoring zero
alpha channels avoids this.

Reproduced on Ubuntu 24.04. Arch Linux requires "noto-fonts-emoji".
@jlindgren90
Copy link
Member

LGTM, thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants