Merge branch 'big-glyphs' into 'master'

Big glyphs

See merge request GNOME/gtk!904

.gitlab-ci.yml

@@ -28,6 +28,7 @@ fedora-x86_64:
       - "${CI_PROJECT_DIR}/_build/report.xml"
       - "${CI_PROJECT_DIR}/_build/report.html"
       - "${CI_PROJECT_DIR}/_build/testsuite/reftests/output/*.png"
+      - "${CI_PROJECT_DIR}/_build/testsuite/gsk/compare/*/*.png"
   cache:
     key: "$CI_JOB_NAME"
     <<: *cache-paths

gsk/gl/gskglglyphcache.c

@@ -33,18 +33,22 @@ static void     glyph_cache_key_free   (gpointer      v);
 static void     glyph_cache_value_free (gpointer      v);
 
 static GskGLGlyphAtlas *
-create_atlas (GskGLGlyphCache *cache)
+create_atlas (GskGLGlyphCache *self,
+              int              width,
+              int              height)
 {
   GskGLGlyphAtlas *atlas;
 
   atlas = g_new0 (GskGLGlyphAtlas, 1);
-  atlas->width = ATLAS_SIZE;
-  atlas->height = ATLAS_SIZE;
+  atlas->width = MAX (width, ATLAS_SIZE);
+  atlas->height = MAX (height, ATLAS_SIZE);
   atlas->y0 = 1;
   atlas->y = 1;
   atlas->x = 1;
   atlas->image = NULL;
 
+  GSK_RENDERER_NOTE(self->renderer, GLYPH_CACHE, g_message ("Create atlas %d x %d", atlas->width, atlas->height));
+
   return atlas;
 }
 
@@ -70,7 +74,6 @@ gsk_gl_glyph_cache_init (GskGLGlyphCache *self,
   self->hash_table = g_hash_table_new_full (glyph_cache_hash, glyph_cache_equal,
                                             glyph_cache_key_free, glyph_cache_value_free);
   self->atlases = g_ptr_array_new_with_free_func (free_atlas);
-  g_ptr_array_add (self->atlases, create_atlas (self));
 
   self->renderer = renderer;
   self->gl_driver = gl_driver;
@@ -167,7 +170,7 @@ add_to_cache (GskGLGlyphCache  *cache,
 
   if (i == cache->atlases->len)
     {
-      atlas = create_atlas (cache);
+      atlas = create_atlas (cache, width + 2, height + 2);
       g_ptr_array_add (cache->atlases, atlas);
     }
 
@@ -187,11 +190,10 @@ add_to_cache (GskGLGlyphCache  *cache,
 #ifdef G_ENABLE_DEBUG
   if (GSK_RENDERER_DEBUG_CHECK (cache->renderer, GLYPH_CACHE))
     {
-      g_print ("Glyph cache:\n");
       for (i = 0; i < cache->atlases->len; i++)
         {
           atlas = g_ptr_array_index (cache->atlases, i);
-          g_print ("\tGskGLGlyphAtlas %d (%dx%d): %.2g%% old pixels, filled to %d, %d / %d\n",
+          g_message ("atlas %d (%dx%d): %.2g%% old pixels, filled to %d, %d / %d",
                    i, atlas->width, atlas->height,
                    100.0 * (double)atlas->old_pixels / (double)(atlas->width * atlas->height),
                    atlas->x, atlas->y0, atlas->y);
@@ -223,7 +225,7 @@ render_glyph (const GskGLGlyphAtlas *atlas,
 
   /* TODO: Give glyphs that large their own texture in the proper size. Don't
    *       put them in the atlas at all. */
-  if (surface_width > ATLAS_SIZE || surface_height > ATLAS_SIZE)
+  if (surface_width > atlas->width || surface_height > atlas->height)
     return FALSE;
 
   surface = cairo_image_surface_create (CAIRO_FORMAT_ARGB32, surface_width, surface_height);
@@ -306,15 +308,15 @@ gsk_gl_glyph_cache_lookup (GskGLGlyphCache *cache,
     {
       const guint age = cache->timestamp - value->timestamp;
 
-      if (MAX_AGE <= age && age < MAX_AGE + CHECK_INTERVAL)
+      if (MAX_AGE <= age)
         {
           GskGLGlyphAtlas *atlas = value->atlas;
 
           if (atlas)
             atlas->old_pixels -= value->draw_width * value->draw_height;
-
-          value->timestamp = cache->timestamp;
         }
+
+      value->timestamp = cache->timestamp;
     }
 
   if (create && value == NULL)
@@ -379,7 +381,7 @@ gsk_gl_glyph_cache_begin_frame (GskGLGlyphCache *self)
   GHashTableIter iter;
   GlyphCacheKey *key;
   GskGLCachedGlyph *value;
-  guint dropped = 0;
+  GHashTable *removed = g_hash_table_new (g_direct_hash, g_direct_equal);
 
   self->timestamp++;
 
@@ -419,18 +421,45 @@ gsk_gl_glyph_cache_begin_frame (GskGLGlyphCache *self)
               atlas->image->texture_id = 0;
             }
 
-          /* Remove all glyphs that point to this atlas */
-          g_hash_table_iter_init (&iter, self->hash_table);
-          while (g_hash_table_iter_next (&iter, (gpointer *)&key, (gpointer *)&value))
-            {
-              if (value->atlas == atlas)
-                g_hash_table_iter_remove (&iter);
-            }
-          /* TODO: The above loop inside this other loop could be slow... */
+          g_hash_table_add (removed, atlas);
 
           g_ptr_array_remove_index (self->atlases, i);
-        }
+       }
     }
 
-  GSK_RENDERER_NOTE(self->renderer, GLYPH_CACHE, g_message ("Dropped %d glyphs", dropped));
+  if (g_hash_table_size (removed) > 0)
+    {
+      guint dropped = 0;
+
+      /* Remove all glyphs whose atlas was removed */
+      g_hash_table_iter_init (&iter, self->hash_table);
+      while (g_hash_table_iter_next (&iter, (gpointer *)&key, (gpointer *)&value))
+        {
+          if (g_hash_table_contains (removed, value->atlas))
+            {
+              g_hash_table_iter_remove (&iter);
+              dropped++;
+            }
+        }
+
+      GSK_RENDERER_NOTE(self->renderer, GLYPH_CACHE, if (dropped > 0) g_message ("Dropped %d glyphs", dropped));
+    }
+
+  g_hash_table_unref (removed);
+
+#if 0
+  for (i = 0; i < self->atlases->len; i++)
+    {
+      GskGLGlyphAtlas *atlas = g_ptr_array_index (self->atlases, i);
+
+      if (atlas->image)
+        {
+          char *filename;
+
+          filename = g_strdup_printf ("glyphatlas%d-%ld.png", i, self->timestamp);
+          gsk_gl_image_write_to_png (atlas->image, self->gl_driver, filename);
+          g_free (filename);
+        }
+    }
+#endif
 }

testsuite/gsk/compare-render.c

@@ -4,6 +4,46 @@
 #include <stdlib.h>
 #include "reftest-compare.h"
 
+static char *arg_output_dir = NULL;
+
+static const char *
+get_output_dir (void)
+{
+  static const char *output_dir = NULL;
+  GError *error = NULL;
+
+  if (output_dir)
+    return output_dir;
+
+  if (arg_output_dir)
+    {
+      GFile *file = g_file_new_for_commandline_arg (arg_output_dir);
+      output_dir = g_file_get_path (file);
+      g_object_unref (file);
+    }
+  else
+    {
+      output_dir = g_get_tmp_dir ();
+    }
+
+  if (!g_file_test (output_dir, G_FILE_TEST_EXISTS))
+    {
+      GFile *file;
+
+      file = g_file_new_for_path (output_dir);
+      if (!g_file_make_directory_with_parents (file, NULL, &error))
+        {
+          g_error ("Failed to create output dir: %s", error->message);
+          g_error_free (error);
+          return NULL;
+        }
+
+      g_object_unref (file);
+    }
+
+  return output_dir;
+}
+
 char *
 file_replace_extension (const char *old_file,
                         const char *old_ext,
@@ -30,7 +70,7 @@ get_output_file (const char *file,
   char *result, *base;
   char *name;
 
-  dir = g_get_tmp_dir ();
+  dir = get_output_dir ();
   base = g_path_get_basename (file);
   name = file_replace_extension (base, orig_ext, new_ext);
 
@@ -67,8 +107,14 @@ deserialize_error_func (const GtkCssSection *section,
   free (section_str);
 }
 
+static const GOptionEntry options[] = {
+  { "output", 0, 0, G_OPTION_ARG_FILENAME, &arg_output_dir,
+    "Directory to save image files to", "DIR" },
+  { NULL }
+};
+
 /*
- * Arguments:
+ * Non-option arguments:
  *   1) .node file to compare
  *   2) .png file to compare the rendered .node file to
  */
@@ -85,6 +131,18 @@ main (int argc, char **argv)
   const char *node_file;
   const char *png_file;
   gboolean success = TRUE;
+  GError *error = NULL;
+  GOptionContext *context;
+
+  context = g_option_context_new ("- run GSK node tests");
+  g_option_context_add_main_entries (context, options, NULL);
+  g_option_context_set_ignore_unknown_options (context, TRUE);
+
+  if (!g_option_context_parse (context, &argc, &argv, &error))
+    {
+      g_error ("Option parsing failed: %s\n", error->message);
+      return 1;
+    }
 
   g_assert (argc == 3);
 

testsuite/gsk/compare/big-glyph.node

@@ -0,0 +1,5 @@
+text {
+  font:"Cantarell 500";
+  glyphs:"ABC";
+  offset: 0 0;
+}

testsuite/gsk/compare/huge-glyph.node

@@ -0,0 +1,5 @@
+text {
+  font:"Cantarell 800";
+  glyphs:"ABC";
+  offset: 0 0;
+}

testsuite/gsk/meson.build

@@ -18,6 +18,7 @@ node_parser = executable(
 )
 
 compare_render_tests = [
+  'big-glyph',
   'blend-normal',
   'blend-difference',
   'clip-coordinates-3d',
@@ -45,6 +46,7 @@ compare_render_tests = [
   'empty-text',
   'empty-texture',
   'empty-transform',
+  'huge-glyph',
   'opacity_clip',
   'outset_shadow_offset_both',
   'outset_shadow_offset_x',
@@ -69,7 +71,8 @@ foreach renderer : renderers
   foreach test : compare_render_tests
     if (renderer[1] == '' or not test.contains(renderer[1]))
       test(renderer[0] + ' ' + test, compare_render,
-           args: [join_paths(meson.current_source_dir(), 'compare', test + '.node'),
+           args: ['--output', join_paths(meson.current_build_dir(), 'compare', renderer[0]),
+                  join_paths(meson.current_source_dir(), 'compare', test + '.node'),
                   join_paths(meson.current_source_dir(), 'compare', test + '.png')],
            env: [ 'GIO_USE_VOLUME_MONITOR=unix',
                   'GSETTINGS_BACKEND=memory',