Merge pull request #75975 from bruvzg/bidi_and_log_errors

Improve line BiDi handling, prevent crash on recursive log updates.

editor/editor_log.cpp

@@ -276,6 +276,11 @@ void EditorLog::_add_log_line(LogMessage &p_message, bool p_replace_previous) {
 		return;
 	}
 
+	if (unlikely(log->is_updating())) {
+		// The new message arrived during log RTL text processing/redraw (invalid BiDi control characters / font error), ignore it to avoid RTL data corruption.
+		return;
+	}
+
 	// Only add the message to the log if it passes the filters.
 	bool filter_active = type_filter_map[p_message.type]->is_active();
 	String search_text = search_box->get_text();

modules/text_server_adv/text_server_adv.cpp

@@ -4138,6 +4138,7 @@ RID TextServerAdvanced::_shaped_text_substr(const RID &p_shaped, int64_t p_start
 	new_sd->direction = sd->direction;
 	new_sd->custom_punct = sd->custom_punct;
 	new_sd->para_direction = sd->para_direction;
+	new_sd->base_para_direction = sd->base_para_direction;
 	for (int i = 0; i < TextServer::SPACING_MAX; i++) {
 		new_sd->extra_spacing[i] = sd->extra_spacing[i];
 	}
@@ -4188,9 +4189,33 @@ bool TextServerAdvanced::_shape_substr(ShapedTextDataAdvanced *p_new_sd, const S
 				if (U_SUCCESS(err)) {
 					ubidi_setLine(p_sd->bidi_iter[ov], start, end, bidi_iter, &err);
 					if (U_FAILURE(err)) {
-						ubidi_close(bidi_iter);
-						bidi_iter = nullptr;
-						ERR_PRINT(vformat("BiDi reordering for the line failed: %s", u_errorName(err)));
+						// Line BiDi failed (string contains incompatible control characters), try full paragraph BiDi instead.
+						err = U_ZERO_ERROR;
+						const UChar *data = p_sd->utf16.get_data();
+						switch (static_cast<TextServer::Direction>(p_sd->bidi_override[ov].z)) {
+							case DIRECTION_LTR: {
+								ubidi_setPara(bidi_iter, data + start, end - start, UBIDI_LTR, nullptr, &err);
+							} break;
+							case DIRECTION_RTL: {
+								ubidi_setPara(bidi_iter, data + start, end - start, UBIDI_RTL, nullptr, &err);
+							} break;
+							case DIRECTION_INHERITED: {
+								ubidi_setPara(bidi_iter, data + start, end - start, p_sd->base_para_direction, nullptr, &err);
+							} break;
+							case DIRECTION_AUTO: {
+								UBiDiDirection direction = ubidi_getBaseDirection(data + start, end - start);
+								if (direction != UBIDI_NEUTRAL) {
+									ubidi_setPara(bidi_iter, data + start, end - start, direction, nullptr, &err);
+								} else {
+									ubidi_setPara(bidi_iter, data + start, end - start, p_sd->base_para_direction, nullptr, &err);
+								}
+							} break;
+						}
+						if (U_FAILURE(err)) {
+							ubidi_close(bidi_iter);
+							bidi_iter = nullptr;
+							ERR_PRINT(vformat("BiDi reordering for the line failed: %s", u_errorName(err)));
+						}
 					}
 				} else {
 					bidi_iter = nullptr;
@@ -5619,25 +5644,25 @@ bool TextServerAdvanced::_shaped_text_shape(const RID &p_shaped) {
 		sd->script_iter = memnew(ScriptIterator(sd->text, 0, sd->text.length()));
 	}
 
-	int base_para_direction = UBIDI_DEFAULT_LTR;
+	sd->base_para_direction = UBIDI_DEFAULT_LTR;
 	switch (sd->direction) {
 		case DIRECTION_LTR: {
 			sd->para_direction = DIRECTION_LTR;
-			base_para_direction = UBIDI_LTR;
+			sd->base_para_direction = UBIDI_LTR;
 		} break;
 		case DIRECTION_RTL: {
 			sd->para_direction = DIRECTION_RTL;
-			base_para_direction = UBIDI_RTL;
+			sd->base_para_direction = UBIDI_RTL;
 		} break;
 		case DIRECTION_INHERITED:
 		case DIRECTION_AUTO: {
 			UBiDiDirection direction = ubidi_getBaseDirection(data, sd->utf16.length());
 			if (direction != UBIDI_NEUTRAL) {
 				sd->para_direction = (direction == UBIDI_RTL) ? DIRECTION_RTL : DIRECTION_LTR;
-				base_para_direction = direction;
+				sd->base_para_direction = direction;
 			} else {
 				sd->para_direction = DIRECTION_LTR;
-				base_para_direction = UBIDI_DEFAULT_LTR;
+				sd->base_para_direction = UBIDI_DEFAULT_LTR;
 			}
 		} break;
 	}
@@ -5666,14 +5691,14 @@ bool TextServerAdvanced::_shaped_text_shape(const RID &p_shaped) {
 					ubidi_setPara(bidi_iter, data + start, end - start, UBIDI_RTL, nullptr, &err);
 				} break;
 				case DIRECTION_INHERITED: {
-					ubidi_setPara(bidi_iter, data + start, end - start, base_para_direction, nullptr, &err);
+					ubidi_setPara(bidi_iter, data + start, end - start, sd->base_para_direction, nullptr, &err);
 				} break;
 				case DIRECTION_AUTO: {
 					UBiDiDirection direction = ubidi_getBaseDirection(data + start, end - start);
 					if (direction != UBIDI_NEUTRAL) {
 						ubidi_setPara(bidi_iter, data + start, end - start, direction, nullptr, &err);
 					} else {
-						ubidi_setPara(bidi_iter, data + start, end - start, base_para_direction, nullptr, &err);
+						ubidi_setPara(bidi_iter, data + start, end - start, sd->base_para_direction, nullptr, &err);
 					}
 				} break;
 			}

modules/text_server_adv/text_server_adv.h

@@ -476,6 +476,7 @@ class TextServerAdvanced : public TextServerExtension {
 
 		/* Shaped data */
 		TextServer::Direction para_direction = DIRECTION_LTR; // Detected text direction.
+		int base_para_direction = UBIDI_DEFAULT_LTR;
 		bool valid = false; // String is shaped.
 		bool line_breaks_valid = false; // Line and word break flags are populated (and virtual zero width spaces inserted).
 		bool justification_ops_valid = false; // Virtual elongation glyphs are added to the string.

scene/gui/rich_text_label.cpp

@@ -2705,6 +2705,10 @@ bool RichTextLabel::is_ready() const {
 	return (main->first_invalid_line.load() == (int)main->lines.size() && main->first_resized_line.load() == (int)main->lines.size() && main->first_invalid_font_line.load() == (int)main->lines.size());
 }
 
+bool RichTextLabel::is_updating() const {
+	return updating.load() || validating.load();
+}
+
 void RichTextLabel::set_threaded(bool p_threaded) {
 	if (threaded != p_threaded) {
 		_stop_thread();
@@ -2729,6 +2733,7 @@ bool RichTextLabel::_validate_line_caches() {
 	if (updating.load()) {
 		return false;
 	}
+	validating.store(true);
 	if (main->first_invalid_line.load() == (int)main->lines.size()) {
 		MutexLock data_lock(data_mutex);
 		Rect2 text_rect = _get_text_rect();
@@ -2747,6 +2752,7 @@ bool RichTextLabel::_validate_line_caches() {
 
 		if (main->first_resized_line.load() == (int)main->lines.size()) {
 			vscroll->set_value(old_scroll);
+			validating.store(false);
 			return true;
 		}
 
@@ -2798,8 +2804,10 @@ bool RichTextLabel::_validate_line_caches() {
 		if (fit_content) {
 			update_minimum_size();
 		}
+		validating.store(false);
 		return true;
 	}
+	validating.store(false);
 	stop_thread.store(false);
 	if (threaded) {
 		updating.store(true);
@@ -2809,7 +2817,9 @@ bool RichTextLabel::_validate_line_caches() {
 		loading_started = OS::get_singleton()->get_ticks_msec();
 		return false;
 	} else {
+		updating.store(true);
 		_process_line_caches();
+		updating.store(false);
 		queue_redraw();
 		return true;
 	}
@@ -5895,6 +5905,7 @@ RichTextLabel::RichTextLabel(const String &p_text) {
 
 	set_text(p_text);
 	updating.store(false);
+	validating.store(false);
 	stop_thread.store(false);
 
 	set_clip_contents(true);

scene/gui/rich_text_label.h

@@ -377,6 +377,7 @@ private:
 	bool threaded = false;
 	std::atomic<bool> stop_thread;
 	std::atomic<bool> updating;
+	std::atomic<bool> validating;
 	std::atomic<double> loaded;
 
 	uint64_t loading_started = 0;
@@ -678,6 +679,7 @@ public:
 	void deselect();
 
 	bool is_ready() const;
+	bool is_updating() const;
 
 	void set_threaded(bool p_threaded);
 	bool is_threaded() const;