Add large world coordinates support#967
Add large world coordinates support#967jamonholmgren wants to merge 1 commit intoTokisanGames:mainfrom
Conversation
41ad840 to
986d2bf
Compare
986d2bf to
2729395
Compare
| f.x = Math::clamp(f.x, 1e-8f, 1.f - 1e-8f); | ||
| f.y = Math::clamp(f.y, 1e-8f, 1.f - 1e-8f); | ||
| f.x = Math::clamp(f.x, real_t(1e-8), real_t(1.0 - 1e-8)); | ||
| f.y = Math::clamp(f.y, real_t(1e-8), real_t(1.0 - 1e-8)); |
There was a problem hiding this comment.
1e-8f is a float literal that can be converted to a double for free.
1e-8 is a double that has a potential loss of precision and a cost to convert to float.
All single and double float literals should have f.
If a Godot macro or template complains about mixing types during a double precision build, then that literal can be wrapped in real_t() but still needs the f.
|
|
||
| inline uint8_t get_base(const uint32_t p_pixel) { return p_pixel >> 27 & 0x1F; } | ||
| inline uint8_t get_base(const float p_pixel) { return get_base(as_uint(p_pixel)); } | ||
| inline uint8_t get_base(const double p_pixel) { return get_base(static_cast<float>(p_pixel)); } |
There was a problem hiding this comment.
When would a pixel be a double?
Image.get_pixel() returns a Color
Color is exactly 4x 32-bit floats, not real_t, never doubles.
https://github.com/godotengine/godot/blob/master/core/math/color.h#L38-L49
Our control map data format is FORMAT_RF: exactly 1x 32-bit float per pixel, never a double. Those 32-bits are only a container for int data, not floats. It should never be converted or treated as a float. This function assumes the source integer data was converted from an invalid float to an invalid double, then converts it back to an invalid float in order to retrieve the integer data out of it. I would be amazed if that worked without data loss, but regardless this is not safe.
If you have a compiling typing error because of this, let's look at that separately, but this is not the solution.
| if (differs(a, b)) { \ | ||
| a = b; \ | ||
| if (differs(a, static_cast<std::remove_reference_t<decltype(a)>>(b))) { \ | ||
| a = static_cast<std::remove_reference_t<decltype(a)>>(b); \ |
There was a problem hiding this comment.
This change allows us to compare mixed types. When was that occurring? In the repo code we only use this macro on setters, where the incoming parameter and the member variable already have the exact same type. So what prompted you to make this change?
Double precision builds were working before, but occasionally we add code that mistakenly breaks it. Usually that just means wrapping something in real_t. If you ran into compiler errors that prompted you to do this, I'd like to know what they are.
As explained above sometimes a float isn't actually used as a float and shouldn't be converted. If we have an explicit float compared against a double, or an int compared with a float, this will now pass it when it should have broken the compile so we could fix it.
I've been working with Godot's large world coordinates build to try to address floating point imprecision issues with the large worlds that my game (Gunship Origins) requires. (Previous attempts include origin shifting, which comes with its own set of tradeoffs.)
When Godot is built with
precision=double,real_tbecomesdoubleinstead offloat. This causes three categories of compile errors in Terrain3D:1. Control-map pixel-unpacking helpers
The helpers
get_base,get_overlay,get_blend,get_uv_rotation,get_uv_scale,is_hole,is_nav,is_autoonly havefloatoverloads. Callers passdoublevalues from Godot's Image API, causing errors.I added
doubleoverloads that cast tofloat.2.
SET_IF_DIFFerrorsThe
differs<T>template errors when the types mismatch.I switched to use
std::remove_reference_t+static_castso both sides have the same type.3.
Math::clampin collision codeMath::clampusesfloatliterals (1e-8f) but the first argument isreal_t(nowdouble), causing errors.I now wrap literals in
real_t().Tested against Godot 4.6-stable built with
precision=doubleand godot-cpp master. Terrain3D appears to work correctly, but more testing would be appreciated, whether you're using double precision builds or not.My C++ skills aren't as good as they could be; feedback welcome.