godot4 branch: slight precision loss on encode_height_to_rgb8_unorm()

[MJacred]

EDIT: I did not round strictly enough in some cases and not at all in other cases. Now the formulas give almost the same results (my formulas are now a bit stricter than the code -> to avoid precision issues)

~~the decode func does not look like the opposite of the encode func to me. And I can't decode the correct value.~~

https://github.com/Zylann/godot_heightmap_plugin/blob/baeaba83345e1a5017587dd235749df5a528ef66/addons/zylann.hterrain/hterrain_data.gd#L1690-L1691

I set up an example in an ods document (and tried to deduce the magical numbers used in that area):

encode_decode.ods

Basically reproduced the code logic with actual height value (see ods for formulas):

[MJacred]

@Zylann: ok, I got it more or less sorted and adjusted my rounding. Now I see some small precision loss with the current implementation, though that should be neglectable (see screenshot).

EDIT: seems like there's an up to 10 cm difference (not tested with higher terrain yet) Exported: Imported

Changing 65536 to float (65536.0) might reduce some precision loss, but then the decode func goes haywire. An alternative would be to use bitshifting instead of modulo :thinking: but I currently don't see a big enough benefit

https://github.com/Zylann/godot_heightmap_plugin/blob/baeaba83345e1a5017587dd235749df5a528ef66/addons/zylann.hterrain/hterrain_data.gd#L1699

To test other places where imprecision is introduced: I increased the precision for step in the importer to 6 decimal places instead of the current 2. But this actually made the max height smaller by 0.001

Should I make a PR with unit testings? Though that would add the gdunit4 dependency

[Zylann]

Encoded heights cannot always represent heights exactly, encoding is slightly lossy to fit the data in 24 bits (floats are 32-bit, and GDScript floats are 64-bit). So part of what you are seeing is just the consequence of that. Now I suspected that there could be another source of precision loss in floating-point calculations (notably in shaders because they can't use doubles, unlike GDScript) because of relatively large temporary numbers, but havent seen striking issues with it so far.

I'm not sure about what you are suggesting? I don't think up to 10cm of difference is really significant when importing terrains that range hundreds of meters in height and 1-meter step in games... can you think of an improvement of the formula? Maybe there is an off-by-one?

I'm not used to Unit testing in GDScript, and havent evaluated the frameworks available so far so can't really judge. I'd prefer not to add a dependency on people installing the plugin, it would be dev only. Also it would be preferable to separate unit testing and the treatment of this issue if any. Looking at the screenshot of test functions, I don't know where Source, en or height come from...

Note: if you have a 24-bit heightmap, or a float heightmap, they will both loose a tiny bit of precision because the formats are differents either way. The representation inside the plugin is specific to the plugin. When you import a 24-bit RAW that contains a range let's say -1000 to 3000, it is not the same range the plugin uses (-8192..8192), therefore some precision will be lost by conversion to that range.

[MJacred]

I'm not sure about what you are suggesting? I don't think up to 10cm of difference is really significant

Hm… I'm not that worried about those 10 cm. On the first import, that is: If a user has a workflow of "import into Godot -> modify -> export -> rinse and repeat". The shifting will stack with each Godot import.

can you think of an improvement of the formula?

Right now: no. I have some ideas, but I need to test these out

I'd prefer not to add a dependency on people installing the plugin

Hm… I know what you mean. There's .gdignore, but that's also not that nice. The cleanest solution would be either a separate branch or repo : /
I'll leave them out

Looking at the screenshot of test functions, I don't know where Source, en or height come from...

They're from the spreadsheet in the issue description:

const __source = 'res://addons/zylann.hterrain/hterrain_data.gd'
const Source = preload(__source)

const encoded_rgb8:Vector3 = Vector3(0.0, 0.878431, 0.505882)
const approx_vec3:Vector3 = Vector3(0.000001, 0.000001, 0.000001)
const height:float = 120.0
const approx_float:float = 0.000001

[MJacred]

@Zylann: Actually, I just tested with the spreadsheet. And once I set _V2_MIN to -2048, the rounding error becomes less noticeable:

• same values as in screenshot of issue's description: precision error was 7E-06 (with current one, it's a difference of 0.020387)
  • this is really good
• for height range of 0-2048: precision error was 0.013596, (currently it would resolve to a difference of 0.017669)
  • not that much better

Tried some more height values/ranges, and -2048 seems to work better by a large margin. Sometimes even -1024 is better than -2048, dunno why. But anything lower than -2048 performs worse. Would need to run an automated benchmarking to be sure, though

EDIT: Hm… -2048 performs worse than 8192 (by ~20 cm), if height values are near full number (e.g. 300.1, 300.9)

EDIT 2: Argh, I'll just use a different way of encoding to rgb8. Tested it in golang. It was pretty much lossless. Will replicate it on weekend in Godot and test it. If it works, I'll make a PR