feat region streaming #675

druidui · 2025-04-12T18:31:37Z

add options in inspector under "Regions" to change render distance and to toggle region streaming
add region streaming implementation

druidui · 2025-04-12T19:24:42Z

Some issues with performance here, only tested at region size 2048 - if there's anything you can spot which may immediately increase performance, please share!

druidui · 2025-04-13T13:56:33Z

add region loading to it's own thread

TokisanGames

Thanks for a solid start on this. There's some good stuff here. There's data loss and performance issues, but a good start.

When running it with WorldBackground = none, 1-4 regions that don't exist appear under the camera, many more if rings > 1. They are real regions that generate collision. They can't be removed.
Disabling streaming, all of the extra regions still exist. And existing regions that were hidden are insivible/unloaded.
I made 56 regions with data (7x8), and saved. I then enabled region streaming and ran. Several regions had been flattened. I then quit, reverted the scene so streaming was no longer enabled and reloaded the scene. This then loaded all regions from disk that existed when I ran the scene. Several regions were missing entirely. The files had been deleted. Likely set_deleted then saved.
I looked at the smaller code change blocks. I skipped over the large blocks of code changes for now, and will look closer over time as it improves. Because of the extent of the modifications, I want to either remove unrelated changes, or at the very least put them in a separate commit for now. I also want to discuss the design and intention of some of them either here or on discord.
I want the function code to be in the same order as in the header. Right now you have private functions mixed in with the public functions.
What I don't see in here is the lifting of max bounds and letting it generate a smaller window of regions. Which that may be more complicated and better done in a subsequent PR. Why don't you make a tracker in the first post of the various tasks you intend to scope for this PR, marking those you've done and still plan to do.
Each time you do an update, it would be a good idea to rebase on the latest.
Performance is a bit choppy. This is a first pass, but we'll need more refinement before it is production ready. We need to do some profiling, probably with an engine with debug symbols as well. Or at the very least, you could print out the time for every section so we know what is actually slow
I haven't looked closely, but please describe the region states you've added. We had:
- modified (to be saved)
- edited (for undo)
- deleted (to have file removed)
  It seems like you haven't changed this and regions are either completely unloaded or loaded and rendering.

Perhaps we also need separate states for:

on disk only / unloaded
loaded in RAM only
visible/invisible (compiled in the texture array in VRAM, but not rendered - vertices discarded)
rendering normally

This would give us more fine-grained control over performance. Combined with the performance metrics, we'd limit the most expensive operations, or space out when we do them, and allow us to freely do the cheap ones (eg changing visibility). Just changing visibility will improve performance.

@Xtarsia did you play with this?

TokisanGames · 2025-04-20T20:04:33Z

src/terrain_3d.h

@@ -38,7 +38,7 @@ class Terrain3D : public Node3D {
 	};

 private:
-	String _version = "1.1.0-dev";
+	String _version = "1.0.0";


This shouldn't have been changed.

TokisanGames · 2025-04-20T20:06:50Z

src/terrain_3d_data.h

@@ -6,6 +6,8 @@
 #include "constants.h"
 #include "generated_texture.h"
 #include "terrain_3d_region.h"
+#include <godot_cpp/classes/mutex.hpp>
+#include <godot_cpp/classes/thread.hpp>


These should be higher than the #include "". clang-format should have already done this for you, which tells me you're not running it.

TokisanGames · 2025-04-20T20:09:22Z

src/terrain_3d.h

@@ -78,6 +78,10 @@ class Terrain3D : public Node3D {
 	real_t _cull_margin = 0.0f;
 	bool _free_editor_textures = true;

+	// Region Streaming
+	bool _enable_streaming = false;
+	int _streaming_rings = 1;


You don't need these in Terrain3D and Terrain3DData. You can have the alias in the first and the variable in the second. See collision_mode for instance.

TokisanGames · 2025-04-20T20:10:44Z

src/terrain_3d.h

+
+	// Backward compatibility
+	void set_streaming_distance(const real_t p_distance);
+	real_t get_streaming_distance() const;


If this is newly created, why is it already backwards compatible?

TokisanGames · 2025-04-20T20:16:15Z

src/terrain_3d_data.cpp

+ * - Data is copied from overlapping regions to the new larger regions
+ *
+ * @param p_new_size The new region size (must be power of 2: 64, 128, 256, 512, 1024, 2048)
+ */
 void Terrain3DData::change_region_size(int p_new_size) {


I have not analyzed these changes yet, but I'm very surprised that you changed this function. It was already an overly complicated function and now there's more code in it. It's a function that makes a permanent change to the data, whereas region streaming shouldn't change data at all. So, before I dive into it, please explain your reasoning behind this change.

TokisanGames · 2025-04-20T20:21:08Z

src/terrain_3d_data.h

 	}
-	return nullptr;
+
+	return cast_to<Terrain3DRegion>(obj);


has_region() and _regions.has() are two different things.

Was changing this function necessary for region streaming? If so, why?

TokisanGames · 2025-04-20T20:28:50Z

src/terrain_3d.cpp

@@ -1160,4 +1221,7 @@ void Terrain3D::_bind_methods() {

 	ADD_SIGNAL(MethodInfo("material_changed"));
 	ADD_SIGNAL(MethodInfo("assets_changed"));
+
+	ClassDB::bind_method(D_METHOD("set_streaming_distance", "distance"), &Terrain3D::set_streaming_distance);
+	ClassDB::bind_method(D_METHOD("get_streaming_distance"), &Terrain3D::get_streaming_distance);


No distance property has been exposed.

TokisanGames · 2025-04-20T20:50:39Z

src/terrain_3d_data.cpp

+
+	int regions_saved = 0;
+	int regions_removed = 0;
+	LOG(DEBUG, "Checking ", regions_keys.size(), " regions for cleanup");


This and Marking print continuously so should be marked EXTREME. This change can wait until closer to merge.

initial commit

cd5509b

fix: decrease stutter, decrease RAM usage

0c37180

TokisanGames requested changes Apr 20, 2025

View reviewed changes

TokisanGames added enhancement New feature or request big terrains Issues that need to be addressed for big terrains labels Apr 20, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat region streaming #675

feat region streaming #675

druidui commented Apr 12, 2025

druidui commented Apr 12, 2025

druidui commented Apr 13, 2025 •

edited

Loading

TokisanGames left a comment •

edited

Loading

TokisanGames Apr 20, 2025

TokisanGames Apr 20, 2025

TokisanGames Apr 20, 2025

TokisanGames Apr 20, 2025

TokisanGames Apr 20, 2025

TokisanGames Apr 20, 2025

TokisanGames Apr 20, 2025

TokisanGames Apr 20, 2025

feat region streaming #675

Are you sure you want to change the base?

feat region streaming #675

Conversation

druidui commented Apr 12, 2025

druidui commented Apr 12, 2025

druidui commented Apr 13, 2025 • edited Loading

TokisanGames left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

druidui commented Apr 13, 2025 •

edited

Loading

TokisanGames left a comment •

edited

Loading