Skip to content

Commit

Permalink
Fix skinning initialization in MeshInstance when loaded from thread
Browse files Browse the repository at this point in the history
Fix for a regression from software skinning support:
instance_attach_skeleton wasn't called in set_mesh before, and it's
causing issues when the mesh instance is loaded from a thread.
1. Call from a thread queues instance_attach_skeleton with RID() in the
visual server.
2. Call from the main thread when entering tree calls
instance_attach_skeleton immediately with a valid skeleton
3. Queued instance_attach_skeleton resets the attached skeleton

This change prevents that to happen by making sure
instance_attach_skeleton is not called on set_mesh as it was doing
before, but there might be a more general problem to solve in how
visual server commands are executed when resources are loaded from
a different thread.
  • Loading branch information
pouleyKetchoupp committed Apr 26, 2021
1 parent 595a74c commit feee9f9
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 6 deletions.
16 changes: 11 additions & 5 deletions scene/3d/mesh_instance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ void MeshInstance::set_mesh(const Ref<Mesh> &p_mesh) {
mesh->connect(CoreStringNames::get_singleton()->changed, this, SceneStringNames::get_singleton()->_mesh_changed);
materials.resize(mesh->get_surface_count());

_initialize_skinning();
_initialize_skinning(false, false);
} else {

set_base(RID());
Expand Down Expand Up @@ -208,7 +208,7 @@ bool MeshInstance::_is_software_skinning_enabled() const {
return global_software_skinning;
}

void MeshInstance::_initialize_skinning(bool p_force_reset) {
void MeshInstance::_initialize_skinning(bool p_force_reset, bool p_call_attach_skeleton) {
if (mesh.is_null()) {
return;
}
Expand Down Expand Up @@ -324,7 +324,9 @@ void MeshInstance::_initialize_skinning(bool p_force_reset) {
update_mesh = true;
}

visual_server->instance_attach_skeleton(get_instance(), RID());
if (p_call_attach_skeleton) {
visual_server->instance_attach_skeleton(get_instance(), RID());
}

if (is_visible_in_tree() && (software_skinning_flags & SoftwareSkinning::FLAG_BONES_READY)) {
// Intialize from current skeleton pose.
Expand All @@ -336,7 +338,9 @@ void MeshInstance::_initialize_skinning(bool p_force_reset) {
skin_ref->get_skeleton_node()->disconnect("skeleton_updated", this, "_update_skinning");
}

visual_server->instance_attach_skeleton(get_instance(), skin_ref->get_skeleton());
if (p_call_attach_skeleton) {
visual_server->instance_attach_skeleton(get_instance(), skin_ref->get_skeleton());
}

if (software_skinning) {
memdelete(software_skinning);
Expand All @@ -345,7 +349,9 @@ void MeshInstance::_initialize_skinning(bool p_force_reset) {
}
}
} else {
visual_server->instance_attach_skeleton(get_instance(), RID());
if (p_call_attach_skeleton) {
visual_server->instance_attach_skeleton(get_instance(), RID());
}
if (software_skinning) {
memdelete(software_skinning);
software_skinning = nullptr;
Expand Down
2 changes: 1 addition & 1 deletion scene/3d/mesh_instance.h
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ class MeshInstance : public GeometryInstance {
bool _is_software_skinning_enabled() const;
static bool _is_global_software_skinning_enabled();

void _initialize_skinning(bool p_force_reset = false);
void _initialize_skinning(bool p_force_reset = false, bool p_call_attach_skeleton = true);
void _update_skinning();

protected:
Expand Down

0 comments on commit feee9f9

Please sign in to comment.