Time  Nick        Message
06:00 nerzhul     hello, merging #9665 (trailing whitespaces fixed)
06:00 ShadowBot   https://github.com/minetest/minetest/issues/9665 -- Verify database connection on interval by nerzhul
06:01 nerzhul     and #9674
06:01 ShadowBot   https://github.com/minetest/minetest/issues/9674 -- Fix parsing JSON with large integers by sfan5
06:09 nerzhul     nice to see increasing activity and bugfixes :)
06:11 nerzhul     sfan5: #9671 has been fixed and is ready for a last review
06:11 ShadowBot   https://github.com/minetest/minetest/issues/9671 -- Optimize get_object_inside_radius call by nerzhul
08:13 nerzhul     new MR for our build architecture: #9677
08:13 ShadowBot   https://github.com/minetest/minetest/issues/9677 -- Add an option to disable unittest build, & disable them on Docker build by nerzhul
14:27 nerzhul     finally merging #9066
14:27 ShadowBot   https://github.com/minetest/minetest/issues/9066 -- Android: add Android Studio support, completely redone build system and java part by MoNTE48
15:10 sfan5       Termos19: hi
15:15 Termos19    do you read me?
15:15 sfan5       yes
15:18 sfan5       Termos19: I added some debug code locally to look at the collision calculations https://0x0.st/iQVi.txt
15:18 sfan5       the output there is from the same situation as the reproduction video, jumping in a corner and getting stuck in air
15:19 sfan5       while the float calculations there are pretty close, they are not incorrect: 16155.000000 - 16139.000977 - 16.000000 = -0.000976999999693362 which is indeed less than 0
15:20 sfan5       so I think this is a red herring and the actual problem lies in the fact that the player is somehow allowed to get too close to the nodes
15:20 sfan5       thoughts?
15:21 Termos19    ok let me read the code
15:22 sfan5       it's basically just printing out the comparison that happens inside the if (.........) return COLLISION_AXIS_Y;
15:27 Termos19    The problem I think is how we get this strange figure: 16139.000977. It's the sum of the object position (big number) and box extent (small number
15:29 Termos19    that's where f32 usually go wrong. I provided an example in #9343 issue how that happens
15:29 ShadowBot   https://github.com/minetest/minetest/issues/9343 -- Collision various fixes by TheTermos
15:29 sfan5       since node extents are perfectly constant I'd assume the 0.000977 comes from the player position, right?
15:30 Termos19    I don't think so, many zeros and then something is usually fp precision error, it's probably a miscalculated whole number
15:31 sfan5       hm
15:32 Termos19    with that a big number only 3 decimal places are usable, what follows is precision noise
15:32 sfan5       then why not modify the checks from A - B - C < 0 to A - B - C < (-0.001f)
15:32 sfan5       would work, wouldn't it?
15:33 sfan5       hm, in fact isn't this what we had before with the "d" constant(?)
15:35 Termos19    no, that was different, worked on different principle, and d was more than quarter of a node originally.
15:35 sfan5       I see
15:36 p_gimeno    switching to minkowski difference + raycasting is not an option at this point, I guess?
15:38 Termos19    As I mentioned, I have a version that creates a safety cushion between boxes, but maybe investigating that flag as well wouldn't hurt?
15:39 sfan5       yes
15:42 sfan5       where can I find that other version of the code btw?
15:42 Termos19    Nowhere online atm, I'm going to put it up on github
15:49 Termos19    but here: https://github.com/TheTermos/minetest/blob/41dd0c290b09f923fe5cc1ac63bbd1dc17354388/src/collision.cpp
15:50 Termos19    there's that commented out part at line 86, that used to be it
15:53 Termos19    0.03 is about the error that can theoretically happen around 31000
16:13 sfan5       I see
16:16 Termos19    anyway, this poor precision causes also other problems.
16:18 Termos19    should have been 64 bit
16:23 nerzhul     sfan5 if you get time there is some PR to review :D
16:23 nerzhul     #9671 is ready for integration
16:23 ShadowBot   https://github.com/minetest/minetest/issues/9671 -- Optimize get_object_inside_radius call by nerzhul
16:23 nerzhul     #9676 needs review
16:23 ShadowBot   https://github.com/minetest/minetest/issues/9676 -- Fix GCC warning spam on ABI by nerzhul
16:31 Termos19    so I'm going to make a new version just in case
16:31 Termos19    Too bad I cant test this stuff with different compilers
16:33 sfan5       most people use gcc so if -ffast-math breaks it there, that's what matters
16:33 rubenwardy  that's what CI is for
16:34 sfan5       I don't think it's easy to reduce this into an unittest
16:37 Krock       well, floating point precision tests would be possible
16:41 p_gimeno    fast math breaks many assumptions, http://www.formauri.es/personal/pgimeno/pastes/fast-math.c
16:43 p_gimeno    that was causing #3943 too
16:43 ShadowBot   https://github.com/minetest/minetest/issues/3943 -- testStreamRead and testBufReader failures
16:44 Krock       pretty sure I saw that just recently
16:44 sfan5       those aren't a problem though if you compare using abs(a-b) < 0.0001f instead of equality
16:44 Krock       Test assertion failed: readF1000(is) == 53.534f
16:44 Krock       at test_serialization.cpp:308
16:44 Krock       still happening in current master
16:45 sfan5       maybe we really should just disable -ffast-math
16:45 nerzhul     i think it's better, we don't need a such optimization and stop loosing time on it. In 2017 we were already discussion about this
16:46 rubenwardy  comparing using  abs(a-b) < 0.0001f   is the correct way to compare floats
16:46 p_gimeno    Krock: yeah I was sceptical it was fixed :)
16:47 p_gimeno    and no, that's not the correct way to compare floats, it's not how you compare them in Lua and it should not how you compare them in proper code
16:47 p_gimeno    should not be*
16:48 rubenwardy  it's how I compare them in Lua \o/
16:48 p_gimeno    it's how you compare them when you have no guarantees about the correctness of the underlying operations
16:48 sfan5       do you ever have that?
16:48 p_gimeno    do you mean in Lua you do: if math.abs(index - 1) < 0.0001 then ... instead of: if index == 1 then ... ?
16:49 rubenwardy  no
16:49 rubenwardy  https://gitlab.com/rubenwardy/conquer/-/blob/master/tests/utils_spec.lua#L22
16:50 p_gimeno    sfan5: yes, IEEE758 guarantees correct rounding for addition, subtraction, multiplication, division, square root and fmaf/fma, and GCC (and other compilers) has modes to switch it on
16:50 p_gimeno    but most of the time, in GCC you switch off fast-math and get that
16:51 sfan5       ok but 10/1000 will still not equal 10 * 0.0001, will it?
16:52 p_gimeno    sfan5: correct
16:53 sfan5       well * 0.001 but I think you get what I mean
16:53 p_gimeno    but division will perform the former without optimizing to the latter
16:53 sfan5       yes that's one of the things -ffast-math enables
16:53 sfan5       but my point is
16:53 sfan5       in real-world mathematics those two are definitely equal
16:54 sfan5       so you still end up with a need to do "fuzzy" equality comparison
16:54 p_gimeno    you don't need to do range comparisons in many cases if you know what you're doing
16:54 p_gimeno    that unit test is one example
16:55 p_gimeno    but with -ffast-math you lose the guarantees that allow you to know what you're doing
16:55 p_gimeno    this is an essay on how it behaves and what you can expect from it: https://docs.oracle.com/cd/E19957-01/806-3568/ncg_goldberg.html
16:56 p_gimeno    one of the guarantees you have with correct rounding is that division of a number by itself will give 1, not e.g. 1.000001
16:56 p_gimeno    and that is important in many cases
16:57 nerzhul     sfan5: #9676 fixed
16:57 ShadowBot   https://github.com/minetest/minetest/issues/9676 -- Fix GCC warning spam on ABI by nerzhul
17:01 sfan5       nerzhul: with which gcc version did you see the Wabi warnings?
17:01 nerzhul     9 and 8
17:02 sfan5       right
17:07 sfan5       hm actually
17:07 sfan5       I have no idea which -Wabi= we should be using anyway
17:08 sfan5       https://gcc.gnu.org/onlinedocs/gcc/C_002b_002b-Dialect-Options.html
17:08 sfan5       -Wabi=8 for compatibility with 4.9? why at all?
17:08 rubenwardy  should we be using that check? We're not a library, we don't care about consistent ABIs
17:08 Termos19    anyway, I'd try getting rid of the flag if that's possible, otherwise I have the modification so we're going to be ok.
17:09 sfan5       rubenwardy: right that's what I was thinking
17:09 nerzhul     we should use 11 to be c++ 1 compliant, 8 is not complete if i remember
17:10 rubenwardy  is it about c++ standards compliance or abi stability?
17:10 rubenwardy  I've not read the docs
17:11 sfan5       > Warn when G++ it generates code that is probably not compatible with the vendor-neutral C++ ABI
17:11 sfan5       so does that matter for linking to shared libraries? or only if you expose functions yourself?
17:11 sfan5       and does it even matter at all when distros use *only* gcc or *only* clang?
17:16 sfan5       nerzhul: can you change it to -Wabi=7 and see if the CI shows any warnings?
17:23 Termos19    sfan, in that debug, it could be helpful to know what the input was (the boxes and speed)
17:34 nerzhul     sfan5 sounds some sorcery no ? when the flag is not present gcc asked me to set explicitely abi=11
17:45 sfan5       that's just an example from the gcc devs
17:45 sfan5       Termos19: i'll try to print those too
17:55 sfan5       Termos19: https://0x0.st/iQWD.txt
17:56 rubenwardy  What's this syntax called? I've not seen it before
17:56 rubenwardy  https://github.com/minetest/minetest/pull/9668/files#diff-fc5bd7f599797bce163af517e57a314cR1998
17:56 rubenwardy  !title
17:56 ShadowBot   Collision information for Lua entities by sfan5 · Pull Request #9668 · minetest/minetest · GitHub
17:56 rubenwardy  named elements?
17:56 rubenwardy  static const char *collision_type_str[] = { [COLLISION_NODE] = "node", [COLLISION_OBJECT] = "object",  }
17:57 sfan5       https://stackoverflow.com/a/17581108
17:57 rubenwardy  thanks
17:57 sfan5       " C++ doesn't support the same behaviour." ?
17:57 rubenwardy  uh oh
17:59 sfan5       none of the compilers on CI had anything to say about it
18:06 nerzhul     quite old answer, 2013
18:06 sfan5       https://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html#Designated-Inits
18:07 sfan5       even gcc documentation says you can't do this in C++ (not even as an extension)
18:07 sfan5       but this is evidently wrong
18:08 nerzhul     gcc devs: we don't know how our compiler works, but it works like we wanted to od
18:08 nerzhul     do*
18:09 nerzhul     sfan5 you approved & unapproved my abi PR, what do you want more ? i tend to respect the parameter asked by gcc with the value it mentioned than trying another obscure value
18:10 sfan5       it's unclear whether -Wabi makes any sense for Minetest at all and -Wabi=11 is still just an example in gcc's warning
18:10 nerzhul     the -Wabi was for old clang compiler if i remember, and the -Wabi=11 is just for gcc 7 and upper
18:38 sfan5       oh god the item entities collision performs horribly if you leave -ffast-math enabled
18:42 DS-minetest would it be possible to force-disable -ffast-math on parts of the code?
18:43 sfan5       it sure would but why not disable it globally then?
18:54 sfan5       rubenwardy: 30 seconds of -ffast-math (or insufficient fp comparisons, depending on your viewpoint) https://0x0.st/iQ4J.mp4
18:55 Termos19    Thanks sfan, I'll post on github if I find something interesting
18:55 sfan5       sure
20:17 Termos19    unfortunately that debug isn't very useful, all calculations seem correct, the box is in fact in collision with y plane. the problem occured earlier when it clipped along x.
20:50 sfan5       p_gimeno: https://github.com/minetest/minetest/pull/9682
21:24 Termos19    axisallignedcollision seems to works correctly, the problem with fast math occurs outside in collisionmovesimple. the < 0 comparison is correct.