From cb9277847814827f0cf99d5f82239e87abb7fd0b Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Wed, 13 Mar 2024 00:18:59 -0400 Subject: [PATCH 1/4] gsk: Drop the glyph-align flag It wasn't doing anymore what it was designed for, and we are not sure that we need it. --- gsk/gpu/gskgpunodeprocessor.c | 24 ++++-------------------- gsk/gpu/gskgpurenderer.c | 1 - gsk/gpu/gskgputypesprivate.h | 3 +-- 3 files changed, 5 insertions(+), 23 deletions(-) diff --git a/gsk/gpu/gskgpunodeprocessor.c b/gsk/gpu/gskgpunodeprocessor.c index 500e37abae..8eef5d2819 100644 --- a/gsk/gpu/gskgpunodeprocessor.c +++ b/gsk/gpu/gskgpunodeprocessor.c @@ -3000,7 +3000,6 @@ gsk_gpu_node_processor_add_glyph_node (GskGpuNodeProcessor *self, guint i, num_glyphs; float scale, inv_scale; GdkRGBA color; - gboolean glyph_align; gboolean hinting; if (self->opacity < 1.0 && @@ -3024,7 +3023,6 @@ gsk_gpu_node_processor_add_glyph_node (GskGpuNodeProcessor *self, scale = MAX (graphene_vec2_get_x (&self->scale), graphene_vec2_get_y (&self->scale)); inv_scale = 1.f / scale; - glyph_align = gsk_gpu_frame_should_optimize (self->frame, GSK_GPU_OPTIMIZE_GLYPH_ALIGN); hinting = gsk_font_get_hint_style (font) != CAIRO_HINT_STYLE_NONE; for (i = 0; i < num_glyphs; i++) @@ -3038,7 +3036,7 @@ gsk_gpu_node_processor_add_glyph_node (GskGpuNodeProcessor *self, glyph_origin = GRAPHENE_POINT_INIT (offset.x + (float) glyphs[i].geometry.x_offset / PANGO_SCALE, offset.y + (float) glyphs[i].geometry.y_offset / PANGO_SCALE); - if (hinting && glyph_align) + if (hinting) { /* Force glyph_origin.y to be device pixel aligned. * The hinter expects that. @@ -3048,7 +3046,7 @@ gsk_gpu_node_processor_add_glyph_node (GskGpuNodeProcessor *self, glyph_origin.x = 0.25 * inv_scale * glyph_origin.x; glyph_origin.y = floor (glyph_origin.y * scale + .5) * inv_scale; } - else if (glyph_align) + else { glyph_origin.x = floor (glyph_origin.x * scale * 4 + .5); glyph_origin.y = floor (glyph_origin.y * scale * 4 + .5); @@ -3057,12 +3055,6 @@ gsk_gpu_node_processor_add_glyph_node (GskGpuNodeProcessor *self, glyph_origin.x = 0.25 * inv_scale * glyph_origin.x; glyph_origin.y = 0.25 * inv_scale * glyph_origin.y; } - else - { - glyph_origin.x = floor (glyph_origin.x * scale + .5) * inv_scale; - glyph_origin.y = floor (glyph_origin.y * scale + .5) * inv_scale; - flags = 0; - } image = gsk_gpu_device_lookup_glyph_image (device, self->frame, @@ -3121,7 +3113,6 @@ gsk_gpu_node_processor_create_glyph_pattern (GskGpuPatternWriter *self, guint32 tex_id; GskGpuImage *last_image; graphene_point_t offset; - gboolean glyph_align; gboolean hinting; if (gsk_text_node_has_color_glyphs (node)) @@ -3142,7 +3133,6 @@ gsk_gpu_node_processor_create_glyph_pattern (GskGpuPatternWriter *self, gsk_gpu_pattern_writer_append_rgba (self, gsk_text_node_get_color (node)); gsk_gpu_pattern_writer_append_uint (self, num_glyphs); - glyph_align = gsk_gpu_frame_should_optimize (self->frame, GSK_GPU_OPTIMIZE_GLYPH_ALIGN); hinting = gsk_font_get_hint_style (font) != CAIRO_HINT_STYLE_NONE; last_image = NULL; @@ -3156,7 +3146,7 @@ gsk_gpu_node_processor_create_glyph_pattern (GskGpuPatternWriter *self, glyph_origin = GRAPHENE_POINT_INIT (offset.x + (float) glyphs[i].geometry.x_offset / PANGO_SCALE, offset.y + (float) glyphs[i].geometry.y_offset / PANGO_SCALE); - if (hinting && glyph_align) + if (hinting) { /* Force glyph_origin.y to be device pixel aligned. * The hinter expects that. @@ -3166,7 +3156,7 @@ gsk_gpu_node_processor_create_glyph_pattern (GskGpuPatternWriter *self, glyph_origin.x = 0.25 * inv_scale * glyph_origin.x; glyph_origin.y = roundf (glyph_origin.y * scale) * inv_scale; } - else if (glyph_align) + else { glyph_origin.x = roundf (glyph_origin.x * scale * 4); glyph_origin.y = roundf (glyph_origin.y * scale * 4); @@ -3175,12 +3165,6 @@ gsk_gpu_node_processor_create_glyph_pattern (GskGpuPatternWriter *self, glyph_origin.x = 0.25 * inv_scale * glyph_origin.x; glyph_origin.y = 0.25 * inv_scale * glyph_origin.y; } - else - { - glyph_origin.x = roundf (glyph_origin.x * scale) * inv_scale; - glyph_origin.y = roundf (glyph_origin.y * scale) * inv_scale; - flags = 0; - } image = gsk_gpu_device_lookup_glyph_image (device, self->frame, diff --git a/gsk/gpu/gskgpurenderer.c b/gsk/gpu/gskgpurenderer.c index a74216eb9a..b6703f676a 100644 --- a/gsk/gpu/gskgpurenderer.c +++ b/gsk/gpu/gskgpurenderer.c @@ -30,7 +30,6 @@ static const GdkDebugKey gsk_gpu_optimization_keys[] = { { "blit", GSK_GPU_OPTIMIZE_BLIT, "Use shaders instead of vkCmdBlit()/glBlitFramebuffer()" }, { "gradients", GSK_GPU_OPTIMIZE_GRADIENTS, "Don't supersample gradients" }, { "mipmap", GSK_GPU_OPTIMIZE_MIPMAP, "Avoid creating mipmaps" }, - { "glyph-align", GSK_GPU_OPTIMIZE_GLYPH_ALIGN, "Never align glyphs to the subpixel grid" }, { "gl-baseinstance", GSK_GPU_OPTIMIZE_GL_BASE_INSTANCE, "Assume no ARB/EXT_base_instance support" }, }; diff --git a/gsk/gpu/gskgputypesprivate.h b/gsk/gpu/gskgputypesprivate.h index a3f6c1d159..55e85fccda 100644 --- a/gsk/gpu/gskgputypesprivate.h +++ b/gsk/gpu/gskgputypesprivate.h @@ -118,8 +118,7 @@ typedef enum { GSK_GPU_OPTIMIZE_BLIT = 1 << 3, GSK_GPU_OPTIMIZE_GRADIENTS = 1 << 4, GSK_GPU_OPTIMIZE_MIPMAP = 1 << 5, - GSK_GPU_OPTIMIZE_GLYPH_ALIGN = 1 << 6, /* These require hardware support */ - GSK_GPU_OPTIMIZE_GL_BASE_INSTANCE = 1 << 7, + GSK_GPU_OPTIMIZE_GL_BASE_INSTANCE = 1 << 6, } GskGpuOptimizations; From c71a66b6f692599e6f9b55d57ef038abe8ab36cd Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Tue, 12 Mar 2024 23:38:11 -0400 Subject: [PATCH 2/4] gsk: Simplify our inner loop Pull out the if-else and precompute things before the loop. --- gsk/gpu/gskgpunodeprocessor.c | 86 ++++++++++++++++++----------------- 1 file changed, 44 insertions(+), 42 deletions(-) diff --git a/gsk/gpu/gskgpunodeprocessor.c b/gsk/gpu/gskgpunodeprocessor.c index 8eef5d2819..2de3b015fd 100644 --- a/gsk/gpu/gskgpunodeprocessor.c +++ b/gsk/gpu/gskgpunodeprocessor.c @@ -3000,7 +3000,9 @@ gsk_gpu_node_processor_add_glyph_node (GskGpuNodeProcessor *self, guint i, num_glyphs; float scale, inv_scale; GdkRGBA color; - gboolean hinting; + float align_scale_x, align_scale_y; + float inv_align_scale_x, inv_align_scale_y; + unsigned int flags_mask; if (self->opacity < 1.0 && gsk_text_node_has_color_glyphs (node)) @@ -3023,7 +3025,20 @@ gsk_gpu_node_processor_add_glyph_node (GskGpuNodeProcessor *self, scale = MAX (graphene_vec2_get_x (&self->scale), graphene_vec2_get_y (&self->scale)); inv_scale = 1.f / scale; - hinting = gsk_font_get_hint_style (font) != CAIRO_HINT_STYLE_NONE; + if (gsk_font_get_hint_style (font) != CAIRO_HINT_STYLE_NONE) + { + align_scale_x = scale * 4; + align_scale_y = scale; + flags_mask = 3; + } + else + { + align_scale_x = align_scale_y = scale * 4; + flags_mask = 15; + } + + inv_align_scale_x = 1 / align_scale_x; + inv_align_scale_y = 1 / align_scale_y; for (i = 0; i < num_glyphs; i++) { @@ -3036,25 +3051,11 @@ gsk_gpu_node_processor_add_glyph_node (GskGpuNodeProcessor *self, glyph_origin = GRAPHENE_POINT_INIT (offset.x + (float) glyphs[i].geometry.x_offset / PANGO_SCALE, offset.y + (float) glyphs[i].geometry.y_offset / PANGO_SCALE); - if (hinting) - { - /* Force glyph_origin.y to be device pixel aligned. - * The hinter expects that. - */ - glyph_origin.x = floor (glyph_origin.x * scale * 4 + .5); - flags = ((int) glyph_origin.x & 3); - glyph_origin.x = 0.25 * inv_scale * glyph_origin.x; - glyph_origin.y = floor (glyph_origin.y * scale + .5) * inv_scale; - } - else - { - glyph_origin.x = floor (glyph_origin.x * scale * 4 + .5); - glyph_origin.y = floor (glyph_origin.y * scale * 4 + .5); - flags = ((int) glyph_origin.x & 3) | - (((int) glyph_origin.y & 3) << 2); - glyph_origin.x = 0.25 * inv_scale * glyph_origin.x; - glyph_origin.y = 0.25 * inv_scale * glyph_origin.y; - } + glyph_origin.x = floorf (glyph_origin.x * align_scale_x + 0.5f); + glyph_origin.y = floorf (glyph_origin.y * align_scale_y + 0.5f); + flags = (((int) glyph_origin.x & 3) | (((int) glyph_origin.y & 3) << 2)) & flags_mask; + glyph_origin.x *= inv_align_scale_x; + glyph_origin.y *= inv_align_scale_y; image = gsk_gpu_device_lookup_glyph_image (device, self->frame, @@ -3113,7 +3114,9 @@ gsk_gpu_node_processor_create_glyph_pattern (GskGpuPatternWriter *self, guint32 tex_id; GskGpuImage *last_image; graphene_point_t offset; - gboolean hinting; + float align_scale_x, align_scale_y; + float inv_align_scale_x, inv_align_scale_y; + unsigned int flags_mask; if (gsk_text_node_has_color_glyphs (node)) return FALSE; @@ -3133,7 +3136,20 @@ gsk_gpu_node_processor_create_glyph_pattern (GskGpuPatternWriter *self, gsk_gpu_pattern_writer_append_rgba (self, gsk_text_node_get_color (node)); gsk_gpu_pattern_writer_append_uint (self, num_glyphs); - hinting = gsk_font_get_hint_style (font) != CAIRO_HINT_STYLE_NONE; + if (gsk_font_get_hint_style (font) != CAIRO_HINT_STYLE_NONE) + { + align_scale_x = scale * 4; + align_scale_y = scale; + flags_mask = 3; + } + else + { + align_scale_x = align_scale_y = scale * 4; + flags_mask = 15; + } + + inv_align_scale_x = 1 / align_scale_x; + inv_align_scale_y = 1 / align_scale_y; last_image = NULL; for (i = 0; i < num_glyphs; i++) @@ -3146,25 +3162,11 @@ gsk_gpu_node_processor_create_glyph_pattern (GskGpuPatternWriter *self, glyph_origin = GRAPHENE_POINT_INIT (offset.x + (float) glyphs[i].geometry.x_offset / PANGO_SCALE, offset.y + (float) glyphs[i].geometry.y_offset / PANGO_SCALE); - if (hinting) - { - /* Force glyph_origin.y to be device pixel aligned. - * The hinter expects that. - */ - glyph_origin.x = roundf (glyph_origin.x * scale * 4); - flags = ((int) glyph_origin.x & 3); - glyph_origin.x = 0.25 * inv_scale * glyph_origin.x; - glyph_origin.y = roundf (glyph_origin.y * scale) * inv_scale; - } - else - { - glyph_origin.x = roundf (glyph_origin.x * scale * 4); - glyph_origin.y = roundf (glyph_origin.y * scale * 4); - flags = ((int) glyph_origin.x & 3) | - (((int) glyph_origin.y & 3) << 2); - glyph_origin.x = 0.25 * inv_scale * glyph_origin.x; - glyph_origin.y = 0.25 * inv_scale * glyph_origin.y; - } + glyph_origin.x = floorf (glyph_origin.x * align_scale_x + 0.5f); + glyph_origin.y = floorf (glyph_origin.y * align_scale_y + 0.5f); + flags = (((int) glyph_origin.x & 3) | (((int) glyph_origin.y & 3) << 2)) & flags_mask; + glyph_origin.x *= inv_align_scale_x; + glyph_origin.y *= inv_align_scale_y; image = gsk_gpu_device_lookup_glyph_image (device, self->frame, From 2fda256bb01b974d2f54f98a2da5af4ed2b305c1 Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Wed, 13 Mar 2024 00:37:13 -0400 Subject: [PATCH 3/4] gsk: Avoid some unnecessary calls Most of the time, the image we get for the glyphs will be the same (the atlas), so avoid adding it to the descriptor set over and over, and check first if have to. This matches what the pattern variant of this function already does. --- gsk/gpu/gskgpunodeprocessor.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/gsk/gpu/gskgpunodeprocessor.c b/gsk/gpu/gskgpunodeprocessor.c index 2de3b015fd..2165bb852f 100644 --- a/gsk/gpu/gskgpunodeprocessor.c +++ b/gsk/gpu/gskgpunodeprocessor.c @@ -3003,6 +3003,8 @@ gsk_gpu_node_processor_add_glyph_node (GskGpuNodeProcessor *self, float align_scale_x, align_scale_y; float inv_align_scale_x, inv_align_scale_y; unsigned int flags_mask; + GskGpuImage *last_image; + guint32 descriptor; if (self->opacity < 1.0 && gsk_text_node_has_color_glyphs (node)) @@ -3040,12 +3042,13 @@ gsk_gpu_node_processor_add_glyph_node (GskGpuNodeProcessor *self, inv_align_scale_x = 1 / align_scale_x; inv_align_scale_y = 1 / align_scale_y; + last_image = NULL; + descriptor = 0; for (i = 0; i < num_glyphs; i++) { GskGpuImage *image; graphene_rect_t glyph_bounds, glyph_tex_rect; graphene_point_t glyph_offset, glyph_origin; - guint32 descriptor; GskGpuGlyphLookupFlags flags; glyph_origin = GRAPHENE_POINT_INIT (offset.x + (float) glyphs[i].geometry.x_offset / PANGO_SCALE, @@ -3077,7 +3080,11 @@ gsk_gpu_node_processor_add_glyph_node (GskGpuNodeProcessor *self, glyph_origin = GRAPHENE_POINT_INIT (glyph_origin.x - glyph_offset.x * inv_scale, glyph_origin.y - glyph_offset.y * inv_scale); - descriptor = gsk_gpu_node_processor_add_image (self, image, GSK_GPU_SAMPLER_DEFAULT); + if (image != last_image) + { + descriptor = gsk_gpu_node_processor_add_image (self, image, GSK_GPU_SAMPLER_DEFAULT); + last_image = image; + } if (glyphs[i].attr.is_color) gsk_gpu_texture_op (self->frame, From 380523b41bb2334eae90c2b333d15918d91f9372 Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Wed, 13 Mar 2024 00:56:38 -0400 Subject: [PATCH 4/4] gsk: Eschew more divisions Pull out a pango_scale_inv constant, and use it. --- gsk/gpu/gskgpunodeprocessor.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/gsk/gpu/gskgpunodeprocessor.c b/gsk/gpu/gskgpunodeprocessor.c index 2165bb852f..03308e1500 100644 --- a/gsk/gpu/gskgpunodeprocessor.c +++ b/gsk/gpu/gskgpunodeprocessor.c @@ -3005,6 +3005,7 @@ gsk_gpu_node_processor_add_glyph_node (GskGpuNodeProcessor *self, unsigned int flags_mask; GskGpuImage *last_image; guint32 descriptor; + const float inv_pango_scale = 1.f / PANGO_SCALE; if (self->opacity < 1.0 && gsk_text_node_has_color_glyphs (node)) @@ -3051,8 +3052,8 @@ gsk_gpu_node_processor_add_glyph_node (GskGpuNodeProcessor *self, graphene_point_t glyph_offset, glyph_origin; GskGpuGlyphLookupFlags flags; - glyph_origin = GRAPHENE_POINT_INIT (offset.x + (float) glyphs[i].geometry.x_offset / PANGO_SCALE, - offset.y + (float) glyphs[i].geometry.y_offset / PANGO_SCALE); + glyph_origin = GRAPHENE_POINT_INIT (offset.x + glyphs[i].geometry.x_offset * inv_pango_scale, + offset.y + glyphs[i].geometry.y_offset * inv_pango_scale); glyph_origin.x = floorf (glyph_origin.x * align_scale_x + 0.5f); glyph_origin.y = floorf (glyph_origin.y * align_scale_y + 0.5f); @@ -3104,7 +3105,7 @@ gsk_gpu_node_processor_add_glyph_node (GskGpuNodeProcessor *self, &glyph_tex_rect, &color); - offset.x += (float) glyphs[i].geometry.width / PANGO_SCALE; + offset.x += glyphs[i].geometry.width * inv_pango_scale; } } @@ -3124,6 +3125,7 @@ gsk_gpu_node_processor_create_glyph_pattern (GskGpuPatternWriter *self, float align_scale_x, align_scale_y; float inv_align_scale_x, inv_align_scale_y; unsigned int flags_mask; + const float inv_pango_scale = 1.f / PANGO_SCALE; if (gsk_text_node_has_color_glyphs (node)) return FALSE; @@ -3166,8 +3168,8 @@ gsk_gpu_node_processor_create_glyph_pattern (GskGpuPatternWriter *self, graphene_point_t glyph_offset, glyph_origin; GskGpuGlyphLookupFlags flags; - glyph_origin = GRAPHENE_POINT_INIT (offset.x + (float) glyphs[i].geometry.x_offset / PANGO_SCALE, - offset.y + (float) glyphs[i].geometry.y_offset / PANGO_SCALE); + glyph_origin = GRAPHENE_POINT_INIT (offset.x + glyphs[i].geometry.x_offset * inv_pango_scale, + offset.y + glyphs[i].geometry.y_offset * inv_pango_scale); glyph_origin.x = floorf (glyph_origin.x * align_scale_x + 0.5f); glyph_origin.y = floorf (glyph_origin.y * align_scale_y + 0.5f); @@ -3213,7 +3215,7 @@ gsk_gpu_node_processor_create_glyph_pattern (GskGpuPatternWriter *self, ), &glyph_origin); - offset.x += (float) glyphs[i].geometry.width / PANGO_SCALE; + offset.x += glyphs[i].geometry.width * inv_pango_scale; } return TRUE;