From 046c69c477df2e731cf427ce527f47ee850f0a1d Mon Sep 17 00:00:00 2001 From: Ed_ Date: Sat, 11 Jan 2025 20:38:43 -0500 Subject: [PATCH] Realized while writing the docs that I need to preseve non-visible glyphs in the shape. (Fixed some crashing as well) So the shaper has been adjusted along with downstream codepaths in drawlist gen pass. --- vefontcache/draw.odin | 92 +++++++++++++++++++++--------------- vefontcache/parser.odin | 4 +- vefontcache/pkg_mapping.odin | 1 + vefontcache/shaper.odin | 80 ++++++++++++++++++++----------- vefontcache/vefontcache.odin | 2 +- 5 files changed, 110 insertions(+), 69 deletions(-) diff --git a/vefontcache/draw.odin b/vefontcache/draw.odin index 9a2b403..62b5997 100644 --- a/vefontcache/draw.odin +++ b/vefontcache/draw.odin @@ -28,6 +28,7 @@ Glyph_Draw_Quad :: struct { // This is used by generate_shape_draw_list & batch_generate_glyphs_draw_list // to track relevant glyph data in soa format for pipelined processing Glyph_Pack_Entry :: struct #packed { + vis_index : i16, position : Vec2, atlas_index : i32, @@ -320,7 +321,7 @@ generate_shape_draw_list :: proc( draw_list : ^Draw_List, shape : Shaped_Text, oversized := & glyph_buffer.oversized to_cache := & glyph_buffer.to_cache cached := & glyph_buffer.cached - resize_soa_non_zero(glyph_pack, len(shape.glyph)) + resize_soa_non_zero(glyph_pack, len(shape.visible)) append_sub_pack :: #force_inline proc ( pack : ^[dynamic]i32, entry : i32 ) { @@ -332,11 +333,18 @@ generate_shape_draw_list :: proc( draw_list : ^Draw_List, shape : Shaped_Text, profile_begin("translate") for & glyph, index in glyph_pack { - glyph.position = target_position + (shape.position[index]) * target_scale + // Throughout the draw list generation vis_id will need to be used over index as + // not all glyphs or positions for the shape are visibly rendered. + vis_id := shape.visible[index] + glyph.position = target_position + (shape.position[vis_id]) * target_scale } profile_end() profile_begin("batching & segregating glyphs") + // We do any reservation up front as appending to the array's will not check. + reserve(oversized, len(shape.visible)) + reserve(to_cache, len(shape.visible)) + reserve(cached, len(shape.visible)) clear(oversized) clear(to_cache) clear(cached) @@ -344,15 +352,13 @@ generate_shape_draw_list :: proc( draw_list : ^Draw_List, shape : Shaped_Text, for & glyph, index in glyph_pack { + // atlas_lru_code, region_kind, and bounds are all 1:1 with shape.visible atlas_key := shape.atlas_lru_code[index] region_kind := shape.region_kind[index] bounds := shape.bounds[index] bounds_size_scaled := size(bounds) * font_scale - if region_kind == .None { - assert(false, "FAILED TO ASSGIN REGION") - continue - } + assert(region_kind != .None, "FAILED TO ASSGIN REGION") when ENABLE_OVERSIZED_GLYPHS { if region_kind == .E @@ -454,7 +460,11 @@ generate_shape_draw_list :: proc( draw_list : ^Draw_List, shape : Shaped_Text, The transform and draw quads are computed first (getting the math done in one spot as possible) Some of the math from to_cache pass for glyph generation was not moved over (it could be but I'm not sure its worth it) - Order: Oversized first, then to_cache, then cached. + Order : Oversized first, then to_cache, then cached. + Important: These slices store ids for glyph_pack which matches shape.visible in index. + shape.position and shape.glyph DO NOT. + + There are only two places this matters for: getting glyph shapes when doing glyph pass generation for oversized and to_cache iterations. Oversized and to_cache will both enqueue operations for rendering glyphs to the glyph buffer render target. The compute section will have operations regarding how many glyphs they may alloate before a flush must occur. @@ -589,12 +599,13 @@ batch_generate_glyphs_draw_list :: proc ( draw_list : ^Draw_List, colour.g = 1.0 colour.b = 0.0 } - // for pack_id, index in oversized { - // error : Allocator_Error - // glyph_pack[pack_id].shape, error = parser_get_glyph_shape(entry.parser_info, shape.glyph[pack_id]) - // assert(error == .None) - // assert(glyph_pack[pack_id].shape != nil) - // } + for pack_id, index in oversized { + vis_id := shape.visible[pack_id] + error : Allocator_Error + glyph_pack[pack_id].shape, error = parser_get_glyph_shape(entry.parser_info, shape.glyph[vis_id]) + assert(error == .None) + assert(glyph_pack[pack_id].shape != nil) + } for id, index in oversized { glyph := & glyph_pack[id] @@ -605,10 +616,11 @@ batch_generate_glyphs_draw_list :: proc ( draw_list : ^Draw_List, & glyph_buffer.allocated_x ) - error : Allocator_Error - glyph.shape, error = parser_get_glyph_shape(entry.parser_info, shape.glyph[id]) - assert(error == .None) - assert(len(glyph.shape) > 0) + // vis_id := shape.visible[id] + // error : Allocator_Error + // glyph.shape, error = parser_get_glyph_shape(entry.parser_info, shape.glyph[vis_id]) + // assert(error == .None) + // assert(len(glyph.shape) > 0) generate_glyph_pass_draw_list( draw_list, & glyph_buffer.shape_gen_scratch, glyph_pack[id].shape, @@ -618,8 +630,8 @@ batch_generate_glyphs_draw_list :: proc ( draw_list : ^Draw_List, glyph_pack[id].draw_transform.scale ) - assert(len(glyph.shape) > 0) - parser_free_shape(entry.parser_info, glyph.shape) + // assert(len(glyph.shape) > 0) + // parser_free_shape(entry.parser_info, glyph.shape) target_quad := & glyph_pack[id].draw_quad @@ -639,10 +651,10 @@ batch_generate_glyphs_draw_list :: proc ( draw_list : ^Draw_List, } flush_glyph_buffer_draw_list(draw_list, & glyph_buffer.draw_list, & glyph_buffer.clear_draw_list, & glyph_buffer.allocated_x) - // for pack_id, index in oversized { - // assert(glyph_pack[pack_id].shape != nil) - // parser_free_shape(entry.parser_info, glyph_pack[pack_id].shape) - // } + for pack_id, index in oversized { + assert(glyph_pack[pack_id].shape != nil) + parser_free_shape(entry.parser_info, glyph_pack[pack_id].shape) + } } profile_end() @@ -671,12 +683,13 @@ batch_generate_glyphs_draw_list :: proc ( draw_list : ^Draw_List, profile_begin("to_cache: caching to atlas") if len(to_cache) > 0 { - // for pack_id, index in to_cache { - // error : Allocator_Error - // glyph_pack[pack_id].shape, error = parser_get_glyph_shape(entry.parser_info, shape.glyph[pack_id]) - // assert(error == .None) - // assert(glyph_pack[pack_id].shape != nil) - // } + for pack_id, index in to_cache { + vis_id := shape.visible[pack_id] + error : Allocator_Error + glyph_pack[pack_id].shape, error = parser_get_glyph_shape(entry.parser_info, shape.glyph[vis_id]) + assert(error == .None) + assert(glyph_pack[pack_id].shape != nil) + } for id, index in to_cache { @@ -730,10 +743,11 @@ batch_generate_glyphs_draw_list :: proc ( draw_list : ^Draw_List, append( & glyph_buffer.clear_draw_list.calls, clear_target_region ) append( & glyph_buffer.draw_list.calls, blit_to_atlas ) - error : Allocator_Error - glyph.shape, error = parser_get_glyph_shape(entry.parser_info, shape.glyph[id]) - assert(error == .None) - assert(len(glyph.shape) > 0) + // vis_id := shape.visible[id] + // error : Allocator_Error + // glyph.shape, error = parser_get_glyph_shape(entry.parser_info, shape.glyph[vis_id]) + // assert(error == .None) + // assert(len(glyph.shape) > 0) // Render glyph to glyph render target (FBO) generate_glyph_pass_draw_list( draw_list, & glyph_buffer.shape_gen_scratch, @@ -744,15 +758,15 @@ batch_generate_glyphs_draw_list :: proc ( draw_list : ^Draw_List, glyph.draw_transform.scale ) - assert(len(glyph.shape) > 0) - parser_free_shape(entry.parser_info, glyph.shape) + // assert(len(glyph.shape) > 0) + // parser_free_shape(entry.parser_info, glyph.shape) } flush_glyph_buffer_draw_list(draw_list, & glyph_buffer.draw_list, & glyph_buffer.clear_draw_list, & glyph_buffer.allocated_x) - // for pack_id, index in to_cache { - // assert(glyph_pack[pack_id].shape != nil) - // parser_free_shape(entry.parser_info, glyph_pack[pack_id].shape) - // } + for pack_id, index in to_cache { + assert(glyph_pack[pack_id].shape != nil) + parser_free_shape(entry.parser_info, glyph_pack[pack_id].shape) + } profile_begin("gen_cached_draw_list: to_cache") when ENABLE_DRAW_TYPE_VISUALIZATION { diff --git a/vefontcache/parser.odin b/vefontcache/parser.odin index 433acb3..9223baf 100644 --- a/vefontcache/parser.odin +++ b/vefontcache/parser.odin @@ -74,7 +74,9 @@ parser_stbtt_allocator_proc :: proc( assert(error == .None) if type == .Alloc || type == .Resize { - return transmute(rawptr) & result[0] + raw := transmute(Raw_Slice) result + // assert(raw.len > 0, "Allocation is 0 bytes?") + return transmute(rawptr) raw.data } else do return nil } diff --git a/vefontcache/pkg_mapping.odin b/vefontcache/pkg_mapping.odin index fd884dc..acf9be7 100644 --- a/vefontcache/pkg_mapping.odin +++ b/vefontcache/pkg_mapping.odin @@ -5,6 +5,7 @@ import "base:builtin" import "base:runtime" Raw_Dynamic_Array :: runtime.Raw_Dynamic_Array Raw_Map :: runtime.Raw_Map + Raw_Slice :: runtime.Raw_Slice raw_soa_footer :: runtime.raw_soa_footer nil_allocator :: runtime.nil_allocator import "core:hash" diff --git a/vefontcache/shaper.odin b/vefontcache/shaper.odin index a407ecf..f609b4d 100644 --- a/vefontcache/shaper.odin +++ b/vefontcache/shaper.odin @@ -27,6 +27,7 @@ Shape_Key :: u32 Shaped_Text :: struct #packed { glyph : [dynamic]Glyph, position : [dynamic]Vec2, + visible : [dynamic]i32, atlas_lru_code : [dynamic]Atlas_Key, region_kind : [dynamic]Atlas_Region_Kind, bounds : [dynamic]Range2, @@ -100,7 +101,7 @@ shaper_load_font :: #force_inline proc( ctx : ^Shaper_Context, label : string, d shaper_unload_font :: #force_inline proc( info : ^Shaper_Info ) { - if info.blob != nil do harfbuzz.font_destroy( info.font ) + if info.font != nil do harfbuzz.font_destroy( info.font ) if info.face != nil do harfbuzz.face_destroy( info.face ) if info.blob != nil do harfbuzz.blob_destroy( info.blob ) } @@ -125,6 +126,7 @@ shaper_shape_harfbuzz :: proc( ctx : ^Shaper_Context, clear( & output.glyph ) clear( & output.position ) + clear( & output.visible ) current_script := harfbuzz.Script.UNKNOWN hb_ucfunc := harfbuzz.unicode_funcs_get_default() @@ -210,10 +212,15 @@ shaper_shape_harfbuzz :: proc( ctx : ^Shaper_Context, (position^) += advance (max_line_width^) = max(max_line_width^, position.x) - is_empty := parser_is_glyph_empty(entry.parser_info, glyph) - if ! is_empty && glyph != 0 { - append( & output.glyph, glyph ) - append( & output.position, glyph_pos) + // We track all glyphs so that user can use the shape for navigation purposes. + append( & output.glyph, glyph ) + append( & output.position, glyph_pos) + + // We don't accept all glyphs for rendering, harfbuzz preserves positions of non-visible codepoints (as .notdef glyphs) + // We also double check to make sure the glyph isn't detected for drawing by the parser. + visible_glyph := glyph != 0 && ! parser_is_glyph_empty(entry.parser_info, glyph) + if visible_glyph { + append( & output.visible, cast(i32) len(output.glyph) - 1 ) } } @@ -234,17 +241,23 @@ shaper_shape_harfbuzz :: proc( ctx : ^Shaper_Context, // Can we continue the current run? ScriptKind :: harfbuzz.Script - special_script : b32 = script == ScriptKind.UNKNOWN || script == ScriptKind.INHERITED || script == ScriptKind.COMMON - if special_script \ + // These scripts don't break runs because they don't represent script transitions - they adapt to their context. + // Maintaining the current shaping run for these scripts ensures correct processing of marks, numbers, + // and punctuation within the primary text flow. + is_neutral_script := script == ScriptKind.UNKNOWN || script == ScriptKind.INHERITED || script == ScriptKind.COMMON + + // Essentially if the script is neutral, or the same as current, + // or this is the first codepoint: add it to the buffer and continue the loop. + if is_neutral_script \ || script == current_script \ || byte_offset == 0 { harfbuzz.buffer_add( ctx.hb_buffer, hb_codepoint, codepoint == '\n' ? 1 : 0 ) - current_script = special_script ? current_script : script + current_script = is_neutral_script ? current_script : script continue } - // End current run since we've encountered a script change. + // End current run since we've encountred a significant script change. shape_run( output, entry, ctx.hb_buffer, @@ -281,22 +294,26 @@ shaper_shape_harfbuzz :: proc( ctx : ^Shaper_Context, // Resolve each glyphs: bounds, atlas lru, and the atlas region as we have everything we need now. - resize( & output.atlas_lru_code, len(output.glyph) ) - resize( & output.region_kind, len(output.glyph) ) - resize( & output.bounds, len(output.glyph) ) + resize( & output.atlas_lru_code, len(output.visible) ) + resize( & output.region_kind, len(output.visible) ) + resize( & output.bounds, len(output.visible) ) profile_begin("atlas_lru_code") - for id, index in output.glyph { - output.atlas_lru_code[index] = atlas_glyph_lru_code(entry.id, font_px_size, id) + for vis_id, index in output.visible { + glyph_id := output.glyph[vis_id] + output.atlas_lru_code[index] = atlas_glyph_lru_code(entry.id, font_px_size, glyph_id) + // atlas_lru_code is 1:1 with visible index } profile_end() profile_begin("bounds & region") - for id, index in output.glyph { + for vis_id, index in output.visible { + glyph_id := output.glyph[vis_id] bounds := & output.bounds[index] - (bounds ^) = parser_get_bounds( entry.parser_info, id ) + (bounds ^) = parser_get_bounds( entry.parser_info, glyph_id ) bounds_size_scaled := (bounds.p1 - bounds.p0) * font_scale output.region_kind[index] = atlas_decide_region( atlas, glyph_buffer_size, bounds_size_scaled ) + // bounds & region_kind are 1:1 with visible index } profile_end() @@ -323,6 +340,7 @@ shaper_shape_text_latin :: proc( ctx : ^Shaper_Context, clear( & output.glyph ) clear( & output.position ) + clear( & output.visible ) line_height := (entry.ascent - entry.descent + entry.line_gap) * font_scale @@ -353,14 +371,16 @@ shaper_shape_text_latin :: proc( ctx : ^Shaper_Context, glyph_index := parser_find_glyph_index( entry.parser_info, codepoint ) is_glyph_empty := parser_is_glyph_empty( entry.parser_info, glyph_index ) - if ! is_glyph_empty - { - if ctx.snap_glyph_position { - position.x = ceil(position.x) - position.y = ceil(position.y) - } - append( & output.glyph, glyph_index) - append( & output.position, position) + + if ctx.snap_glyph_position { + position.x = ceil(position.x) + position.y = ceil(position.y) + } + append( & output.glyph, glyph_index) + append( & output.position, position) + + if ! is_glyph_empty { + append( & output.visible, cast(i32) len(output.glyph) - 1 ) } advance, _ := parser_get_codepoint_horizontal_metrics( entry.parser_info, codepoint ) @@ -381,17 +401,21 @@ shaper_shape_text_latin :: proc( ctx : ^Shaper_Context, resize( & output.bounds, len(output.glyph) ) profile_begin("atlas_lru_code") - for id, index in output.glyph { - output.atlas_lru_code[index] = atlas_glyph_lru_code(entry.id, font_px_size, id) + for vis_id, index in output.visible { + glyph_id := output.glyph[vis_id] + output.atlas_lru_code[index] = atlas_glyph_lru_code(entry.id, font_px_size, glyph_id) + // atlas_lru_code is 1:1 with visible index } profile_end() profile_begin("bounds & region") - for id, index in output.glyph { + for vis_id, index in output.visible { + glyph_id := output.glyph[vis_id] bounds := & output.bounds[index] - (bounds ^) = parser_get_bounds( entry.parser_info, id ) + (bounds ^) = parser_get_bounds( entry.parser_info, glyph_id ) bounds_size_scaled := (bounds.p1 - bounds.p0) * font_scale output.region_kind[index] = atlas_decide_region( atlas, glyph_buffer_size, bounds_size_scaled ) + // bounds & region_kind are 1:1 with visible index } profile_end() diff --git a/vefontcache/vefontcache.odin b/vefontcache/vefontcache.odin index c8f3168..0f65d82 100644 --- a/vefontcache/vefontcache.odin +++ b/vefontcache/vefontcache.odin @@ -581,8 +581,8 @@ unload_font :: proc( ctx : ^Context, font : Font_ID ) entry := & ctx.entries[ font ] entry.used = false - parser_unload_font( & entry.parser_info ) shaper_unload_font( & entry.shaper_info ) + parser_unload_font( & entry.parser_info ) } // Can be used with hot-reload