godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
91.07k stars 21.18k forks source link

Warnings "AABB size is negative" when importing glTF file with zero-weight bones #56458

Closed lukas-toenne closed 1 year ago

lukas-toenne commented 2 years ago

Bugsquad note: This issue has been confirmed several times already. No need to confirm it further.


Godot version

master (937fb63a)

System information

Windows 10

Issue description

I'm seeing a lot of "AABB size is negative" spam when importing a glTF file with a skeleton, because some bones have zero weights (non-deform bones) and have an "empty" AABB. RendererStorageRD::mesh_add_surface still tries to merge them with the mesh AABB.

        for (int i = 0; i < p_surface.bone_aabbs.size(); i++) {
            mesh->bone_aabbs.write[i].merge_with(p_surface.bone_aabbs[i]);
        }
        mesh->aabb.merge_with(p_surface.aabb);

This is caused by the recent addition of warnings in AABB::merge_with: #37626 I have a tentative fix by just checking if both AABBs are valid ("has_no_volume"), but not sure if that's a proper solution. I agree with commenters that these warnings should not be issued for a valid case like this.

        for (int i = 0; i < p_surface.bone_aabbs.size(); i++) {
            if (p_surface.bone_aabbs[i].has_no_volume()) {
                continue;
            }
            if (mesh->bone_aabbs.write[i].has_no_volume()) {
                mesh->bone_aabbs.write[i] = p_surface.bone_aabbs[i];
            } else {
                mesh->bone_aabbs.write[i].merge_with(p_surface.bone_aabbs[i]);
            }
        }
        mesh->aabb.merge_with(p_surface.aabb);

Steps to reproduce

  1. Open the attached project
  2. Select the Beetle.glb file in the asset browser
  3. Open the Import tab
  4. Click the Reimport button

Minimal reproduction project

BoneAABBWarnings.zip

lukas-toenne commented 2 years ago

Related issue: #56007

akien-mga commented 2 years ago

CC @aaronfranke @lawnjelly

aaronfranke commented 2 years ago

The problem is caused by this code in servers/rendering_server.cpp:659

aabb-rendering-server

I looked around at the code, it doesn't seem that there are any checks for this value specifically, so it could easily be (0, 0, 0) or (-5, -5, -5). The former would represent no size, which judging by this comment // Need some size otherwise will get culled in servers/rendering/renderer_rd/render_storage_rd.cpp:6218 I guess a zero size AABB will be culled.

However, I think that this warning has revealed a more serious bug. I think it's a bug that the rendering code tries to merge bone AABBs that are explicitly marked as unused here in the rendering server. If I'm not mistaken, this means that for bones the position at the origin is always included in the AABB, even if the bones are not at the origin. This probably also applies to the position (-1, -1, -1).

I think the fix is that the merging code mesh->bone_aabbs.write[i].merge_with(p_surface.bone_aabbs[i]); in servers/rendering/renderer_rd/render_storage_rd.cpp:3271 should have a check for if the size == Vector3(-1, -1, -1) and skip those, or alternatively there could be a check when the AABBs are being generated to just not have the Vector<AABB> include AABBs that are supposed to be unused. This needs some @godotengine/rendering folks to look into this further.

fire commented 2 years ago

Is there a aabb has volume method?

aaronfranke commented 2 years ago

@fire https://github.com/godotengine/godot/blob/master/core/math/aabb.h#L50

If we want the solution of checking the AABB before trying to merge it, this check would work well.

nonunknown commented 2 years ago

I can confirm this bug still occurs in v4.0.alpha.mono.custom_build [ac591d990]

do you guys have a workaround for now?

kalysti commented 2 years ago

Same issue. Mono master branch. Game stops with error image

njdf18 commented 2 years ago

I got same error on Mono master branch too. It occurs when loading scene including models or adding models to scene. image

Valeryn4 commented 2 years ago

problem actual last master branch. image image

LOG

glTF: accessor offset 0 view offset: 7048876 total buffer len: 7055660 view len 384
glTF: type float component type: Float stride: 4 amount 32
glTF: accessor offset 0 view offset: 4363748 total buffer len: 7055660 view len 128
glTF: type vec3 component type: Float stride: 12 amount 32
glTF: accessor offset 0 view offset: 7049260 total buffer len: 7055660 view len 384
glTF: type float component type: Float stride: 4 amount 32
glTF: accessor offset 0 view offset: 4363748 total buffer len: 7055660 view len 128
glTF: type vec4 component type: Float stride: 16 amount 32
glTF: accessor offset 0 view offset: 7049644 total buffer len: 7055660 view len 512
glTF: type float component type: Float stride: 4 amount 32
glTF: accessor offset 0 view offset: 4363748 total buffer len: 7055660 view len 128
glTF: type vec3 component type: Float stride: 12 amount 32
glTF: accessor offset 0 view offset: 7050156 total buffer len: 7055660 view len 384
glTF: type float component type: Float stride: 4 amount 32
glTF: accessor offset 0 view offset: 4363748 total buffer len: 7055660 view len 128
glTF: type vec3 component type: Float stride: 12 amount 32
glTF: accessor offset 0 view offset: 7050540 total buffer len: 7055660 view len 384
glTF: type float component type: Float stride: 4 amount 32
glTF: accessor offset 0 view offset: 4363748 total buffer len: 7055660 view len 128
glTF: type vec4 component type: Float stride: 16 amount 32
glTF: accessor offset 0 view offset: 7050924 total buffer len: 7055660 view len 512
glTF: type float component type: Float stride: 4 amount 32
glTF: accessor offset 0 view offset: 4363748 total buffer len: 7055660 view len 128
glTF: type vec3 component type: Float stride: 12 amount 32
glTF: accessor offset 0 view offset: 7051436 total buffer len: 7055660 view len 384
glTF: type float component type: Float stride: 4 amount 32
glTF: accessor offset 0 view offset: 4363748 total buffer len: 7055660 view len 128
glTF: type vec3 component type: Float stride: 12 amount 32
glTF: accessor offset 0 view offset: 7051820 total buffer len: 7055660 view len 384
glTF: type float component type: Float stride: 4 amount 32
glTF: accessor offset 0 view offset: 4363748 total buffer len: 7055660 view len 128
glTF: type vec4 component type: Float stride: 16 amount 32
glTF: accessor offset 0 view offset: 7052204 total buffer len: 7055660 view len 512
glTF: type float component type: Float stride: 4 amount 32
glTF: accessor offset 0 view offset: 4363748 total buffer len: 7055660 view len 128
glTF: type vec3 component type: Float stride: 12 amount 32
glTF: accessor offset 0 view offset: 7052716 total buffer len: 7055660 view len 384
glTF: type float component type: Float stride: 4 amount 32
glTF: accessor offset 0 view offset: 4363748 total buffer len: 7055660 view len 128
glTF: type vec3 component type: Float stride: 12 amount 32
glTF: accessor offset 0 view offset: 7053100 total buffer len: 7055660 view len 384
glTF: type float component type: Float stride: 4 amount 32
glTF: accessor offset 0 view offset: 4363748 total buffer len: 7055660 view len 128
glTF: type vec4 component type: Float stride: 16 amount 32
glTF: accessor offset 0 view offset: 7053484 total buffer len: 7055660 view len 512
glTF: type float component type: Float stride: 4 amount 32
glTF: accessor offset 0 view offset: 4363748 total buffer len: 7055660 view len 128
glTF: type vec3 component type: Float stride: 12 amount 32
glTF: accessor offset 0 view offset: 7053996 total buffer len: 7055660 view len 384
glTF: type float component type: Float stride: 4 amount 32
glTF: accessor offset 0 view offset: 4363748 total buffer len: 7055660 view len 128
glTF: type vec3 component type: Float stride: 12 amount 32
glTF: accessor offset 0 view offset: 7054380 total buffer len: 7055660 view len 384
glTF: type float component type: Float stride: 4 amount 32
glTF: accessor offset 0 view offset: 4363748 total buffer len: 7055660 view len 128
glTF: type vec4 component type: Float stride: 16 amount 32
glTF: accessor offset 0 view offset: 7054764 total buffer len: 7055660 view len 512
glTF: type float component type: Float stride: 4 amount 32
glTF: accessor offset 0 view offset: 4363748 total buffer len: 7055660 view len 128
glTF: type vec3 component type: Float stride: 12 amount 32
glTF: accessor offset 0 view offset: 7055276 total buffer len: 7055660 view len 384
glTF: Total animations '20'.
glTF: Converting spatial: Armature
glTF: Creating mesh for: backpack10
glTF: Creating mesh for: backpack13
glTF: Creating mesh for: backpack15
glTF: Creating mesh for: backpack16
glTF: Creating mesh for: backpack17
glTF: Creating mesh for: backpack18
glTF: Creating mesh for: backpack19
glTF: Creating mesh for: backpack20
glTF: Creating mesh for: backpack21
glTF: Creating mesh for: backpack3
glTF: Creating mesh for: backpack4
glTF: Creating mesh for: backpack5
glTF: Creating mesh for: backpack6
glTF: Creating mesh for: backpack7
glTF: Creating mesh for: backpack8
glTF: Creating mesh for: backpack9
glTF: Creating mesh for: belt
glTF: Creating mesh for: belt4
glTF: Creating mesh for: belt5
glTF: Creating mesh for: belt6
glTF: Creating mesh for: button
glTF: Creating mesh for: button1
glTF: Creating mesh for: button10
glTF: Creating mesh for: button11
glTF: Creating mesh for: button12
glTF: Creating mesh for: button13
glTF: Creating mesh for: button14
glTF: Creating mesh for: button15
glTF: Creating mesh for: button16
glTF: Creating mesh for: button17
glTF: Creating mesh for: button18
glTF: Creating mesh for: button19
glTF: Creating mesh for: button2
glTF: Creating mesh for: button20
glTF: Creating mesh for: button21
glTF: Creating mesh for: button22
glTF: Creating mesh for: button23
glTF: Creating mesh for: button24
glTF: Creating mesh for: button25
glTF: Creating mesh for: button26
glTF: Creating mesh for: button27
glTF: Creating mesh for: button28
glTF: Creating mesh for: button29
glTF: Creating mesh for: button3
glTF: Creating mesh for: button4
glTF: Creating mesh for: button5
glTF: Creating mesh for: button6
glTF: Creating mesh for: button7
glTF: Creating mesh for: button8
glTF: Creating mesh for: button9
glTF: Creating mesh for: gasmask
glTF: Creating mesh for: gasmask1
glTF: Creating mesh for: gasmask2
glTF: Creating mesh for: gasmask3
glTF: Creating mesh for: gasmask4
glTF: Creating mesh for: gasmask5
glTF: Creating mesh for: gasmask6
glTF: Creating mesh for: gasmask7
glTF: Creating mesh for: gasmask8
glTF: Creating mesh for: gasmask9
glTF: Creating mesh for: Group41582
glTF: Creating mesh for: Group41583
glTF: Creating mesh for: Group41584
glTF: Creating mesh for: Group6944
glTF: Creating mesh for: helmet10
glTF: Creating mesh for: helmet11
glTF: Creating mesh for: helmet12
glTF: Creating mesh for: helmet13
glTF: Creating mesh for: helmet4
glTF: Creating mesh for: helmet5
glTF: Creating mesh for: helmet6
glTF: Creating mesh for: helmet7
glTF: Creating mesh for: helmet8
glTF: Creating mesh for: helmet9
glTF: Creating mesh for: left_hand
glTF: Creating mesh for: left_legright_bootmesh_GRP
glTF: Creating mesh for: needle1
glTF: Creating mesh for: pants3
glTF: Creating mesh for: pSphere1
glTF: Creating mesh for: pSphere2
glTF: Creating mesh for: respirator2
glTF: Creating mesh for: respirator3
glTF: Creating mesh for: respirator4
glTF: Creating mesh for: respirator6
glTF: Creating mesh for: respirator_tube
glTF: Creating mesh for: right_bootmesh_GRP
glTF: Creating mesh for: right_hand
glTF: Creating mesh for: shoulderpads2
glTF: Creating mesh for: shoulderpads3
glTF: Creating mesh for: shoulderpads4
glTF: Creating mesh for: shoulderpads5
glTF: Creating mesh for: shovelgrab
glTF: Creating mesh for: shovelgrab1
glTF: Creating mesh for: shovelhead1
glTF: Creating mesh for: shovelstraps
glTF: Creating mesh for: trenchcoat_lower
glTF: Creating mesh for: trenchcoat_upper
ERROR: AABB size is negative, this is not supported. Use AABB.abs() to get an AABB with a positive size.
   at: AABB::merge_with (core\math\aabb.cpp:51)
ERROR: AABB size is negative, this is not supported. Use AABB.abs() to get an AABB with a positive size.

GLB: gasmask_character.zip

Doesn't work in 4.0-master. In 3.3 it works.

aaronfranke commented 2 years ago

This is still an issue that we encounter frequently. My previous investigation should still have accurate information, but this issue needs somebody more familiar with rendering/bones/weights to decide what the proper fix is.

kalysti commented 2 years ago

Somebody have an temp fix for this issue?

darrylryan commented 2 years ago

+1 I am hitting this too.. its quite a serious bug I think because its one that leads most people to run around in circles for some time before realizing it is an engine bug :D

kalysti commented 2 years ago

Testing the fix, works well for me.

Dal @.***> schrieb am Fr., 8. Juli 2022, 20:11:

+1 I am hitting this too.. its quite a serious bug I think because its one that leads most people to run around in circles for some time before realizing it is an engine bug :D

— Reply to this email directly, view it on GitHub https://github.com/godotengine/godot/issues/56458#issuecomment-1179246668, or unsubscribe https://github.com/notifications/unsubscribe-auth/ASLMWNAJ5I2SAE3I76FPNZLVTBVMHANCNFSM5LE4FVIQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>

Calinou commented 2 years ago

Testing the fix, works well for me.

Which fix are you referring to? Removing the error message isn't a proper fix, as it is here for a reason. Instead, the originating code should be fixed to never generate AABBs with a negative size.

fire commented 2 years ago

Here's a proposal

  1. Take the code from https://github.com/godotengine/godot/pull/60115 that applies the scale on the scene
  2. On any invalid aabb object with negative size
  3. Apply the scale to remove the aabb error.

Edited:

For example a box is Vector3(-1,1,-1) sized can have it's scaled applied and become 1,1,1.

Edited:

One source of artist workflow is when they have two walls, they duplicate the mesh and then mirror it with -1. Godot Engine doesn't handle this.

clayjohn commented 2 years ago

This is still reproducible in certain edge cases as https://github.com/godotengine/godot/pull/65035 highlights and fixes

mrfisty commented 1 year ago

I'm not sure which issue thread to comment on here as there seems to be a few (#65035 for instance), but I'm receiving the "AABB size is negative" error message as well.

I had a 3D character model from Reallusion's Character Creator that I brought into Mixamo for rigging and then Blender, and once imported into Godot as a .glb file, I receive that error message.

Now while I can still use the engine and see the character model, saving, loading, and running takes much longer to the point where it feels unusable.

I'm using the beta6 version of Godot 4. I can share the character model somewhere. If you'd like me to link it on GitHub I can or host it else where, just let me know.

Workflow: Character Creator -> Mixamo -> Blender -> Godot.

nathanfranke commented 1 year ago

Should "zero-weight bones" in the title be revised? Not sure how many different ways the specified warning can appear.

clayjohn commented 1 year ago

I think it's fine. Hopefully since this was fixed in #65035 we won't need to revisit this issue again

Closing as fixed by #65035

Calinou commented 1 year ago

Reopening per https://github.com/godotengine/godot/issues/69516#issuecomment-1345430146.