From 7412e0b6c4d77441cc455d402a16fd747fbc3c22 Mon Sep 17 00:00:00 2001 From: Goober5000 Date: Sun, 19 Oct 2025 20:45:17 -0400 Subject: [PATCH] refactor split_str_once Rewrite `split_str_once` to be more robust, to handle Unicode, and to return length and position information rather than modifying the source string, taking the approach also used by `str_wrap_to_width`. In combination with the new length variants of common string utility functions, this allows strings to be split without copying them. In many cases, using the old design, strings were copied multiple times per frame in numerous places. Includes unit tests to handle numerous usual and unusual cases. All of the historical string splitting anomalies should now be fixed, including a bug in the retail algorithm where an exactly matching string followed by whitespace wasn't split properly. A followup PR will refactor `split_str`. --- code/hud/hudmessage.cpp | 113 ++++---- code/hud/hudmessage.h | 2 +- code/mission/missionlog.cpp | 92 ++++--- code/mission/missiontraining.cpp | 39 ++- code/missionui/missiondebrief.cpp | 19 +- code/parse/parselo.cpp | 370 ++++++++------------------- code/parse/parselo.h | 6 +- code/scripting/api/libs/graphics.cpp | 22 +- test/src/parse/test_split_str.cpp | 279 ++++++++++++++++++++ test/src/source_groups.cmake | 1 + 10 files changed, 522 insertions(+), 421 deletions(-) create mode 100644 test/src/parse/test_split_str.cpp diff --git a/code/hud/hudmessage.cpp b/code/hud/hudmessage.cpp index 582d44d6d1b..b6c8cec2249 100644 --- a/code/hud/hudmessage.cpp +++ b/code/hud/hudmessage.cpp @@ -104,7 +104,6 @@ static int Hud_mission_log_time2_coords[GR_NUM_RESOLUTIONS][2] = { #define SHOW_OBJS_BUTTON 4 #define ACCEPT_BUTTON 5 -#define HUD_MSG_LENGTH_MAX 2048 //#define HUD_MSG_MAX_PIXEL_W 439 // maximum number of pixels wide message display area is //#define HUD_MSG_MAX_PIXEL_W 619 // maximum number of pixels wide message display area is @@ -293,47 +292,30 @@ void HudGaugeMessages::pageIn() void HudGaugeMessages::processMessageBuffer() { - int x, offset = 0; - size_t i; - char *msg; - char *split_str, *ptr; - - for ( i = 0; i < HUD_msg_buffer.size(); i++ ) { - msg = new char [HUD_msg_buffer[i].text.size()+1]; - strcpy(msg, HUD_msg_buffer[i].text.c_str()); - - ptr = strstr(msg, NOX(": ")); - if ( ptr ) { - int sw; - gr_get_string_size(&sw, nullptr, msg, 1.0f, (ptr + 2 - msg)); - offset = sw; - } - - x = 0; - split_str = msg; - - while ((ptr = split_str_once(split_str, Max_width - x - 7)) != nullptr) { // the 7 is a fudge hack - // make sure that split went ok, if not then bail - if (ptr == split_str) { - break; - } - - addPending(split_str, HUD_msg_buffer[i].source, x); - split_str = ptr; + for (auto &hud_msg: HUD_msg_buffer) + { + auto msg = hud_msg.text.c_str(); + + int x = 0, offset = 0; + auto ptr = strstr(msg, NOX(": ")); + if (ptr) + gr_get_string_size(&offset, nullptr, msg, 1.0f, static_cast(ptr + 2 - msg)); + + size_t split_len, split_next_pos; + do { + std::tie(split_len, split_next_pos, std::ignore) = split_str_once(msg, Max_width - x - 7); // the 7 is a fudge hack + addPending(msg, split_len, hud_msg.source, x); + msg += split_next_pos; x = offset; - } - - addPending(split_str, HUD_msg_buffer[i].source, x); - - delete[] msg; + } while (split_next_pos > 0); } } -void HudGaugeMessages::addPending(const char *text, int source, int x) +void HudGaugeMessages::addPending(const char *text, size_t len, int source, int x) { Assert(text != nullptr); - pending_messages.emplace(text, source, x); + pending_messages.emplace(SCP_string(text, len), source, x); } void HudGaugeMessages::scrollMessages() @@ -829,50 +811,42 @@ void hud_scrollback_button_pressed(int n) // scroll height of the scrollback UI. void hud_initialize_scrollback_lines() { - Msg_scrollback_lines.clear(); - if ((Msg_scrollback_vec.size() > 0) && HUD_msg_inited) { - - for (int j = 0; j < (int)Msg_scrollback_vec.size(); j++) { - line_node node_msg = Msg_scrollback_vec[j]; - - int width = 0; - int height = 0; - gr_get_string_size(&width, &height, node_msg.text.c_str(), 1.0f, node_msg.text.length()); + if (!Msg_scrollback_vec.empty() && HUD_msg_inited) { + for (auto &node_msg: Msg_scrollback_vec) { + auto text = node_msg.text.c_str(); int max_width = Hud_mission_log_list2_coords[gr_screen.res][2]; - if (width > max_width) { - char c_text[HUD_MSG_LENGTH_MAX]; - strcpy_s(c_text, node_msg.text.c_str()); - - char* text = c_text; - - char* split = split_str_once(text, max_width); - Msg_scrollback_lines.push_back({node_msg.time, The_mission.HUD_timer_padding, node_msg.source, node_msg.x, 1, node_msg.underline_width, text}); - - while (split != nullptr) { - text = split; - split = nullptr; - split = split_str_once(text, max_width); - - int offset = 1; - if (split == nullptr) - offset = height / 3; - Msg_scrollback_lines.push_back({0, 0, node_msg.source, node_msg.x, offset, 0, text}); - } - } else { - node_msg.y = height / 3; - Msg_scrollback_lines.push_back(std::move(node_msg)); - } + int height = 0; + bool first = true; + size_t start_pos = 0; + size_t split_len, split_next_pos; + + do { + std::tie(split_len, split_next_pos, std::ignore) = split_str_once(text, max_width, std::string::npos, 1.0f, nullptr, &height); + + Msg_scrollback_lines.emplace_back( + first ? node_msg.time : 0, + first ? node_msg.timer_padding : 0, + node_msg.source, + node_msg.x, + split_next_pos > 0 ? 1 : height / 3, // y coordinate is 1 for every line except the last one + first ? node_msg.underline_width : 0, + node_msg.text.substr(start_pos, split_len) + ); + + text += split_next_pos; + start_pos += split_next_pos; + first = false; + } while (split_next_pos > 0); } } } void hud_scrollback_init() { - // pause all game sounds weapon_pause_sounds(); audiostream_pause_all(); @@ -1050,8 +1024,7 @@ void hud_scrollback_do_frame(float /*frametime*/) int y = 0; if (!Msg_scrollback_lines.empty() && HUD_msg_inited) { int i = 0; - for (int j = 0; j < (int)Msg_scrollback_lines.size(); j++) { - line_node node_msg = Msg_scrollback_lines[j]; + for (auto &node_msg: Msg_scrollback_lines) { if ((node_msg.source == HUD_SOURCE_HIDDEN) || (i++ < Scroll_offset)) { continue; diff --git a/code/hud/hudmessage.h b/code/hud/hudmessage.h index 6ce2cdfb053..d32d267198e 100644 --- a/code/hud/hudmessage.h +++ b/code/hud/hudmessage.h @@ -120,7 +120,7 @@ class HudGaugeMessages: public HudGauge // HUD_MESSAGE_LINES void clearMessages(); void processMessageBuffer(); - void addPending(const char *text, int source, int x = 0); + void addPending(const char *text, size_t len, int source, int x = 0); void scrollMessages(); void preprocess() override; void render(float frametime, bool config = false) override; diff --git a/code/mission/missionlog.cpp b/code/mission/missionlog.cpp index 42d54ebb4de..502c8006257 100644 --- a/code/mission/missionlog.cpp +++ b/code/mission/missionlog.cpp @@ -437,86 +437,82 @@ int mission_log_get_count( LogType type, const char *pname, const char *sname ) } -void message_log_add_seg(log_text_seg *entry, int x, int line_offset, int msg_color, const char *text, int flags = 0) +void message_log_add_seg(log_text_seg *entry, int x, int line_offset, int msg_color, const char *text, size_t len, int flags = 0) { // set the vector - entry->text.reset(vm_strdup(text)); + entry->text.reset(vm_strndup(text, len)); entry->color = msg_color; entry->x = x; entry->line_offset = line_offset; entry->flags = flags; } -void message_log_add_seg(log_text_seg *entry, int x, int line_offset, int msg_color, const char *text, size_t len, int flags = 0) +void message_log_add_seg(log_text_seg *entry, int x, int line_offset, int msg_color, const char *text, int flags = 0) { - // set the vector - entry->text.reset(vm_strndup(text, len)); - entry->color = msg_color; - entry->x = x; - entry->line_offset = line_offset; - entry->flags = flags; + message_log_add_seg(entry, x, line_offset, msg_color, text, strlen(text), flags); } -void message_log_add_segs(const char *source_string, int msg_color, int flags, SCP_vector *entry, bool split_string) +void message_log_add_segs(const char *text, int msg_color, int flags, SCP_vector *entry, bool split_string) { - if (!source_string || !entry) { + if (!text || !entry) { mprintf(("Why are you passing a NULL pointer to message_log_add_segs?\n")); return; } - if (!*source_string) { - return; - } - - // duplicate the string so that we can split it without modifying the source - char *dup_string = vm_strdup(source_string); - char *str = dup_string; - char *split = NULL; if (split_string) { int sanity_counter = 0; while (true) { if (X == ACTION_X) { - while (is_white_space(*str)) - str++; + ignore_white_space(&text); + } + if (!*text) { + return; } - if (P_width - X > 0) { - split = split_str_once(str, P_width - X); + int w; + auto [split_len, split_next_pos, forced] = split_str_once(text, P_width - X, std::string::npos, 1.0f, &w); - // if we couldn't actually split the string, try again on a new line - if (split == str) { - sanity_counter++; - if (sanity_counter > 5) { - Error(LOCATION, "Too many attempts to wrap a mission log line! Get a coder!\nLine = %s", str); - break; - } - } else { - log_text_seg new_seg; - message_log_add_seg(&new_seg, X, Line_offset, msg_color, str, flags); + // we have more text to write (since we didn't return, above), so if we don't actually have enough room, try again on a new line + if (split_len == 0 || forced) { + split_next_pos = 0; + sanity_counter++; + if (sanity_counter > 5) { + Error(LOCATION, "Too many attempts to wrap a mission log line! Get a coder!\nLine = %s", text); + break; + } + } else { + log_text_seg new_seg; + + // When this is the final piece of the segment (no further split), preserve the + // whole input (including any trailing whitespace) and use its full width. Other + // colored segments may be concatenated onto the same rendered line after this one, + // and the trailing whitespace is what visually separates them. + if (split_next_pos == 0) { + message_log_add_seg(&new_seg, X, Line_offset, msg_color, text, flags); entry->push_back(std::move(new_seg)); - - if (!split) { - int w; - gr_get_string_size(&w, nullptr, str); - X += w; - break; - } + gr_get_string_size(&w, nullptr, text); + X += w; + break; } + + message_log_add_seg(&new_seg, X, Line_offset, msg_color, text, split_len, flags); + entry->push_back(std::move(new_seg)); } X = ACTION_X; Line_offset++; - str = split; + text += split_next_pos; } } else { + if (!*text) { + return; + } + log_text_seg new_seg; - message_log_add_seg(&new_seg, X, Line_offset, msg_color, str, flags); + message_log_add_seg(&new_seg, X, Line_offset, msg_color, text, flags); entry->push_back(std::move(new_seg)); } - - // free the buffer - vm_free(dup_string); } int mission_log_color_get_team(int msg_color) @@ -691,8 +687,7 @@ void mission_log_init_scrollback(int pw, bool split_string) Assert(!(entry.index & CARGO_NO_DEPLETE)); message_log_add_segs(XSTR("Cargo revealed: ", 418), LOG_COLOR_NORMAL, 0, &thisEntry.segments, split_string); - strncpy(text, Cargo_names[entry.index], sizeof(text) - 1); - message_log_add_segs(text, LOG_COLOR_BRIGHT, 0, &thisEntry.segments, split_string); + message_log_add_segs(Cargo_names[entry.index], LOG_COLOR_BRIGHT, 0, &thisEntry.segments, split_string); break; case LOG_CAP_SUBSYS_CARGO_REVEALED: @@ -701,8 +696,7 @@ void mission_log_init_scrollback(int pw, bool split_string) message_log_add_segs(entry.sname_display.c_str(), LOG_COLOR_NORMAL, 0, &thisEntry.segments, split_string); message_log_add_segs(XSTR( " subsystem cargo revealed: ", 1488), LOG_COLOR_NORMAL, 0, &thisEntry.segments, split_string); - strncpy(text, Cargo_names[entry.index], sizeof(text) - 1); - message_log_add_segs(text, LOG_COLOR_BRIGHT, 0, &thisEntry.segments, split_string); + message_log_add_segs(Cargo_names[entry.index], LOG_COLOR_BRIGHT, 0, &thisEntry.segments, split_string); break; diff --git a/code/mission/missiontraining.cpp b/code/mission/missiontraining.cpp index 169dfc14f85..a8ea8eba3fe 100644 --- a/code/mission/missiontraining.cpp +++ b/code/mission/missiontraining.cpp @@ -282,7 +282,6 @@ void HudGaugeDirectives::render(float /*frametime*/, bool config) int bx = x; int by = y + fl2i(middle_frame_offset_y * scale); - char* second_line; for (int i=0; i 0 ) { if (directives_middle.first_frame >= 0) renderBitmap(directives_middle.first_frame, bx, by, scale, config); @@ -387,14 +383,17 @@ void HudGaugeDirectives::render(float /*frametime*/, bool config) // blit the text gr_set_color_fast(c); - renderString(ox + line_x_offset, oy, EG_OBJ1 + i, buf, scale, config); + renderString(ox + line_x_offset, oy, EG_OBJ1 + i, buf.c_str(), split_len, scale, config); Training_obj_num_display_lines++; - if ( second_line ) { + if ( split_next_pos > 0 ) { + auto second_line = buf.c_str() + split_next_pos; + std::tie(split_len, split_next_pos, std::ignore) = split_str_once(second_line, fl2i(max_line_width * scale), std::string::npos, scale); + oy = y + fl2i(text_start_offsets[1] * scale) + Training_obj_num_display_lines * fl2i(text_h * scale); - renderString(ox+12, oy, EG_OBJ1 + i + 1, second_line, scale, config); + renderString(ox+12, oy, EG_OBJ1 + i + 1, second_line, split_len, scale, config); Training_obj_num_display_lines++; } diff --git a/code/missionui/missiondebrief.cpp b/code/missionui/missiondebrief.cpp index acd4a06937b..ed91141733f 100644 --- a/code/missionui/missiondebrief.cpp +++ b/code/missionui/missiondebrief.cpp @@ -2244,7 +2244,6 @@ void debrief_add_award_text(const char *str) return; } - char *line2; int field_width = (Medal_bitmap > 0) ? Debrief_award_text_width[gr_screen.res][DB_WITH_MEDAL] : Debrief_award_text_width[gr_screen.res][DB_WITHOUT_MEDAL]; // copy in the line @@ -2264,10 +2263,22 @@ void debrief_add_award_text(const char *str) // if its too long, split once ONLY // assumes text isnt > 2 lines, but this is a safe assumption due to the char limits of the ranks/badges/etc if (Debrief_award_text_num_lines < AWARD_TEXT_MAX_LINES) { - line2 = split_str_once(Debrief_award_text[Debrief_award_text_num_lines-1], field_width); - if (line2 != NULL) { - sprintf(Debrief_award_text[Debrief_award_text_num_lines], " %s", line2); // indent a space + auto cur_line = Debrief_award_text[Debrief_award_text_num_lines - 1]; + size_t split_len, split_next_pos; + std::tie(split_len, split_next_pos, std::ignore) = split_str_once(cur_line, field_width); + + if (split_next_pos > 0) { + cur_line[split_len] = '\0'; + auto tail = cur_line + split_next_pos; + + auto next_line = Debrief_award_text[Debrief_award_text_num_lines]; + next_line[0] = ' '; // indent the next line by one space + + size_t tail_len = std::min(strlen(tail), AWARD_TEXT_MAX_LINE_LENGTH - 2); + memmove(next_line + 1, tail, tail_len); + next_line[tail_len + 1] = '\0'; } + Debrief_award_text_num_lines++; // leave blank line even if it all fits into 1 } } diff --git a/code/parse/parselo.cpp b/code/parse/parselo.cpp index c4076577b9f..4d7b2ca7138 100644 --- a/code/parse/parselo.cpp +++ b/code/parse/parselo.cpp @@ -3691,83 +3691,131 @@ void display_parse_diagnostics() nprintf(("Parse", "%i errors. %i warnings.\n", Error_count, Warning_count)); } -// Splits a string into 2 lines if the string is wider than max_pixel_w pixels. A null -// terminator is placed where required to make the first line <= max_pixel_w. The remaining -// text is returned (leading whitespace removed). If the line doesn't need to be split, -// NULL is returned. -char *split_str_once(char *src, int max_pixel_w, float scale) -{ - char *brk = nullptr; - char *word_brk = nullptr; // if we fail, break here (even if its in the middle of a word) - bool last_was_white = false; - - Assert(src); - - if (max_pixel_w <= 0) - return src; // if there's no width, skip everything else - - int w; - gr_get_string_size(&w, nullptr, src, scale); - if ( (w <= max_pixel_w) && !strstr(src, "\n") ) { - return nullptr; // string doesn't require a cut +// Splits a string into two lines if the string is wider than max_pixel_w pixels, using a natural end-of-line or word separator if possible. +// Leading grayspace is skipped on the second line. Returns three values: the number of characters in the first line, the position offset +// of the second line, and a flag indicating whether this was a forced split (i.e. no natural break was found). If the line doesn't need +// to be split, the function returns [length, 0, false]. If either or both of the split_width and split_height parameters are supplied, +// they are stuffed with the width and height of the split string. +std::tuple split_str_once(const char *src, int max_pixel_w, size_t max_line_len, float scale, int *split_width, int *split_height) +{ + if (!src || !*src || max_line_len == 0) + { + if (split_width) + *split_width = 0; + if (split_height) + *split_height = 0; + return std::make_tuple(0, 0, false); } - size_t i; + unicode::codepoint_range range(src); + auto range_begin = std::begin(range); + auto range_end = std::end(range); + + // Implementation note: The pos, len, and max_line_len variables always refer to the number of chars (bytes), + // regardless of whether the game is running in Unicode mode or not. size_t len = strlen(src); - for (i=0; i max_pixel_w) + break; + + // otherwise remember the width because it's valid + last_split_width = *split_width; } - } - // if we are over max pixel width and weren't able to come up with a good non-word - // split then just split the word - if ( (w > max_pixel_w) && ((i == 0) || !brk) ) { - return word_brk; + // if we would exceed the max length on the next iteration, bail now (before the loop iterates) so we keep a valid value in pos + if (pos >= max_line_len) + break; } - if (!brk) { - brk = src + i; - } + // string fit without needing a split + if (iter == range_end) + { + if (saved_len != 0) + { + // We tracked a whitespace candidate during iteration, but since the whole string fit, the + // only reason to honor it is to trim trailing whitespace. If there is any non-whitespace + // after saved_iter, the candidate was just an internal space and should be ignored. + auto next_iter = saved_iter + 1; + while (next_iter != range_end && is_gray_space(*next_iter)) + ++next_iter; + if (next_iter == range_end) + return std::make_tuple(saved_len, 0, false); + } - *brk = 0; - src = brk + 1; - while (is_white_space(*src)) - src++; + return std::make_tuple(len, 0, false); + } - if (!*src) - return nullptr; // end of the string anyway + // if we are over max pixel width or max line length, and we weren't able to find a good non-word split, just force a split + if (((*split_width > max_pixel_w) || (pos >= max_line_len)) && (saved_len == 0)) + { + *split_width = last_split_width; + return std::make_tuple(pos, pos, true); + } - if (*src == '\n') - src++; + // now find a good place to start the next line + size_t pos2 = 0; + auto next_iter = saved_iter + 1; // be sure to move past the character that caused the split + if (next_iter != range_end) + { + while (is_gray_space(*next_iter)) + ++next_iter; + if (next_iter != range_end) + pos2 = (next_iter.pos() - src); + } - return src; + *split_width = saved_split_width; + return std::make_tuple(saved_len, pos2, false); } #define SPLIT_STR_BUFFER_SIZE 512 @@ -4047,208 +4095,6 @@ int split_str(const char *src, int max_pixel_w, SCP_vector &n_chars, SCP_ve return line_num; } -// A narrower but much faster alternative to split_str(), takes a string and a max pixel length, returns a vector with -// one pair (offset and length) per line. Does not currently support a max line count or ignoring of characters. -SCP_vector> str_wrap_to_width(const SCP_string& source_string, int max_pixel_width, bool strip_leading_whitespace, size_t source_start, size_t source_length) -{ - auto lines = SCP_vector>(); - - if (source_length == std::string::npos) - source_length = source_string.length() - source_start; - - Assertion(source_start + source_length <= source_string.length(), "In str_wrap_to_width(), source length must not exceed the actual length of the string!"); - - size_t pos_start = source_start; - size_t pos_end = source_start + source_length; - - // Advance past leading whitespace. - while (strip_leading_whitespace && (pos_start < pos_end) && is_white_space(source_string[pos_start])) - pos_start++; - - // Handle existing line breaks in the string recursively, then append the results. - while (pos_start < pos_end) { - auto newline_at = source_string.find_first_of(UNICODE_CHAR('\n'), pos_start); - if (newline_at == std::string::npos || newline_at >= source_length) - break; - - if (newline_at == pos_start) { - // No content to split so just pushing a new string on. - lines.emplace_back(pos_start, 0); - } else { - auto sublines = str_wrap_to_width(source_string, max_pixel_width, strip_leading_whitespace, pos_start, (newline_at - pos_start)); - lines.reserve(lines.size() + sublines.size()); // coverity[inefficient_reserve:FALSE] - std::move(sublines.begin(), sublines.end(), std::back_inserter(lines)); - } - - pos_start = newline_at + 1; - } - - // With newlines handled, now move into actually wrapping the content. - while (pos_start < pos_end) { - auto split_at = std::string::npos; - // no newlines found, check length. - size_t stringlen = pos_end - pos_start; - int line_width = 0; - gr_get_string_size(&line_width, nullptr, source_string.c_str() + pos_start, 1.0f, stringlen); - if (stringlen <= 1) { - // in this case checking is pointless, single-character strings can't wrap. - // copy into the return vector and then bail. - lines.emplace_back(pos_start, stringlen); - break; - } else if (line_width < max_pixel_width) { - // The remaining string is shorter than our limit so we're done. - // copy into the return vector and then bail. - lines.emplace_back(pos_start, stringlen); - break; - } else { - size_t search_min = 0; - size_t search_max = stringlen; - size_t center = 0; - while (search_max > search_min) { - center = search_min + ((search_max - search_min) / 2); - gr_get_string_size(&line_width, nullptr, source_string.c_str() + pos_start, 1.0f, center); - if (line_width == max_pixel_width) { - search_max = center; - search_min = center; - split_at = center; - } else if (line_width > max_pixel_width) { - search_max = MIN(center, search_max - 1); - split_at = search_max; - } else { - search_min = MAX(center, search_min + 1); - split_at = search_min; - } - } - } - - // don't split out of bounds, but it's ok to split exactly on the string boundary - if (split_at > stringlen) { - split_at = stringlen; - } else if (split_at < stringlen) { - // split_at is now the last point where we can split, but could be mid-word - // work backwards to find whitespace. - while ((split_at > 0) && !is_white_space(source_string[pos_start + split_at])) - split_at--; - - // we need to always remove something from the current line or we're stuck - if (split_at == 0) - split_at = 1; - } - - lines.emplace_back(pos_start, split_at); - pos_start += split_at; - - // Trim the leading whitespace off the next line. - while ((pos_start < pos_end) && is_white_space(source_string[pos_start])) - pos_start++; - } - - return lines; -} - -SCP_vector> str_wrap_to_width(const char* source_string, int max_pixel_width, bool strip_leading_whitespace, size_t source_length) -{ - auto lines = SCP_vector>(); - - if (source_length == std::string::npos) - source_length = strlen(source_string); - - Assertion(source_length <= strlen(source_string), "In str_wrap_to_width(), source length must not exceed the actual length of the string!"); - - const char* ch_start = source_string; - const char* ch_end = ch_start + source_length; - - // Advance past leading whitespace. - while (strip_leading_whitespace && (ch_start < ch_end) && is_white_space(*ch_start)) - ch_start++; - - // Handle existing line breaks in the string recursively, then append the results. - while (ch_start < ch_end) { - auto newline_at = strchr(ch_start, UNICODE_CHAR('\n')); - if (newline_at == nullptr || newline_at >= ch_end) - break; - - if (newline_at == ch_start) { - // No content to split so just pushing a new string on. - lines.emplace_back(ch_start - source_string, 0); - } else { - auto sublines = str_wrap_to_width(ch_start, max_pixel_width, strip_leading_whitespace, (newline_at - ch_start)); - - // need to adjust the positions to make them relative to the source string - if (ch_start != source_string) - for (auto& subline : sublines) - subline.first += (ch_start - source_string); - - lines.reserve(lines.size() + sublines.size()); // coverity[inefficient_reserve:FALSE] - std::move(sublines.begin(), sublines.end(), std::back_inserter(lines)); - } - - ch_start = newline_at + 1; - } - - // With newlines handled, now move into actually wrapping the content. - while (ch_start < ch_end) { - auto split_at = std::string::npos; - // no newlines found, check length. - size_t stringlen = ch_end - ch_start; - int line_width = 0; - gr_get_string_size(&line_width, nullptr, ch_start, 1.0f, stringlen); - if (stringlen <= 1) { - // in this case checking is pointless, single-character strings can't wrap. - // copy into the return vector and then bail. - lines.emplace_back(ch_start - source_string, stringlen); - break; - } else if (line_width < max_pixel_width) { - // The remaining string is shorter than our limit so we're done. - // copy into the return vector and then bail. - lines.emplace_back(ch_start - source_string, stringlen); - break; - } else { - size_t search_min = 0; - size_t search_max = stringlen; - size_t center = 0; - while (search_max > search_min) { - center = search_min + ((search_max - search_min) / 2); - gr_get_string_size(&line_width, nullptr, ch_start, 1.0f, center); - if (line_width == max_pixel_width) { - search_max = center; - search_min = center; - split_at = center; - } else if (line_width > max_pixel_width) { - search_max = MIN(center, search_max - 1); - split_at = search_max; - } else { - search_min = MAX(center, search_min + 1); - split_at = search_min; - } - } - } - - // don't split out of bounds, but it's ok to split exactly on the string boundary - if (split_at > stringlen) { - split_at = stringlen; - } else if (split_at < stringlen) { - // split_at is now the last point where we can split, but could be mid-word - // work backwards to find whitespace. - while ((split_at > 0) && !is_white_space(*(ch_start + split_at))) - split_at--; - - // we need to always remove something from the current line or we're stuck - if (split_at == 0) - split_at = 1; - } - - lines.emplace_back(ch_start - source_string, split_at); - ch_start += split_at; - - // Trim the leading whitespace off the next line. - while ((ch_start < ch_end) && is_white_space(*ch_start)) - ch_start++; - } - - return lines; -} - // Goober5000 // accounts for the dumb communications != communication, etc. int subsystem_stricmp(const char *str1, const char *str2) diff --git a/code/parse/parselo.h b/code/parse/parselo.h index 8640962302d..2ee2e3617e8 100644 --- a/code/parse/parselo.h +++ b/code/parse/parselo.h @@ -370,7 +370,7 @@ extern size_t maybe_convert_foreign_characters(const char *in, char *out, bool a extern void maybe_convert_foreign_characters(SCP_string &text); extern size_t get_converted_string_length(const char *text); extern size_t get_converted_string_length(const SCP_string &text); -char *split_str_once(char *src, int max_pixel_w, float scale = 1.0f); +std::tuple split_str_once(const char *src, int max_pixel_w, size_t max_line_len = std::string::npos, float scale = 1.0f, int *w = nullptr, int *h = nullptr); int split_str(const char* src, int max_pixel_w, int* n_chars, @@ -387,10 +387,6 @@ int split_str(const char* src, unicode::codepoint_t ignore_char = (unicode::codepoint_t) -1, bool strip_leading_whitespace = true); -SCP_vector> str_wrap_to_width(const SCP_string& source_string, int max_pixel_width, bool strip_leading_whitespace = true, size_t source_start = 0, size_t source_length = std::string::npos); - -SCP_vector> str_wrap_to_width(const char* source_string, int max_pixel_width, bool strip_leading_whitespace = true, size_t source_length = std::string::npos); - // fred extern int required_string_fred(const char *pstr, const char *end = NULL); extern int required_string_either_fred(const char *str1, const char *str2); diff --git a/code/scripting/api/libs/graphics.cpp b/code/scripting/api/libs/graphics.cpp index 3dd751fcdc1..471cda31e5f 100644 --- a/code/scripting/api/libs/graphics.cpp +++ b/code/scripting/api/libs/graphics.cpp @@ -1463,29 +1463,31 @@ static int drawString_sub(lua_State *L, bool use_resize_arg) std::swap(y, y2); } - auto lines = str_wrap_to_width(s, x2 - x, false); - //Make sure we don't go over size int line_ht = gr_get_font_height(); if (y2 < 0) y2 = y + line_ht; size_t max_num_lines = (y2 - y) / line_ht; - if (max_num_lines < lines.size()) - lines.resize(max_num_lines); - - num_lines = static_cast(lines.size()); int curr_y = y; - for(const auto &line: lines) + + for (size_t line_num = 0; line_num < max_num_lines; ++line_num) { + size_t split_len, split_next_pos; + std::tie(split_len, split_next_pos, std::ignore) = split_str_once(s, x2 - x); + //Draw the string - gr_string(x, curr_y, s + line.first, resize_mode, 1.0f, line.second); + gr_string(x, curr_y, s, resize_mode, 1.0f, split_len); + num_lines++; - //Increment line height + //Increment for the next line curr_y += line_ht; + if (split_next_pos == 0) + break; + s += split_next_pos; } - if (lines.empty()) + if (y == curr_y) { // If no line was drawn then we need to add one so the next line is // aligned right diff --git a/test/src/parse/test_split_str.cpp b/test/src/parse/test_split_str.cpp new file mode 100644 index 00000000000..7fb447811a1 --- /dev/null +++ b/test/src/parse/test_split_str.cpp @@ -0,0 +1,279 @@ + +#include + +#include + +#include "util/FSTestFixture.h" + +class SplitStrTest : public test::FSTestFixture +{ + public: + SplitStrTest() : test::FSTestFixture(INIT_CFILE | INIT_GRAPHICS | INIT_FONTS) {} +}; + +TEST_F(SplitStrTest, split_str_once_empty) +{ + { + auto [len, pos, forced] = split_str_once(nullptr, 100); + ASSERT_EQ(len, 0); + ASSERT_EQ(pos, 0); + ASSERT_EQ(forced, false); + } + + { + auto [len, pos, forced] = split_str_once("", 100); + ASSERT_EQ(len, 0); + ASSERT_EQ(pos, 0); + ASSERT_EQ(forced, false); + } +} + +// per GitHub #5473, zero or negative width should not cause a split +TEST_F(SplitStrTest, split_str_once_zero) +{ + { + auto [len, pos, forced] = split_str_once("abcde", 0); + ASSERT_EQ(len, 5); + ASSERT_EQ(pos, 0); + ASSERT_EQ(forced, false); + } + + { + auto [len, pos, forced] = split_str_once("abc defg", 0); + ASSERT_EQ(len, 8); + ASSERT_EQ(pos, 0); + ASSERT_EQ(forced, false); + } + + { + auto [len, pos, forced] = split_str_once("abc\tdefg", 0); + ASSERT_EQ(len, 8); + ASSERT_EQ(pos, 0); + ASSERT_EQ(forced, false); + } + + { + auto [len, pos, forced] = split_str_once("abc\ndefg", 0); + ASSERT_EQ(len, 8); + ASSERT_EQ(pos, 0); + ASSERT_EQ(forced, false); + } + + { + auto [len, pos, forced] = split_str_once("abc defg", -1); + ASSERT_EQ(len, 8); + ASSERT_EQ(pos, 0); + ASSERT_EQ(forced, false); + } +} + +TEST_F(SplitStrTest, split_str_once_typical) +{ + // note - in the default retail font, most characters are 8 pixels wide + + // no split + { + auto [len, pos, forced] = split_str_once("abcdefghijklmnopqrstuvwxyz", 300); + ASSERT_EQ(len, 26); + ASSERT_EQ(pos, 0); + ASSERT_EQ(forced, false); + } + + // forced split + { + auto [len, pos, forced] = split_str_once("abcdefghijklmnopqrstuvwxyz", 80); + ASSERT_EQ(len, 10); + ASSERT_EQ(pos, 10); + ASSERT_EQ(forced, true); + } + + // Mantis #1152: always split on a newline even if the string would otherwise fit + { + auto [len, pos, forced] = split_str_once("abcde\n fghij", 300); + ASSERT_EQ(len, 5); + ASSERT_EQ(pos, 8); + ASSERT_EQ(forced, false); + } + + // regular splits + { + auto [len, pos, forced] = split_str_once("abcde fghij klmno pqrstuvwxyz abcdefghijklmnopqrstuvwxyz", 300); + ASSERT_EQ(len, 29); + ASSERT_EQ(pos, 30); + ASSERT_EQ(forced, false); + } + { + auto [len, pos, forced] = split_str_once("abcdefghijklmnopqrstuvwxyz abcdefghijklmnopqrstuvwxyz", 300); + ASSERT_EQ(len, 26); + ASSERT_EQ(pos, 27); + ASSERT_EQ(forced, false); + } + { + auto [len, pos, forced] = split_str_once("abcdefghijklmnopqrstuvwxyz\t abcdefghijklmnopqrstuvwxyz", 300); + ASSERT_EQ(len, 26); + ASSERT_EQ(pos, 28); + ASSERT_EQ(forced, false); + } + { + auto [len, pos, forced] = split_str_once("abcdefghijklmnopqrstuvwxyz\n abcdefghijklmnopqrstuvwxyz", 300); + ASSERT_EQ(len, 26); + ASSERT_EQ(pos, 28); + ASSERT_EQ(forced, false); + } + { + auto [len, pos, forced] = split_str_once("abcdefghijklmnopqrstuvwxyz \nabcdefghijklmnopqrstuvwxyz", 300); + ASSERT_EQ(len, 27); + ASSERT_EQ(pos, 28); + ASSERT_EQ(forced, false); + } +} + +TEST_F(SplitStrTest, split_str_once_advanced) +{ + // tricky splits + { + auto [len, pos, forced] = split_str_once("a\n\nbcde", 300); + ASSERT_EQ(len, 1); + ASSERT_EQ(pos, 2); + ASSERT_EQ(forced, false); + } + { + auto [len, pos, forced] = split_str_once("\n\nabcde", 300); + ASSERT_EQ(len, 0); + ASSERT_EQ(pos, 1); + ASSERT_EQ(forced, false); + } + { + auto [len, pos, forced] = split_str_once("abcd ", 300); + ASSERT_EQ(len, 4); + ASSERT_EQ(pos, 0); + ASSERT_EQ(forced, false); + } + { + auto [len, pos, forced] = split_str_once("abcde\t", 300); + ASSERT_EQ(len, 5); + ASSERT_EQ(pos, 0); + ASSERT_EQ(forced, false); + } + { + auto [len, pos, forced] = split_str_once("abc\n", 300); + ASSERT_EQ(len, 3); + ASSERT_EQ(pos, 0); + ASSERT_EQ(forced, false); + } + { + auto [len, pos, forced] = split_str_once("abcd\n ", 300); + ASSERT_EQ(len, 4); + ASSERT_EQ(pos, 0); + ASSERT_EQ(forced, false); + } + { + auto [len, pos, forced] = split_str_once("abcd \n", 300); + ASSERT_EQ(len, 5); + ASSERT_EQ(pos, 0); + ASSERT_EQ(forced, false); + } + { + auto [len, pos, forced] = split_str_once("aa", 16); + ASSERT_EQ(len, 2); + ASSERT_EQ(pos, 0); + ASSERT_EQ(forced, false); + } + { + auto [len, pos, forced] = split_str_once("aa ", 16); + ASSERT_EQ(len, 2); + ASSERT_EQ(pos, 0); + ASSERT_EQ(forced, false); + } + { + auto [len, pos, forced] = split_str_once("aa\n", 16); + ASSERT_EQ(len, 2); + ASSERT_EQ(pos, 0); + ASSERT_EQ(forced, false); + } + + // very tricky + { + auto [len, pos, forced] = split_str_once("\nabcdefghijklmnopqrstuvwxyz", 100); // wide enough for the \n, but not wide enough for the whole string + ASSERT_EQ(len, 0); + ASSERT_EQ(pos, 1); + ASSERT_EQ(forced, false); + } + + // whole string fits but contains internal whitespace - must not split + { + auto [len, pos, forced] = split_str_once("abcd efgh", 300); + ASSERT_EQ(len, 9); + ASSERT_EQ(pos, 0); + ASSERT_EQ(forced, false); + } + { + auto [len, pos, forced] = split_str_once("fighter cover pronto, we'll never reach Vega!", 1000); + ASSERT_EQ(len, 45); + ASSERT_EQ(pos, 0); + ASSERT_EQ(forced, false); + } + + // splits constrained by length + { + auto [len, pos, forced] = split_str_once("abcde fghij klmno pqrstuvwxyz abcdefghijklmnopqrstuvwxyz", 300, 15); + ASSERT_EQ(len, 11); + ASSERT_EQ(pos, 12); + ASSERT_EQ(forced, false); + } + { + auto [len, pos, forced] = split_str_once("abcde fghij klmno pqrstuvwxyz abcdefghijklmnopqrstuvwxyz", 300, 5); + ASSERT_EQ(len, 5); + ASSERT_EQ(pos, 6); + ASSERT_EQ(forced, false); + } + { + auto [len, pos, forced] = split_str_once("abcde fghij\nklmno pqrstuvwxyz abcdefghijklmnopqrstuvwxyz", 300, 11); + ASSERT_EQ(len, 11); + ASSERT_EQ(pos, 12); + ASSERT_EQ(forced, false); + } + { + auto [len, pos, forced] = split_str_once("abcdefghijklmnopqrstuvwxyz abcdefghijklmnopqrstuvwxyz", 300, 15); + ASSERT_EQ(len, 15); + ASSERT_EQ(pos, 15); + ASSERT_EQ(forced, true); + } + { + auto [len, pos, forced] = split_str_once("abcde", 300, 5); + ASSERT_EQ(len, 5); + ASSERT_EQ(pos, 0); + ASSERT_EQ(forced, false); + } + { + auto [len, pos, forced] = split_str_once("abcde ", 300, 5); + ASSERT_EQ(len, 5); + ASSERT_EQ(pos, 0); + ASSERT_EQ(forced, false); + } + { + auto [len, pos, forced] = split_str_once("abcde\n", 300, 5); + ASSERT_EQ(len, 5); + ASSERT_EQ(pos, 0); + ASSERT_EQ(forced, false); + } + + // measuring string width + { + int w; + auto [len, pos, forced] = split_str_once("a b cde", 25, std::string::npos, 1.0f, &w); + ASSERT_EQ(len, 3); + ASSERT_EQ(pos, 4); + ASSERT_EQ(forced, false); + ASSERT_EQ(w, 24); + } + { + int w; + auto [len, pos, forced] = split_str_once("a ", 25, std::string::npos, 1.0f, &w); + ASSERT_EQ(len, 1); + ASSERT_EQ(pos, 0); + ASSERT_EQ(forced, false); + ASSERT_EQ(w, 8); + } + +} diff --git a/test/src/source_groups.cmake b/test/src/source_groups.cmake index 08a5c1f8245..a2f15fb37be 100644 --- a/test/src/source_groups.cmake +++ b/test/src/source_groups.cmake @@ -46,6 +46,7 @@ add_file_folder("model" add_file_folder("Parse" parse/test_parselo.cpp parse/test_replace.cpp + parse/test_split_str.cpp ) add_file_folder("Pilotfile"