Page MenuHome

BLF: Consolidate Text Output / Processing
Needs RevisionPublic

Authored by Harley Acheson (harley) on Jul 14 2022, 10:47 PM.

Details

Summary

Simplify code by sharing common parts for text output, measuring, glyph
testing, and wrapping.


We have many nearly-identical functions that process a utf-8 string, convert to utf-32, finds matching glyphs, etc. For example the measuring of output length of a string is the same process as printing that string, just without the final output but returning bounds. Similarly we have other functions that allow a callback per glyph.

Unfortunately we have to keep all these things "in sync". As we change or add complexity (like with bidirectional and complex shaping) we must ensure that all these functions do the same things, in the same order, and return identical results.

This extends the private function blf_font_draw_ex to optionally output, return bounds, and/or callback per glyph. This way it can do the work of these other functions.

A summary of how much is removed:

source/blender/blenfont/intern/blf_font.c | 187 +++++++++---------------------
1 file changed, 55 insertions(+), 132 deletions(-)

Diff Detail

Repository
rB Blender

Event Timeline

Harley Acheson (harley) requested review of this revision.Jul 14 2022, 10:47 PM
Harley Acheson (harley) created this revision.

Measuring now perfect (no need to remove the UI_text_clip_middle_ex assert).
blf_font_boundbox_foreach_glyph needs more work though.
More simplification is possible.

A bit simpler. Still issues with blf_font_boundbox_foreach_glyph_ex to work out

Figured out my blf_font_boundbox_foreach_glyph_ex problem; now working properly.

Updated to the current state of master.

Harley Acheson (harley) retitled this revision from BLF WIP: Consolidate Text Output / Processing to BLF: Consolidate Text Output / Processing.Jul 31 2022, 4:20 AM
Harley Acheson (harley) edited the summary of this revision. (Show Details)

Updated to the current state of master.

Campbell Barton (campbellbarton) requested changes to this revision.EditedAug 5 2022, 6:40 AM

I find merging drawing & bound-box calculation to make the logic more difficult to follow.

While there is some boiler-plate to loop over characters, I don't think it's so bad as-is. Compared to grouping it into one function that can do multiple things per character.

Currently the function blf_font_boundbox_ex is very straightforward and can be understood at a glance, after this commit I find the logic much less straightforward, without any significant gains.

Marking as requiring-change but would prefer to investigate reducing boiler-plate code for iterating over characters, instead of moving different functions logic into a single loop that can do different things.

This revision now requires changes to proceed.Aug 5 2022, 6:40 AM

@Campbell Barton (campbellbarton) - I don't think it's so bad as-is...

The problem to solve is that it can't stay as-is.

Right now we have very neat and tidy code because of the assumptions that are made that will have to change. We just loop over the utf-8, grab one utf-32 character at a time, use that as a codepoint to find a matching glyph, and print it.

But with bidirectional text, complex scripts, and ligatures this can't work. A string of 5 characters could turn into 4 or 6 glyphs that are unrelated to those as codepoints and they might be used in a different order. We'll have to deal with arrays of utf-32, have them reordered based on the Unicode Bidirectional Algorithm, break the result into runs of codepoints that are the same language script and direction, give that to a library that will do ligatures, substitute glyphs based on word position, and do in-word reordering based on context, etc. Then we need to loop over the resulting glyph ids (not codepoints) and print them based on supplied positions.

Obviously this makes for a lot of change, but it will be worse that we'd have to do exactly those same operations to also measure a string's length, for line wrapping, or to loop over glyph sizes, etc.

I will see what I can do, but lets keep this patch in mind as we move forward.