Skip to content
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

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

Closed
Tracked by #60036
lukas-toenne opened this issue Jan 3, 2022 · 20 comments · Fixed by #64416 or #70091
Closed
Tracked by #60036

Comments

@lukas-toenne
Copy link

lukas-toenne commented Jan 3, 2022


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


Godot version

master (937fb63)

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
Copy link
Author

Related issue: #56007

@akien-mga
Copy link
Member

CC @aaronfranke @lawnjelly

@aaronfranke
Copy link
Member

aaronfranke commented Jan 14, 2022

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
Copy link
Member

fire commented Jan 14, 2022

Is there a aabb has volume method?

@aaronfranke
Copy link
Member

aaronfranke commented Jan 14, 2022

@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
Copy link
Contributor

nonunknown commented Apr 6, 2022

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

do you guys have a workaround for now?

@kalysti
Copy link
Contributor

kalysti commented May 27, 2022

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

@njdf18
Copy link

njdf18 commented May 31, 2022

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

@Valeryn4
Copy link
Contributor

Valeryn4 commented Jun 5, 2022

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
Copy link
Member

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
Copy link
Contributor

kalysti commented Jul 2, 2022

Somebody have an temp fix for this issue?

@darrylryan
Copy link
Contributor

+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
Copy link
Contributor

kalysti commented Jul 8, 2022 via email

@Calinou
Copy link
Member

Calinou commented Jul 8, 2022

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
Copy link
Member

fire commented Jul 9, 2022

Here's a proposal

  1. Take the code from Importer option for permanent scaling of the scene #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
Copy link
Member

This is still reproducible in certain edge cases as #65035 highlights and fixes

@clayjohn clayjohn reopened this Oct 27, 2022
@mrfisty
Copy link

mrfisty commented Dec 2, 2022

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
Copy link
Contributor

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

@clayjohn
Copy link
Member

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
Copy link
Member

Calinou commented Dec 11, 2022

Reopening per #69516 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment