Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved error-handling for scaling #925

Merged
merged 1 commit into from
Jul 25, 2024
Merged

Improved error-handling for scaling #925

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions src/misc/math.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,28 @@

#define USEC_TO_SEC(m_usec) (double(m_usec) / 1000000.0)

#ifdef GDJ_CONFIG_EDITOR

#define ENSURE_SCALE_NOT_ZERO(m_transform, m_msg) \
if (unlikely((m_transform).basis.determinant() == 0.0f)) { \
WARN_PRINT(vformat( \
"%s " \
"The basis of the transform was singular, which is not supported by Godot Jolt. " \
"This is likely caused by one or more axes having a scale of zero. " \
"The basis (and thus its scale) will be treated as identity.", \
m_msg \
)); \
\
(m_transform).basis = Basis(); \
} else \
((void)0)

#else // GDJ_CONFIG_EDITOR

#define ENSURE_SCALE_NOT_ZERO(m_transform, m_msg)

#endif // GDJ_CONFIG_EDITOR

namespace godot::Math {

void decompose(Basis& p_basis, Vector3& p_scale);
Expand Down
17 changes: 4 additions & 13 deletions src/objects/jolt_body_impl_3d.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,19 +49,10 @@ JoltBodyImpl3D::~JoltBodyImpl3D() {
}

void JoltBodyImpl3D::set_transform(Transform3D p_transform) {
#ifdef GDJ_CONFIG_EDITOR
if (unlikely(p_transform.basis.determinant() == 0.0f)) {
ERR_PRINT(vformat(
"Failed to set transform for body '%s'. "
"Its basis was found to be singular, which is not supported by Godot Jolt. "
"This is likely caused by one or more axes having a scale of zero. "
"Its basis (and thus its scale) will be treated as identity.",
to_string()
));

p_transform.basis = Basis();
}
#endif // GDJ_CONFIG_EDITOR
ENSURE_SCALE_NOT_ZERO(
p_transform,
vformat("An invalid transform was passed to physics body '%s'.", to_string())
);

Vector3 new_scale;
Math::decompose(p_transform, new_scale);
Expand Down
94 changes: 36 additions & 58 deletions src/objects/jolt_shaped_object_impl_3d.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,21 +131,15 @@ JPH::ShapeRefC JoltShapedObjectImpl3D::try_build_shape() {
}

if (scale != Vector3(1, 1, 1)) {
#ifdef GDJ_CONFIG_EDITOR
if (unlikely(!result->IsValidScale(to_jolt(scale)))) {
ERR_PRINT(vformat(
"Godot Jolt failed to scale body '%s'. "
"%v is not a valid scale for the types of shapes in this body. "
"Its scale will instead be treated as (1, 1, 1).",
to_string(),
scale
));

scale = Vector3(1, 1, 1);
}
#endif // GDJ_CONFIG_EDITOR
Vector3 actual_scale = scale;

result = JoltShapeImpl3D::with_scale(result, scale);
ENSURE_SCALE_VALID(
result,
actual_scale,
vformat("Failed to correctly scale body '%s'.", to_string())
);

result = JoltShapeImpl3D::with_scale(result, actual_scale);
}

if (is_area()) {
Expand Down Expand Up @@ -195,20 +189,14 @@ void JoltShapedObjectImpl3D::add_shape(
Transform3D p_transform,
bool p_disabled
) {
#ifdef GDJ_CONFIG_EDITOR
if (unlikely(p_transform.basis.determinant() == 0.0f)) {
ERR_PRINT(vformat(
"Failed to set transform for shape at index %d of body '%s'. "
"Its basis was found to be singular, which is not supported by Godot Jolt. "
"This is likely caused by one or more axes having a scale of zero. "
"Its basis (and thus its scale) will be treated as identity.",
ENSURE_SCALE_NOT_ZERO(
p_transform,
vformat(
"An invalid transform was passed when adding shape at index %d to physics body '%s'.",
shapes.size(),
to_string()
));

p_transform.basis = Basis();
}
#endif // GDJ_CONFIG_EDITOR
)
);

Vector3 shape_scale;
Math::decompose(p_transform, shape_scale);
Expand Down Expand Up @@ -359,8 +347,8 @@ void JoltShapedObjectImpl3D::post_step(float p_step, JPH::Body& p_jolt_body) {

JPH::ShapeRefC JoltShapedObjectImpl3D::_try_build_single_shape() {
// NOLINTNEXTLINE(modernize-loop-convert)
for (int32_t i = 0; i < shapes.size(); ++i) {
const JoltShapeInstance3D& sub_shape = shapes[i];
for (int32_t shape_index = 0; shape_index < shapes.size(); ++shape_index) {
const JoltShapeInstance3D& sub_shape = shapes[shape_index];

if (!sub_shape.is_enabled() || !sub_shape.is_built()) {
continue;
Expand All @@ -372,20 +360,15 @@ JPH::ShapeRefC JoltShapedObjectImpl3D::_try_build_single_shape() {
const Transform3D sub_shape_transform = sub_shape.get_transform_unscaled();

if (sub_shape_scale != Vector3(1, 1, 1)) {
#ifdef GDJ_CONFIG_EDITOR
if (unlikely(!jolt_sub_shape->IsValidScale(to_jolt(sub_shape_scale)))) {
ERR_PRINT(vformat(
"Godot Jolt failed to scale shape at index %d for body '%s'. "
"%v is not a valid scale for this shape type. "
"Its scale will instead be treated as (1, 1, 1).",
i,
to_string(),
sub_shape_scale
));

sub_shape_scale = Vector3(1, 1, 1);
}
#endif // GDJ_CONFIG_EDITOR
ENSURE_SCALE_VALID(
jolt_sub_shape,
sub_shape_scale,
vformat(
"Failed to correctly scale shape at index %d in body '%s'.",
shape_index,
to_string()
)
);

jolt_sub_shape = JoltShapeImpl3D::with_scale(jolt_sub_shape, sub_shape_scale);
}
Expand All @@ -408,8 +391,8 @@ JPH::ShapeRefC JoltShapedObjectImpl3D::_try_build_compound_shape() {
JPH::StaticCompoundShapeSettings compound_shape_settings;

// NOLINTNEXTLINE(modernize-loop-convert)
for (int32_t i = 0; i < shapes.size(); ++i) {
const JoltShapeInstance3D& sub_shape = shapes[i];
for (int32_t shape_index = 0; shape_index < shapes.size(); ++shape_index) {
const JoltShapeInstance3D& sub_shape = shapes[shape_index];

if (!sub_shape.is_enabled() || !sub_shape.is_built()) {
continue;
Expand All @@ -421,20 +404,15 @@ JPH::ShapeRefC JoltShapedObjectImpl3D::_try_build_compound_shape() {
const Transform3D sub_shape_transform = sub_shape.get_transform_unscaled();

if (sub_shape_scale != Vector3(1, 1, 1)) {
#ifdef GDJ_CONFIG_EDITOR
if (unlikely(!jolt_sub_shape->IsValidScale(to_jolt(sub_shape_scale)))) {
ERR_PRINT(vformat(
"Godot Jolt failed to scale shape at index %d for body '%s'. "
"%v is not a valid scale for this shape type. "
"Its scale will instead be treated as (1, 1, 1).",
i,
to_string(),
sub_shape_scale
));

sub_shape_scale = Vector3(1, 1, 1);
}
#endif // GDJ_CONFIG_EDITOR
ENSURE_SCALE_VALID(
jolt_sub_shape,
sub_shape_scale,
vformat(
"Failed to correctly scale shape at index %d in body '%s'.",
shape_index,
to_string()
)
);

jolt_sub_shape = JoltShapeImpl3D::with_scale(jolt_sub_shape, sub_shape_scale);
}
Expand Down
14 changes: 14 additions & 0 deletions src/shapes/jolt_shape_impl_3d.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,20 @@ JPH::ShapeRefC JoltShapeImpl3D::without_custom_shapes(const JPH::Shape* p_shape)
}
}

Vector3 JoltShapeImpl3D::make_scale_valid(const JPH::Shape* p_shape, const Vector3& p_scale) {
return to_godot(p_shape->MakeScaleValid(to_jolt(p_scale)));
}

bool JoltShapeImpl3D::is_scale_valid(
const Vector3& p_scale,
const Vector3& p_valid_scale,
real_t p_tolerance
) {
return Math::is_equal_approx(p_scale.x, p_valid_scale.x, p_tolerance) &&
Math::is_equal_approx(p_scale.y, p_valid_scale.y, p_tolerance) &&
Math::is_equal_approx(p_scale.z, p_valid_scale.z, p_tolerance);
}

String JoltShapeImpl3D::_owners_to_string() const {
const int32_t owner_count = ref_counts_by_owner.size();

Expand Down
37 changes: 37 additions & 0 deletions src/shapes/jolt_shape_impl_3d.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,14 @@ class JoltShapeImpl3D {

static JPH::ShapeRefC without_custom_shapes(const JPH::Shape* p_shape);

static Vector3 make_scale_valid(const JPH::Shape* p_shape, const Vector3& p_scale);

static bool is_scale_valid(
const Vector3& p_scale,
const Vector3& p_valid_scale,
real_t p_tolerance = 0.01f
);

protected:
virtual JPH::ShapeRefC _build() const = 0;

Expand All @@ -75,3 +83,32 @@ class JoltShapeImpl3D {

JPH::ShapeRefC jolt_ref;
};

#ifdef GDJ_CONFIG_EDITOR

#define ERR_PRINT_INVALID_SCALE_MSG(m_scale, m_valid_scale, m_msg) \
if (unlikely(!JoltShapeImpl3D::is_scale_valid(m_scale, valid_scale))) { \
ERR_PRINT(vformat( \
"%s " \
"A scale of %v is not supported by Godot Jolt for this shape/body. " \
"The scale will instead be treated as %v.", \
m_msg, \
m_scale, \
valid_scale \
)); \
} else \
((void)0)

#else // GDJ_CONFIG_EDITOR

#define ERR_PRINT_INVALID_SCALE_MSG(m_scale, m_valid_scale, m_msg)

#endif // GDJ_CONFIG_EDITOR

#define ENSURE_SCALE_VALID(m_shape, m_scale, m_msg) \
if (true) { \
const Vector3 valid_scale = JoltShapeImpl3D::make_scale_valid(m_shape, m_scale); \
ERR_PRINT_INVALID_SCALE_MSG(m_scale, valid_scale, m_msg); \
(m_scale) = valid_scale; \
} else \
((void)0)
Loading