blender
/
blender
136
407
Fork 589
Code Issues 6.3k Pull Requests 780 Projects 19 Wiki Activity

BLF: Utility to Wrap a String into Multiple Lines #118436

Merged
Harley Acheson merged 7 commits from Harley/blender:WrapString into main 2024-02-21 01:19:14 +01:00
Conversation 6 Commits 7 Files Changed 4 +62 -4
4 changed files with 62 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
source/blender/blenfont/BLF_api.hh
View File

@ -9,7 +9,9 @@
#pragma once
#include "BLI_compiler_attrs.h"
Harley marked this conversation as resolved Outdated
Hans Goudey commented 2024-02-20 16:44:39 +01:00
Outdated
Review
Copy Link

Unnecessary include now :)

Unnecessary include now :)
#include "BLI_string_ref.hh"
#include "BLI_sys_types.h"
#include "BLI_vector.hh"
/* Name of sub-directory inside #BLENDER_DATAFILES that contains font files. */
#define BLF_DATAFILES_FONTS_DIR "fonts"
@ -254,6 +256,10 @@ void BLF_rotation(int fontid, float angle);
void BLF_clipping(int fontid, int xmin, int ymin, int xmax, int ymax);
void BLF_wordwrap(int fontid, int wrap_width);
blender::Vector<blender::StringRef> BLF_string_wrap(int fontid,
blender::StringRef str,
const int max_pixel_width);
#if BLF_BLUR_ENABLE
void BLF_blur(int fontid, int size);
#endif

11
source/blender/blenfont/intern/blf.cc
View File

@ -935,6 +935,17 @@ void BLF_draw_buffer(int fontid, const char *str, const size_t str_len)
BLF_draw_buffer_ex(fontid, str, str_len, nullptr);
}
blender::Vector<blender::StringRef> BLF_string_wrap(int fontid,
blender::StringRef str,
const int max_pixel_width)
{
FontBLF *font = blf_get(fontid);
Harley marked this conversation as resolved
Hans Goudey commented 2024-02-19 01:43:15 +01:00
Review
Copy Link

Flip the check and return {} inside. It's nice to consistently put error handling at the start of the function

Flip the check and `return {}` inside. It's nice to consistently put error handling at the start of the function
if (!font) {
return {};
}
return blf_font_string_wrap(font, str, max_pixel_width);
}
char *BLF_display_name_from_file(const char *filepath)
{
/* While listing font directories this function can be called simultaneously from a greater

42
source/blender/blenfont/intern/blf_font.cc
View File

@ -37,6 +37,7 @@
#include "BLI_string_cursor_utf8.h"
#include "BLI_string_utf8.h"
#include "BLI_threads.h"
#include "BLI_vector.hh"
#include "BLF_api.hh"
@ -1085,6 +1086,7 @@ void blf_str_offset_to_glyph_bounds(FontBLF *font,
static void blf_font_wrap_apply(FontBLF *font,
const char *str,
const size_t str_len,
const int max_pixel_width,
ResultBLF *r_info,
void (*callback)(FontBLF *font,
GlyphCacheBLF *gc,
@ -1109,7 +1111,7 @@ static void blf_font_wrap_apply(FontBLF *font,
struct WordWrapVars {
ft_pix wrap_width;
size_t start, last[2];
} wrap = {font->wrap_width != -1 ? ft_pix_from_int(font->wrap_width) : INT_MAX, 0, {0, 0}};
} wrap = {max_pixel_width != -1 ? ft_pix_from_int(max_pixel_width) : INT_MAX, 0, {0, 0}};
// printf("%s wrapping (%d, %d) `%s`:\n", __func__, str_len, strlen(str), str);
while ((i < str_len) && str[i]) {
@ -1198,7 +1200,8 @@ static void blf_font_draw__wrap_cb(FontBLF *font,
}
void blf_font_draw__wrap(FontBLF *font, const char *str, const size_t str_len, ResultBLF *r_info)
{
blf_font_wrap_apply(font, str, str_len, r_info, blf_font_draw__wrap_cb, nullptr);
blf_font_wrap_apply(
font, str, str_len, font->wrap_width, r_info, blf_font_draw__wrap_cb, nullptr);
}
/** Utility for #blf_font_boundbox__wrap. */
@ -1223,7 +1226,8 @@ void blf_font_boundbox__wrap(
box->ymin = 32000;
box->ymax = -32000;
blf_font_wrap_apply(font, str, str_len, r_info, blf_font_boundbox_wrap_cb, box);
blf_font_wrap_apply(
font, str, str_len, font->wrap_width, r_info, blf_font_boundbox_wrap_cb, box);
}
/** Utility for #blf_font_draw_buffer__wrap. */
@ -1241,7 +1245,37 @@ void blf_font_draw_buffer__wrap(FontBLF *font,
const size_t str_len,
ResultBLF *r_info)
{
blf_font_wrap_apply(font, str, str_len, r_info, blf_font_draw_buffer__wrap_cb, nullptr);
blf_font_wrap_apply(
font, str, str_len, font->wrap_width, r_info, blf_font_draw_buffer__wrap_cb, nullptr);
}
/** Wrap a blender::StringRef. */
static void blf_font_string_wrap_cb(FontBLF * /*font*/,
GlyphCacheBLF * /*gc*/,
const char *str,
const size_t str_len,
ft_pix /*pen_y*/,
Harley marked this conversation as resolved Outdated
Hans Goudey commented 2024-02-20 03:20:49 +01:00
Outdated
Review
Copy Link

Unused variable pen_y? and gc?

Unused variable `pen_y`? and `gc`?
void *str_list_ptr)
Harley marked this conversation as resolved
Hans Goudey commented 2024-02-19 01:47:16 +01:00
Review
Copy Link

Just as an example, these two lines do a fair amount of expensive things:

  • First is the line variable. This has to allocate space for str_len and copy the characters
  • Next is an empty string as the vector increases in size. This is cheap, but worth keeping in mind (
  • Finally, putting the sliced string into the vector. Copy assignment allocates new characters for the string in the vector, then copies from line. With std::vector, emplace_back is used to construct the new value in place rather than default constructing it then filling its value with copy assignment. C++ also has std::move for this reason: line is just temporary data, we can move it into the vector to avoid the copy
Just as an example, these two lines do a fair amount of expensive things: - First is the `line` variable. This has to allocate space for `str_len` and copy the characters - Next is an empty string as the vector increases in size. This is cheap, but worth keeping in mind ( - Finally, putting the sliced string into the vector. Copy assignment allocates new characters for the string in the vector, then copies from `line`. With `std::vector`, `emplace_back` is used to construct the new value in place rather than default constructing it then filling its value with copy assignment. C++ also has `std::move` for this reason: `line` is just temporary data, we can move it into the vector to avoid the copy
{
blender::Vector<blender::StringRef> *list = static_cast<blender::Vector<blender::StringRef> *>(
str_list_ptr);
blender::StringRef line(str, str + str_len);
list->append(line);
}
Harley marked this conversation as resolved Outdated
Hans Goudey commented 2024-02-20 03:20:21 +01:00
Outdated
Review
Copy Link

To be clear, I meant this should return Vector<StringRef>. Then this function doesn't need to do any allocation or copying at all. The only requirement is that the lifetime of the input string is at least as long as the lifetime of the output. But that's a nice tradeoff.

To be clear, I meant this should return `Vector<StringRef>`. Then this function doesn't need to do any allocation or copying at all. The only requirement is that the lifetime of the input string is at least as long as the lifetime of the output. But that's a nice tradeoff.
blender::Vector<blender::StringRef> blf_font_string_wrap(FontBLF *font,
blender::StringRef str,
int max_pixel_width)
{
blender::Vector<blender::StringRef> list;
blf_font_wrap_apply(font,
str.data(),
size_t(str.size()),
max_pixel_width,
nullptr,
blf_font_string_wrap_cb,
&list);
return list;
}
/** \} */

7
source/blender/blenfont/intern/blf_internal.hh
View File

@ -8,6 +8,9 @@
#pragma once
#include "BLI_string_ref.hh"
Harley marked this conversation as resolved Outdated
Hans Goudey commented 2024-02-20 16:44:55 +01:00
Outdated
Review
Copy Link

And here too

And here too
#include "BLI_vector.hh"
struct FontBLF;
struct GlyphBLF;
struct GlyphCacheBLF;
@ -96,6 +99,10 @@ void blf_font_draw__wrap(struct FontBLF *font,
size_t str_len,
struct ResultBLF *r_info);
blender::Vector<blender::StringRef> blf_font_string_wrap(FontBLF *font,
blender::StringRef str,
int max_pixel_width);
Harley marked this conversation as resolved Outdated
Hans Goudey commented 2024-02-19 01:37:26 +01:00
Outdated
Review
Copy Link

const is meaningless in declarations for arguments passed by value

`const` is meaningless in declarations for arguments passed by value
/**
Harley marked this conversation as resolved Outdated
Hans Goudey commented 2024-02-20 03:20:18 +01:00
Outdated
Review
Copy Link

const int means the same thing as int here in the declaration, so typically const isn't included here.

`const int` means the same thing as `int` here in the declaration, so typically `const` isn't included here.
* Use fixed column width, but an utf8 character may occupy multiple columns.
*/