-
-
Notifications
You must be signed in to change notification settings - Fork 156
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.
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
Godot4: support importing and exporting 32-bit raw files #369
Conversation
@Zylann: does the main branch (i.e. the Godot 3 version) support 24-bit? If so, I'd backport the import |
The Godot 3.6 version did not have 24-bit raw import. |
Sorry, that was imprecise. What I meant: Does it support 24-bit heightmaps internally? |
The Godot 3.6 version uses half-precision floats to store heightmaps, which is 16-bit. There is still a bit of advantage in importing a higher-precision heightmap, but most of the precision may be lost indeed. |
Alright, then I'll make a backport after this PR is merged |
5014c2b
to
98bf3c4
Compare
It's so that the user is forced to make a conscious choice in case sth. goes wrong, instead of being fed a potential incorrect fallback/default value.
And that we can set it to Undefined in case of other file types.
It's purely for UX reasons.
If you prefer to keep it simpler, i can remove the Undefined value and item disabling logic.
Which shall it be?
If the latter option, which bit depth shall be fallback/default?
Am 12. Juni 2023 17:38:19 UTC schrieb Marc ***@***.***>:
…
@Zylann commented on this pull request.
> @@ -37,7 +40,13 @@ func _ready():
"raw_endianess": {
"type": TYPE_INT,
"usage": "enum",
- "enum_items": ["Little Endian", "Big Endian"],
+ "enum_items": [[RAW_LITTLE_ENDIAN, "Little Endian"], [RAW_BIG_ENDIAN, "Big Endian"]],
+ "enabled": false
+ },
+ "bit_depth": {
+ "type": TYPE_INT,
+ "usage": "enum",
+ "enum_items": [[HTerrainData.BIT_DEPTH_UNDEFINED, "Undefined"], [HTerrainData.BIT_DEPTH_16, "16-bit"], [HTerrainData.BIT_DEPTH_24, "24-bit"]],
I dont understand why `set_property_enabled` can't just be enough? That's exactly what it's for, is already used for endianess, no need to add something special that only works with OptionButton.
--
Reply to this email directly or view it on GitHub:
#369 (comment)
You are receiving this because you authored the thread.
Message ID: ***@***.***>
|
I kinda lost track of this, how can the user make the wrong choice if the option is simply disabled when the import type changes? |
Ok, then I'll change it to the simpler version
---
how can the user make the wrong choice
For users that don't understand bit depth, use the wrong one, open issues instead of trying to select another bit depth
why would it need to be Undefined for other types
For quicker mental processing.
Some software disable Optionbuttons not necessarily because the choice does not matter right now, but to enforce the one and only valid option because of choices the user made before.
Some beginner users who had that experience might open issues asking "I want to change the bit depth for my PNG/whatever import"
Am 12. Juni 2023 18:32:52 UTC schrieb Marc ***@***.***>:
…I kinda lost track of this, how can the user make the wrong choice if the option is simply disabled when the import type changes?
And why would it need to be Undefined for other types, when those other types simply would not need to read it?
I would indeed prefer something simpler that does not involve creating two ways of disabling a property, when one exists already.
--
Reply to this email directly or view it on GitHub:
#369 (comment)
You are receiving this because you authored the thread.
Message ID: ***@***.***>
|
I see, so far I think |
@Zylann: should I (additionally) rewrite this to importing/exporting 32-bit raw files instead? |
If you still need to import/export 24-bit raw and it's a format known to be used by other software then there is no reason to not have it. Same if you want 32-bit. Note that "raw" isn't a format, what really counts is what's told to be in it, usually fixed-point, float or else. |
a469d86
to
380b82b
Compare
@Zylann: sorry for the delay. I think all your change requests are implemented. And it's now importing as 32-bit for various reasons |
@Zylann: removed debug logs and replaced if-else with clampi |
Thanks! |
Test file for importing
as tar.gz file, as github does not allow uploading .raw file: heightmap_24.raw.tar.gz
created with: https://github.com/cubiest/bmesh-map