Import custom attributes with GLTFDocumentExtension#97756
Import custom attributes with GLTFDocumentExtension#97756huwpascoe wants to merge 1 commit intogodotengine:masterfrom
Conversation
|
I'm having a hard time understanding the extension. Can you link or write a proposal in https://github.com/godotengine/godot-proposals? Something like: I have a game workflow that uses blender I want the vertex attributes to import from blender, but it needs this pr. I can use the attributes for a shader effect. |
d58f372 to
b43538a
Compare
|
Refactored and removed like a hundred duplicate lines of code. _decode_accessor_as_floats();
_decode_accessor_as_ints();
_decode_accessor_as_vec2();
_decode_accessor_as_vec3();
_decode_accessor_as_color();
_decode_accessor_as_quaternion();
_decode_accessor_as_xform2d();
_decode_accessor_as_basis();
_decode_accessor_as_xform();
// VVV
_decode_accessor<T>();Multi-element scalars are now decoded with _decode_accessor<T, N>() array[Mesh::ARRAY_TANGENT] = _decode_accessor<float, 4>(p_state, a["TANGENT"], true, indices_mapping); |
|
Since this PR alters the accessor code, it will have to wait until after #94165 which also touches this code. |
b43538a to
5acc5ad
Compare
PR in question was just merged so we should be good to continue here. |
|
@huwpascoe Many people think this is a good enhancement. Would you happen to have time to rebase it? <3 |
|
Would it be better to keep as an extension or promote it to import options? There are trade-offs for each. |
|
I am trending towards promoting it to import options |
5acc5ad to
afab154
Compare
|
Remember to take this PR out of draft if it's ready :) |
Don't worry! I haven't forgotten.
Right, I've determined that the UI approach is just too much of a mess for what is a technical feature. So I'll be moving forward with the original plan to add the method to DocumentExtension. |
fd4dff7 to
4db4bdb
Compare
4db4bdb to
19491dc
Compare
8921298 to
e429182
Compare
f0f9cd4 to
6848be9
Compare
|
I've updated the test scene for the name change ( |
lyuma
left a comment
There was a problem hiding this comment.
I have some comments about the design that I'd like a chance to discuss.
Firstly, I have concerns about the usability of depending on a customized GLTFDocumentExtension to perform the remapping... it certainly could work, but document extensions are registered globally (project-wide), so any attempt to use this feature would have to condition the remapping on some condition or add a setting to the import dock. If I were to copy the sample code as written, it would break all other glTF models in my project, even if they do not have that custom attribute. But explaining all this is probably too much for a method description, so I'm not sure how to clarify this.
Second, as I noticed, if I use this to override any custom attributes, then I can no longer combine two UV channels into one custom channel which limits the number of custom 2-channel UVs I can import. I feel like maybe something simple like "TEXCOORD_1,TEXCOORD_2" could work to handle that case. I think it only needs to support 2 x vec2 or 1 x anything cases , so no need to handle every combination.
Basically the usecase is for compacting multiple 2-channel UVs into a CUSTOM, and this is Godot's current behavior so it would be nice to support it for that reason without needing a whole special case. At a minimum, it should be possible to return "" for an individual custom channel to get the current behavior without losing all the other custom channels.
That's why GLTFState is provided to determine if changes apply. If this method needs explaining, then every other method of GLTFDocumentExtension would require the same explanation. So that's out of scope, I think.
That sounds reasonable, I'll see if I can get it working efficiently. |
6848be9 to
ee7475d
Compare
f564792 to
0db8539
Compare
0db8539 to
2b47e72
Compare
|
updates? |

Proposal
Import ARRAY_CUSTOM# attributes with GLTFDocumentExtension using a function to specify layer name. Also added "Export Attributes" checkbox to the blender importer.
godotengine/godot-proposals#10892
Test Project
custom_attrib_test.zip
changes
Supported attributes