Move non-const iteration from Vector to Vector.write.

This allows owners of non-const `Vector` to const-iterate it, avoiding an accidental fork, which would be inefficient.
Ivorforce · Dec 22, 2024 · 38cfb57 · 38cfb57
1 parent 216b330
commit 38cfb57
Show file tree

Hide file tree

Showing 31 changed files with 146 additions and 139 deletions.
diff --git a/core/templates/vector.h b/core/templates/vector.h
@@ -53,9 +53,44 @@ template <typename T>
 class VectorWriteProxy {
 public:
 	_FORCE_INLINE_ T &operator[](typename CowData<T>::Size p_index) {
-		CRASH_BAD_INDEX(p_index, ((Vector<T> *)(this))->_cowdata.size());
+		CRASH_BAD_INDEX(p_index, ((Vector<T> *)this)->_cowdata.size());
 
-		return ((Vector<T> *)(this))->_cowdata.ptrw()[p_index];
+		return ((Vector<T> *)this)->_cowdata.ptrw()[p_index];
+	}
+
+	// NOTE: The non-const iterators need to be declared here because otherwise, these functions
+	//       are chosen as the default iteration methods on non-const objects.
+	//       This may cause an unnecessary fork of underlying data buffer.
+	struct Iterator {
+		_FORCE_INLINE_ T &operator*() const {
+			return *elem_ptr;
+		}
+		_FORCE_INLINE_ T *operator->() const { return elem_ptr; }
+		_FORCE_INLINE_ Iterator &operator++() {
+			elem_ptr++;
+			return *this;
+		}
+		_FORCE_INLINE_ Iterator &operator--() {
+			elem_ptr--;
+			return *this;
+		}
+
+		_FORCE_INLINE_ bool operator==(const Iterator &b) const { return elem_ptr == b.elem_ptr; }
+		_FORCE_INLINE_ bool operator!=(const Iterator &b) const { return elem_ptr != b.elem_ptr; }
+
+		Iterator(T *p_ptr) { elem_ptr = p_ptr; }
+		Iterator() {}
+		Iterator(const Iterator &p_it) { elem_ptr = p_it.elem_ptr; }
+
+	private:
+		T *elem_ptr = nullptr;
+	};
+
+	_FORCE_INLINE_ Iterator begin() {
+		return Iterator(((Vector<T> *)this)->ptrw());
+	}
+	_FORCE_INLINE_ Iterator end() {
+		return Iterator(((Vector<T> *)this)->ptrw() + ((Vector<T> *)this)->size());
 	}
 };
 
@@ -218,30 +253,7 @@ class Vector {
 		return false;
 	}
 
-	struct Iterator {
-		_FORCE_INLINE_ T &operator*() const {
-			return *elem_ptr;
-		}
-		_FORCE_INLINE_ T *operator->() const { return elem_ptr; }
-		_FORCE_INLINE_ Iterator &operator++() {
-			elem_ptr++;
-			return *this;
-		}
-		_FORCE_INLINE_ Iterator &operator--() {
-			elem_ptr--;
-			return *this;
-		}
-
-		_FORCE_INLINE_ bool operator==(const Iterator &b) const { return elem_ptr == b.elem_ptr; }
-		_FORCE_INLINE_ bool operator!=(const Iterator &b) const { return elem_ptr != b.elem_ptr; }
-
-		Iterator(T *p_ptr) { elem_ptr = p_ptr; }
-		Iterator() {}
-		Iterator(const Iterator &p_it) { elem_ptr = p_it.elem_ptr; }
-
-	private:
-		T *elem_ptr = nullptr;
-	};
+	using Iterator = typename VectorWriteProxy<T>::Iterator;
 
 	struct ConstIterator {
 		_FORCE_INLINE_ const T &operator*() const {
@@ -268,16 +280,11 @@ class Vector {
 		const T *elem_ptr = nullptr;
 	};
 
-	_FORCE_INLINE_ Iterator begin() {
-		return Iterator(ptrw());
-	}
-	_FORCE_INLINE_ Iterator end() {
-		return Iterator(ptrw() + size());
-	}
-
+	// For non-const iteration, iterate the .write property.
 	_FORCE_INLINE_ ConstIterator begin() const {
 		return ConstIterator(ptr());
 	}
+	// For non-const iteration, iterate the .write property.
 	_FORCE_INLINE_ ConstIterator end() const {
 		return ConstIterator(ptr() + size());
 	}

diff --git a/drivers/metal/metal_objects.mm b/drivers/metal/metal_objects.mm
@@ -1688,7 +1688,7 @@
 
 MDRenderPass::MDRenderPass(Vector<MDAttachment> &p_attachments, Vector<MDSubpass> &p_subpasses) :
 		attachments(p_attachments), subpasses(p_subpasses) {
-	for (MDAttachment &att : attachments) {
+	for (MDAttachment &att : attachments.write) {
 		att.linkToSubpass(*this);
 	}
 }

diff --git a/editor/doc_tools.cpp b/editor/doc_tools.cpp
@@ -123,7 +123,7 @@ static void merge_constructors(Vector<DocData::MethodDoc> &p_to, const Vector<Do
 	int64_t from_size = p_from.size();
 
 	// TODO: Improve constructor merging.
-	for (DocData::MethodDoc &to : p_to) {
+	for (DocData::MethodDoc &to : p_to.write) {
 		for (int64_t from_i = 0; from_i < from_size; ++from_i) {
 			const DocData::MethodDoc &from = from_ptr[from_i];
 
@@ -209,7 +209,7 @@ static void merge_constants(Vector<DocData::ConstantDoc> &p_to, const Vector<Doc
 
 	SearchArray<DocData::ConstantDoc> search_array;
 
-	for (DocData::ConstantDoc &to : p_to) {
+	for (DocData::ConstantDoc &to : p_to.write) {
 		int64_t found = search_array.bisect(from_ptr, from_size, to, true);
 
 		if (found >= from_size) {

diff --git a/editor/editor_data.cpp b/editor/editor_data.cpp
@@ -145,7 +145,7 @@ void EditorSelectionHistory::add_object(ObjectID p_object, const String &p_prope
 }
 
 void EditorSelectionHistory::replace_object(ObjectID p_old_object, ObjectID p_new_object) {
-	for (HistoryElement &element : history) {
+	for (HistoryElement &element : history.write) {
 		for (int index = 0; index < element.path.size(); index++) {
 			if (element.path[index].object == p_old_object) {
 				element.path.write[index].object = p_new_object;

diff --git a/editor/gui/editor_file_dialog.cpp b/editor/gui/editor_file_dialog.cpp
@@ -110,7 +110,7 @@ void EditorFileDialog::_native_dialog_cb(bool p_ok, const Vector<String> &p_file
 
 	Vector<String> files = p_files;
 	if (access != ACCESS_FILESYSTEM) {
-		for (String &file_name : files) {
+		for (String &file_name : files.write) {
 			file_name = ProjectSettings::get_singleton()->localize_path(file_name);
 		}
 	}

diff --git a/editor/plugins/game_view_plugin.cpp b/editor/plugins/game_view_plugin.cpp
@@ -80,15 +80,15 @@ void GameViewDebugger::set_suspend(bool p_enabled) {
 	Array message;
 	message.append(p_enabled);
 
-	for (Ref<EditorDebuggerSession> &I : sessions) {
+	for (const Ref<EditorDebuggerSession> &I : sessions) {
 		if (I->is_active()) {
 			I->send_message("scene:suspend_changed", message);
 		}
 	}
 }
 
 void GameViewDebugger::next_frame() {
-	for (Ref<EditorDebuggerSession> &I : sessions) {
+	for (const Ref<EditorDebuggerSession> &I : sessions) {
 		if (I->is_active()) {
 			I->send_message("scene:next_frame", Array());
 		}
@@ -101,7 +101,7 @@ void GameViewDebugger::set_node_type(int p_type) {
 	Array message;
 	message.append(p_type);
 
-	for (Ref<EditorDebuggerSession> &I : sessions) {
+	for (const Ref<EditorDebuggerSession> &I : sessions) {
 		if (I->is_active()) {
 			I->send_message("scene:runtime_node_select_set_type", message);
 		}
@@ -114,7 +114,7 @@ void GameViewDebugger::set_selection_visible(bool p_visible) {
 	Array message;
 	message.append(p_visible);
 
-	for (Ref<EditorDebuggerSession> &I : sessions) {
+	for (const Ref<EditorDebuggerSession> &I : sessions) {
 		if (I->is_active()) {
 			I->send_message("scene:runtime_node_select_set_visible", message);
 		}
@@ -127,7 +127,7 @@ void GameViewDebugger::set_select_mode(int p_mode) {
 	Array message;
 	message.append(p_mode);
 
-	for (Ref<EditorDebuggerSession> &I : sessions) {
+	for (const Ref<EditorDebuggerSession> &I : sessions) {
 		if (I->is_active()) {
 			I->send_message("scene:runtime_node_select_set_mode", message);
 		}
@@ -147,15 +147,15 @@ void GameViewDebugger::set_camera_manipulate_mode(EditorDebuggerNode::CameraOver
 }
 
 void GameViewDebugger::reset_camera_2d_position() {
-	for (Ref<EditorDebuggerSession> &I : sessions) {
+	for (const Ref<EditorDebuggerSession> &I : sessions) {
 		if (I->is_active()) {
 			I->send_message("scene:runtime_node_select_reset_camera_2d", Array());
 		}
 	}
 }
 
 void GameViewDebugger::reset_camera_3d_position() {
-	for (Ref<EditorDebuggerSession> &I : sessions) {
+	for (const Ref<EditorDebuggerSession> &I : sessions) {
 		if (I->is_active()) {
 			I->send_message("scene:runtime_node_select_reset_camera_3d", Array());
 		}

diff --git a/editor/plugins/tiles/tile_set_atlas_source_editor.cpp b/editor/plugins/tiles/tile_set_atlas_source_editor.cpp
@@ -2296,7 +2296,7 @@ void TileSetAtlasSourceEditor::_cleanup_outside_tiles() {
 }
 
 void TileSetAtlasSourceEditor::_auto_create_tiles() {
-	for (Ref<TileSetAtlasSource> &atlas_source : atlases_to_auto_create_tiles) {
+	for (const Ref<TileSetAtlasSource> &atlas_source : atlases_to_auto_create_tiles) {
 		if (atlas_source.is_valid()) {
 			Ref<Texture2D> texture = atlas_source->get_texture();
 			if (texture.is_valid()) {