Skip to content

Commit

Permalink
Merge branch 'master' into sqlite_large_coordinates
Browse files Browse the repository at this point in the history
# Conflicts:
#	streams/sqlite/voxel_stream_sqlite.cpp
  • Loading branch information
Zylann committed May 25, 2024
2 parents 48a58bf + 22e6dc9 commit 56da55a
Show file tree
Hide file tree
Showing 7 changed files with 79 additions and 72 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ bfoster68
gumby-cmyk
Joshua Woods (jpw1991)
jjoshpoland (Josh)
jbbieber1127 (John Bieber)
```

### Supporters
Expand Down
4 changes: 3 additions & 1 deletion doc/source/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ Semver is not yet in place, so each version can have breaking changes, although
- `VoxelViewer`: added `view_distance_vertical_ratio` to use different vertical view distance proportionally to the horizontal distance

- Fixes
- `VoxelStreamSQLite`: fixed `set_key_cache_enabled(true)` caused nothing to load
- `VoxelStreamSQLite`:
- Fixed `set_key_cache_enabled(true)` caused nothing to load
- Fixed slow loading when the database path contains `res:https://` or `user:https://`


1.2 - 20/04/2024 - branch `1.2` - tag `v1.2.0`
Expand Down
52 changes: 22 additions & 30 deletions doc/source/scripting.md
Original file line number Diff line number Diff line change
Expand Up @@ -150,52 +150,44 @@ Generators are invoked from multiple threads. Make sure your code is thread-safe

If your generator uses resources or exports parameters that you want to change while it might be running, you should make sure they are read-only or copied per thread, so if the resource is modified from outside or another thread it won't disrupt the generator.

You can use `Mutex` to enforce single-thread access to variables, but use it with caution because otherwise you could end up limiting performance to one thread (while the other waits for the lock to be released). Using Read-Write locks and thread-locals are good options, unfortunately the Godot script API does not provide this.

Careful about lazy-initialization, it can cause crashes if two threads run it at the same time. `Curve` is one of the resources doing that: if you call `interpolate_baked()` and it wasn't baked yet, it will be baked at the very last moment. Here is an example of working around this:
You can use `Mutex` to enforce single-thread access to variables that can be modified:

```gdscript
extends VoxelGeneratorScript
var _dictionary := {}
var _dictionary_mutex := Mutex.new()
const MountainsCurve : Curve = preload("moutains_curve.tres")
func _generate_block(...):
# ...
# This is called when the generator is created
func _init():
# Call `bake()` to be sure it doesn't happen later inside `generate_block()`.
MountainsCurve.bake()
_dictionary_mutex.lock()
# ...
var x := 0
if not _dictionary.has(key):
_dictionary[key] = x
else:
x = _dictionary[key]
_dictionary_mutex.unlock()
# ...
```

A similar story occurs with `Image`. It needs to be locked before you can access pixels, but calling `lock()` and `unlock()` itself is not thread-safe. One approach to solve this is to `lock()` the image in `_init()` and leave it locked for the whole lifetime of the generator. This assumes of course that the image is never accessed from outside:
However, mutexes must be used with a lot of care: if they are locked a lot of times or remain locked for too long, you could end up limiting performance to one thread (while the other waits for the lock to be released). If you use more than one and lock them in different orders, that can also lead to [deadlocks](https://en.wikipedia.org/wiki/Deadlock).
Using Read-Write locks and thread-locals are good options depending on the situation, unfortunately the Godot script API does not provide this.

```gdscript
extends VoxelGeneratorScript
Careful about lazy-initialization, it can cause crashes if two threads run it at the same time. `Curve` is one of the resources doing that: if you call `interpolate_baked()` and it wasn't baked yet, it will be baked at the very last moment. That involves modifying internal states which might overlap with other threads doing the same thing. Here is an example of working around this:

var image : Image
```gdscript
const MountainsCurve : Curve = preload("moutains_curve.tres")
# This is called when the generator is created
func _init():
image = Image.new()
image.load("some_heightmap.png")
image.lock()
func generate_block(buffer : VoxelBuffer, origin : Vector3i, lod : int) -> void:
# ... use image.get_pixel() freely ...
# ... but DO NOT use image.set_pixel() ...
func _notification(what: int):
if what == NOTIFICATION_PREDELETE:
# Called when the script is destroyed.
# I don't know if it's really required, but unlock for correctness.
image.unlock()
# Call `bake()` to be sure it doesn't happen later inside `generate_block()`.
MountainsCurve.bake()
# ...
```

Image.lock() won't be required anymore in Godot 4.



Custom stream
---------------
Expand Down
14 changes: 12 additions & 2 deletions doc/source/smooth_terrain.md
Original file line number Diff line number Diff line change
Expand Up @@ -212,8 +212,15 @@ WEIGHTS: aaaa bbbb cccc dddd

By default, these channels default to indices `(0,1,2,3)` and weights `(1,0,0,0)`, meaning voxels always start with texture `0`. Another rule is that indices must not appear twice (for example, indices `(0,1,1,1)` are invalid). If an index appears twice, in particular with different weights, it can cause ambiguous calculations.

The feature is recent and will need further work or changes in this area.
At the moment, indices and weights are mostly applied manually. It is possible to set them directly with `VoxelTool.set_voxel` but it is up to you to pack them properly. One easy way to paint is to use `VoxelTool.do_sphere()`:
At the moment, indices and weights are mostly applied manually. It is possible to set them directly with `VoxelTool.set_voxel` but it is up to you to pack them properly.

You may use `VoxelTool` helper functions to encode/decode these values:
- [vec4i_to_u16_indices](https://voxel-tools.readthedocs.io/en/latest/api/VoxelTool/#i_vec4i_to_u16_indices)
- [color_to_u16_weights](https://voxel-tools.readthedocs.io/en/latest/api/VoxelTool/#i_color_to_u16_weights)
- [u16_indices_to_vec4i](https://voxel-tools.readthedocs.io/en/latest/api/VoxelTool/#i_u16_indices_to_vec4i)
- [u16_weights_to_color](https://voxel-tools.readthedocs.io/en/latest/api/VoxelTool/#i_u16_weights_to_color)

One easy way to paint is to use `VoxelTool.do_sphere()`:

```gdscript
# Paints texture 2 in a sphere area (does not create matter)
Expand All @@ -225,6 +232,9 @@ voxel_tool.do_sphere(hit_position, radius)

It is also possible to generate this in `VoxelGeneratorGraph` using special outputs, but it still requires a bit of math to produce valid data.

See also this [painting demo](https://github.com/Zylann/voxelgame/tree/master/project/smooth_materials).


#### Mesh data

The mesher will include texturing information in the `CUSTOM1` attribute of vertices. Contrary to voxel values, the packed information will have 8 bits of precision:
Expand Down
1 change: 1 addition & 0 deletions editor/about_window.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,7 @@ VoxelAboutWindow::VoxelAboutWindow() {
"gumby-cmyk\n"
"Joshua Woods (jpw1991)\n"
"jjoshpoland (Josh)\n"
"jbbieber1127 (John Bieber)\n"
"\n"
"[b]Supporters:[/b]\n"
"rcorre (Ryan Roden-Corrent)\n"
Expand Down
75 changes: 37 additions & 38 deletions streams/sqlite/voxel_stream_sqlite.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include "../../util/errors.h"
#include "../../util/godot/classes/project_settings.h"
#include "../../util/godot/core/array.h"
#include "../../util/godot/core/string.h"
#include "../../util/math/conv.h"
#include "../../util/math/funcs.h"
#include "../../util/profiling.h"
Expand Down Expand Up @@ -1296,7 +1297,7 @@ VoxelStreamSQLite::VoxelStreamSQLite() {}

VoxelStreamSQLite::~VoxelStreamSQLite() {
ZN_PRINT_VERBOSE("~VoxelStreamSQLite");
if (!_connection_path.is_empty() && _cache.get_indicative_block_count() > 0) {
if (!_globalized_connection_path.empty() && _cache.get_indicative_block_count() > 0) {
ZN_PRINT_VERBOSE("~VoxelStreamSQLite flushy flushy");
flush_cache();
ZN_PRINT_VERBOSE("~VoxelStreamSQLite flushy done");
Expand All @@ -1310,20 +1311,17 @@ VoxelStreamSQLite::~VoxelStreamSQLite() {

void VoxelStreamSQLite::set_database_path(String path) {
MutexLock lock(_connection_mutex);
if (path == _connection_path) {
if (path == _user_specified_connection_path) {
return;
}
if (!_connection_path.is_empty() && _cache.get_indicative_block_count() > 0) {
if (!_globalized_connection_path.empty() && _cache.get_indicative_block_count() > 0) {
// Save cached data before changing the path.
// Not using get_connection() because it locks, we are already locked.
VoxelStreamSQLiteInternal con;
// To support Godot shortcuts like `user:https://` and `res:https://` (though the latter won't work on exported builds)
const String globalized_connection_path = ProjectSettings::get_singleton()->globalize_path(_connection_path);
const CharString cpath = globalized_connection_path.utf8();
// Note, the path could be invalid,
// Since Godot helpfully sets the property for every character typed in the inspector.
// So there can be lots of errors in the editor if you type it.
if (con.open(cpath.get_data(), to_internal_coordinate_format(_preferred_coordinate_format))) {
if (con.open(_globalized_connection_path.data(), to_internal_coordinate_format(_preferred_coordinate_format))) {
flush_cache_to_connection(&con);
}
}
Expand All @@ -1333,13 +1331,16 @@ void VoxelStreamSQLite::set_database_path(String path) {
_block_keys_cache.clear();
_connection_pool.clear();

_connection_path = path;
_user_specified_connection_path = path;
// To support Godot shortcuts like `user:https://` and `res:https://` (though the latter won't work on exported builds)
_globalized_connection_path = zylann::godot::to_std_string(ProjectSettings::get_singleton()->globalize_path(path));

// Don't actually open anything here. We'll do it only when necessary
}

String VoxelStreamSQLite::get_database_path() const {
MutexLock lock(_connection_mutex);
return _connection_path;
return _user_specified_connection_path;
}

void VoxelStreamSQLite::load_voxel_block(VoxelStream::VoxelQueryData &q) {
Expand Down Expand Up @@ -1661,31 +1662,29 @@ void VoxelStreamSQLite::flush_cache_to_connection(VoxelStreamSQLiteInternal *p_c
}

VoxelStreamSQLiteInternal *VoxelStreamSQLite::get_connection() {
_connection_mutex.lock();
if (_connection_path.is_empty()) {
_connection_mutex.unlock();
return nullptr;
}
if (_connection_pool.size() != 0) {
VoxelStreamSQLiteInternal *s = _connection_pool.back();
_connection_pool.pop_back();
_connection_mutex.unlock();
return s;
}
// First connection we get since we set the database path
StdString fpath;
CoordinateFormat preferred_coordinate_format;
{
MutexLock mlock(_connection_mutex);

const String fpath = _connection_path;
const CoordinateFormat preferred_coordinate_format = _preferred_coordinate_format;
_connection_mutex.unlock();
if (_globalized_connection_path.empty()) {
return nullptr;
}
if (_connection_pool.size() != 0) {
VoxelStreamSQLiteInternal *s = _connection_pool.back();
_connection_pool.pop_back();
return s;
}
// First connection we get since we set the database path
fpath = _globalized_connection_path;
preferred_coordinate_format = _preferred_coordinate_format;
}

if (fpath.is_empty()) {
if (fpath.empty()) {
return nullptr;
}
VoxelStreamSQLiteInternal *con = new VoxelStreamSQLiteInternal();
// To support Godot shortcuts like `user:https://` and `res:https://` (though the latter won't work on exported builds)
const String globalized_fpath = ProjectSettings::get_singleton()->globalize_path(fpath);
const CharString fpath_utf8 = globalized_fpath.utf8();
if (!con->open(fpath_utf8.get_data(), to_internal_coordinate_format(preferred_coordinate_format))) {
if (!con->open(fpath.data(), to_internal_coordinate_format(preferred_coordinate_format))) {
delete con;
con = nullptr;
}
Expand All @@ -1700,16 +1699,16 @@ VoxelStreamSQLiteInternal *VoxelStreamSQLite::get_connection() {
}

void VoxelStreamSQLite::recycle_connection(VoxelStreamSQLiteInternal *con) {
String con_path = con->get_opened_file_path();
_connection_mutex.lock();
// If path differs, delete this connection
if (_connection_path != con_path) {
_connection_mutex.unlock();
delete con;
} else {
_connection_pool.push_back(con);
_connection_mutex.unlock();
const char *con_path = con->get_opened_file_path();
// Put back in the pool if the connection path didn't change
{
MutexLock mlock(_connection_mutex);
if (_globalized_connection_path == con_path) {
_connection_pool.push_back(con);
return;
}
}
delete con;
}

void VoxelStreamSQLite::set_key_cache_enabled(bool enable) {
Expand Down
4 changes: 3 additions & 1 deletion streams/sqlite/voxel_stream_sqlite.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include "../../util/containers/std_unordered_set.h"
#include "../../util/containers/std_vector.h"
#include "../../util/math/vector3i16.h"
#include "../../util/string/std_string.h"
#include "../../util/thread/mutex.h"
#include "../voxel_block_serializer.h"
#include "../voxel_stream.h"
Expand Down Expand Up @@ -130,7 +131,8 @@ class VoxelStreamSQLite : public VoxelStream {

static void _bind_methods();

String _connection_path;
String _user_specified_connection_path;
StdString _globalized_connection_path;
StdVector<VoxelStreamSQLiteInternal *> _connection_pool;
Mutex _connection_mutex;
// This cache stores blocks in memory, and gets flushed to the database when big enough.
Expand Down

0 comments on commit 56da55a

Please sign in to comment.