Imported models are flat. #82890

KnightNine · 2023-10-06T02:22:00Z

Godot version

v4.2.dev.custom_build [f71259b]

System information

Windows 10

Issue description

IT'S FLATT!?!?

My custom changes shouldn't affect this but can someone confirm that this happens in the raw build?

Steps to reproduce

Import a blender file containing a model into your project.
Open it.
Observe Flatness.

Minimal reproduction project

flatt test.zip

The text was updated successfully, but these errors were encountered:

aaronfranke · 2023-10-06T03:17:01Z

I don't observe this behavior with my test models. You must provide a minimal reproduction project.

KnightNine · 2023-10-06T05:29:33Z

@aaronfranke I did a fresh build of the master branch (v4.2.dev.custom_build [9e455f4]), created a new project, and the issue persists.
Did you open a .blend file and are using the master branch?
It would be worrying if I am the only person experiencing this.

Didn't think I needed to considering how visible and easily testable this issue is but I'll post just an empty project with my models imported.

aaronfranke · 2023-10-06T06:38:59Z

@KnightNine I tested the project you posted with Godot 9e455f4 and Blender 3.6.4 on macOS. It's not flat for me. Maybe it's a problem with the Blender version you are using?

LiveTrower · 2023-10-06T14:21:16Z

I can confirm that this happens and it's because of #81138.

If you enable the "Force Disable Compression" option, this gets resolved, but not its AABB.

clayjohn · 2023-10-06T19:46:45Z

The issue goes away with a reimport for any reason. I tested both just re-importing with the import button and deleting the .import folder.

So it seems the issue here is that the mesh upgrade path is not being triggered for this mesh. I will investigate further

clayjohn · 2023-10-06T20:55:49Z

@KnightNine Can you please elaborate on how you created the MRP? When I reimport the model the issue goes away. The issue also goes away if I reimport from an earlier dev release and run it in current master. This indicates that the issue is specific to how you imported the model in.

To be clear, I can only reproduce if I use the copy of the mesh in the .import folder. Anything imported in Master or older appears to work correctly. Accordingly, like AaronFranke, I am suspecting that the blender version be causing this issue

KnightNine · 2023-10-06T21:17:43Z

@clayjohn
wdym by "MRP"?
I am using the same blender version (3.6.4) but through Steam.
I'll try validating the integrity of my files or downloading blender outside of steam to see if I can resolve the issue.
Could blender addons affect the import process somehow?

clayjohn · 2023-10-06T21:36:01Z

wdym by "MRP"?

Minimal Reproduction Project. I.e. "flatt test.zip"

KnightNine · 2023-10-06T21:41:45Z

@clayjohn
I simply created a new project and imported my blender models with the default settings by dragging the .blend file into the editor.

Anyways I downloaded a fresh version of blender a and tried enabling "Force Disable Compression" before reimporting.... the issue still persists. 💀
The only other thing it could be is hardware related though it seems unlikely as previous versions of godot worked fine. 🤔

Idk where to go from here.

clayjohn · 2023-10-06T21:46:41Z

Its definitely not hardware related as I get the flat model when using your .import folder.

Did you update the version of blender in the Editor Settings as well (filesystem/import/blender/blender3_path)?

LiveTrower · 2023-10-06T22:35:30Z

@clayjohn I have a project that I use to test development versions, and since #81138 was merged, I've had various issues with the models, from them appearing flat to disappearing from the camera's view. It seems that the AABBs don't work even if I force the compression to be disabled.
It even happens with the built-in meshes.

2023-10-06.16-26-10.mp4

Note: I did delete the .godot folder.

KnightNine · 2023-10-06T22:38:38Z

@clayjohn

Did you update the version of blender in the Editor Settings as well (filesystem/import/blender/blender3_path)?

yes I did.

LiveTrower · 2023-10-07T19:14:46Z

@clayjohn Ok, I've noticed something very strange. When I use the artifact from the master branch, these problems occur, but when I use a version that I've compiled from my own computer, everything works somewhat better.

2023-10-07.13-05-24.mp4

2023-10-07.13-08-36.mp4

KnightNine · 2023-10-08T02:34:44Z

@LiveTrower

It seems that the AABBs don't work even if I force the compression to be disabled. It even happens with the built-in meshes.

Same here, the AABBs aren't correct even when compression is disabled.
And disabling compression doesn't show within the advanced import settings window and it only appears fixed (discounting the AABBs) if the blend file is opened as a scene, so I didn't realize it could be somewhat fixed by disabling compression until now.

I'm gonna try to undo #81138 so that I can take advantage of godot's recent fixes without this issue until this is fixed.

kevinkuo52 · 2023-10-08T20:14:48Z

also faced issue with flat imported models with my project separate from repro posted, with latest master commit 6916349
Just sharing my observation if it helps.

I exported a simple model from blender (ver 3.3.1) to .obj, glb, and .dae to test (did not try .blend).

I see this issue for .obj and .glb file
.dae works fine (edit: works fine for mesh vertices but AABB seems to be still messed up)
when compiled to windows x86_32, i dont see this issue. I see this issue when compiled to x86_64

setting force_disable_mesh_compression to true in ResourceImporterOBJ.xml fixes the obj import for me

kevinkuo52 · 2023-10-09T00:03:49Z

sharing some more notes

debug print in add_surface_from_arrays after _surface_set_data

format: 34896613399
aabb: [P: (-1, -1, -1), S: (2, 2.00001, 0.00001)] // value with issue: 0.00001
array size: 288
vertex count: 24
index size: 72
index count: 36
primitive: 3
New Scene Root
Create Node

but after adding print line in _surface_set_data to print AABB values, issue is gone............
looks like AABB somehow get fixed from 0.00001 to 2 in the 2nd aabb.expand_to

KnightNine · 2023-10-09T00:25:22Z

@kevinkuo52

.dae works fine

the AABBs are still broken with .dae.

setting force_disable_mesh_compression to true in ResourceImporterOBJ.xml fixes the obj import for me

are the AABBs fixed with that method?

but after adding print line in _surface_set_data to print aabb values, issue is gone............
looks like aabb somehow get fixed from 0.00001 to 2 in the 2nd aabb.expand_to

Strange, though I may be completely wrong, seems like a threading issue from my experience. The print causes a delay which allows something to update that would be unable to without it. Does the engine use threads for these base functions?

kevinkuo52 · 2023-10-09T00:42:41Z

sorry i wasn't clear. No, AABB wasn't fix for me as well with force_disable_mesh_compression = true, just mesh vertices. Also it seems flaky for me, just tried again with turning off compression, but it no longer fixes the mesh vertices either

KnightNine · 2023-10-09T00:55:35Z

I am hoping for a workaround, as there seems to be no quick way of removing the PR merge from my fork of Godot with my limited knowledge of git if there even is a way. 😭

Edit:
I found a way in case anyone was wondering:
After cloning the main branch I used "git revert 51ed3aef63c0fdfc7666c004cc6d94dd15322d81" in it to yeet the commit. And then just added my custom files to it after the fact.

clayjohn · 2023-10-11T18:36:47Z

This might have been fixed by #83056

After careful testing again, I still can't reproduce the issue properly. Loading the MRP I can observe the flat meshes, but inspecting the meshes, it is clear that they have been saved in a way that is broken. Accordingly, the issue likely lies in the importer or in the way the meshes were formed/saved originally. For me, the meshes are always fixed on reimport.

A few points from above are especially interesting:

@LiveTrower Notices a difference between compiling locally and using an artifact from the master branch
@kevinkuo52 Notices some improvement when adding print statements (indicating that the issue could be threading/synchronization related)
kevinkuo52 Also noticed that the issue goes away in 32 bit builds
kevinkuo52 only sees the issue for .obj and .gltf, not .dae

Investigating a little further led me to #83169 which may help this issue. Since I can't reproduce on my hardware, I am just guessing. But implicitly converting the 64 bit version to 32 bits and back could explain this issue. It also makes sense with the observed behaviours listed above. As the exact behaviour will be undefined in this case.

Could someone who is building locally test on master with #83169 applied and let me know if it fixes the issue?

clayjohn · 2023-10-11T22:02:24Z

Re-opening until we have confirmation that this is fixed. I'm still not sure

LiveTrower · 2023-10-11T23:49:22Z

@clayjohn Problems persist

clayjohn · 2023-10-11T23:53:13Z

@LiveTrower Problems persist after pulling from master and building fresh in the last hour and reimporting your models?

Can you also let me know what system and build settings you are using?

LiveTrower · 2023-10-12T00:03:04Z

@clayjohn I tested the artifact from the master branch v4.2.beta.custom_build [b137180] and the problem persists, but it seems that the artifact from PR #83169 wasn't compiled.
The configuration I used to compile on my PC is x64 Native Tools Command Prompt for VS 2019 with the command scons platform=windows.

kevinkuo52 · 2023-10-12T02:59:15Z

@clayjohn
i tested with latest master #82431 and still see the issue, still flat for me

I see this issue for .obj and .glb file

.dae works fine

when compiled to windows x86_32, i dont see this issue. I see this issue when compiled to x86_64

@clayjohn just to clarify on this since I didnt pay attention to AABB on this first post, only on mesh vertices:

AABB for .dae was still messed up in both 64 and 32 bit builds (tho not flat just very large) or is this expected for .dae? sorry not too familiar with this file format
obj and .glb works fine in 32 bit build for both AABB and mesh vertices

KnightNine · 2023-10-12T05:09:33Z

@kevinkuo52
That is not expected for .dae . Without #81138 .dae's AABBs fit properly to the mesh.

clayjohn · 2023-10-12T17:49:48Z

I've now tested on a Windows 10 system to see if its a Windows related issue and I still can't reproduce. I have also tested debug and non-debug builds to see if that is the issue and can't reproduce (both on Linux and on Windows).

I'm going to keep investigating, but it is challenging to confirm anything without being able to reproduce the issue.

Because the AABBs are broken both when using compressed and uncompressed, it seems there is something broken with calculating, storing, or retrieving the mesh AABBs. I will investigate that further and see if I can identify a situation where AABBs may change depending on build, OS, or hardware.

clayjohn · 2023-10-12T20:09:53Z

Okay, I've made another PR that might help with this.

I can't find anywhere where the AABB generation has changed. So my current theory is that at certain points of the pipeline the mesh is treated as compressed, while at others it isn't, leading to a disconnect between the size of the AABB and the underlying surface data.

Please test it and let me know if it helps

LiveTrower · 2023-10-12T21:23:11Z

@clayjohn I tried PR #83211 but everything remains the same.
What's curious is that I tested the beta 1 version from here https://github.com/godotengine/godot-builds/releases/tag/4.2-beta1 and everything works fine. I tested both the x64 and x32 versions.

clayjohn · 2023-10-12T21:26:46Z

@clayjohn I tried PR #83211 but everything remains the same. What's curious is that I tested the beta 1 version from here https://github.com/godotengine/godot-builds/releases/tag/4.2-beta1 and everything works fine. I tested both the x64 and x32 versions.

Is there any chance that you are accidentally testing an old build? Perhaps a build with a slightly different name/config?

LiveTrower · 2023-10-12T22:02:48Z

I don't think so.
v4.2.beta.custom_build [f57748493]

2023-10-12.15-44-58.mp4

kevinkuo52 · 2023-10-13T00:31:59Z

@clayjohn also tried PR #83211, same behavior as before as well.

pulled ur change and made sure i used the new built exe

I will try to investigate more as well since I have repro, will update if I find anything useful.

kevinkuo52 · 2023-10-13T02:12:55Z

some more notes:

found dev build for 64 bit dont have issue for me, scons platform=windows dev_build=yes (i only see issue when scons platform=windows for 64) @LiveTrower is this true for u as well?
for me, .dae file import have large AABB for all scenario even on stable release... (maybe just my issue? not sure)

I can't seem to have break points hit if i dont build it with dev_build=yes, even with -e arg pass in to launch straight to editor. Will see if i can fix that or ask in new contributor channel to unblock.
Edit: just needed debug_symbols=yes

LiveTrower · 2023-10-13T13:52:16Z

@kevinkuo52 I have no issues with command scons platform=windows for 64

kevinkuo52 · 2023-10-16T00:00:42Z

since scons platform=windows for 64 works for LiveTower, maybe root cause is different.
but at least on my machine, here's some finding of this issue when i do scons platform=windows for 64:

after debugging for some time, i believe the issue is related to how the aabb variable here got optimized out weirdly by the compiler (maybe because of some code logic pattern on aabb)
https://github.com/godotengine/godot/blob/a574c0296b38d5f786f249b12e6251e562c528cc/servers/rendering_server.cpp#L445C6-L445C16
because when compiled with optimize=debug or when printing aabb once, aabb became not optimized out and i dont see the issue with flat model.

compile with scons platform=windows debug_symbols=yes
See aabb3 is optimized out, have issue with flat model, with very small aabb.size.z value in the end: [P: (-1, -1, -1), S: (2, 2.00001, 0.00001)]
(i renamed to aabb3 because aabb from the prev if block with type Rect2 show up even though that code path was never reached, maybe due to debug symbo is messy cause optimize=debug not set)

compile with optimize=debug instead or when printing aabb once:
scons platform=windows debug_symbols=yes optimize=debug
aabb is not optimized out and flat model issue does not appear.

Assembly:

aabb optmized out:

					// Setting vertices means regenerating the AABB.
					AABB aabb3;
00007FF72F2B741C  xor         eax,eax  
00007FF72F2B741E  xorps       xmm6,xmm6  
00007FF72F2B7421  mov         qword ptr [rbp-48h],rax  
00007FF72F2B7425  movups      xmmword ptr [rbp-58h],xmm6  
					//print_line("aab3: " + aabb3);
					if (p_format & ARRAY_FLAG_COMPRESS_ATTRIBUTES) {
00007FF72F2B7429  cmp         qword ptr [rbp-80h],rax  
00007FF72F2B742D  je          RenderingServer::_surface_set_data+0E26h (07FF72F2B7D26h)  
						// First we need to generate the AABB for the entire surface.
						for (int i = 0; i < p_vertex_array_len; i++) {
00007FF72F2B7433  movss       xmm12,dword ptr [rbp-44h]  
00007FF72F2B7439  mov         rdi,r11  
00007FF72F2B743C  movss       xmm13,dword ptr [rbp-48h]  
00007FF72F2B7442  movsxd      r10,r15d  
00007FF72F2B7445  test        r15d,r15d  
00007FF72F2B7448  jle         RenderingServer::_surface_set_data+723h (07FF72F2B7623h)  
00007FF72F2B744E  movss       xmm10,dword ptr [rbp-4Ch]  
00007FF72F2B7454  xor         r13d,r13d  
00007FF72F2B7457  mov         esi,dword ptr [rbp-50h]  
00007FF72F2B745A  movsd       xmm8,mmword ptr [rbp-58h]  
00007FF72F2B7460  mov         r12,qword ptr [rbp+0A0h]  
							//print_line("i: " + i);
							//print_line("src[i]: " + src[i]);
							if (i == 0) {
00007FF72F2B7467  lea         rbx,[rdi+rdi*2]  
00007FF72F2B746B  test        rdi,rdi  
00007FF72F2B746E  jne         RenderingServer::_surface_set_data+5DCh (07FF72F2B74DCh)  
								aabb3 = AABB(src[i], SMALL_VEC3);
00007FF72F2B7470  movsd       xmm0,mmword ptr [r12+1A8h]  
00007FF72F2B747A  mov         eax,dword ptr [r14+rbx*4+8]  
00007FF72F2B747F  movsd       xmm1,mmword ptr [r14+rbx*4]  
00007FF72F2B7485  movsd       mmword ptr [rbp+58h],xmm0  
00007FF72F2B748A  mov         dword ptr [rbp+54h],eax  
00007FF72F2B748D  movups      xmm6,xmmword ptr [rbp+4Ch]  
00007FF72F2B7491  mov         eax,dword ptr [r12+1B0h]  
00007FF72F2B7499  mov         dword ptr [rbp+60h],eax  
00007FF72F2B749C  movsd       xmm6,xmm1  
00007FF72F2B74A0  movsd       xmm1,mmword ptr [rbp+5Ch]  
								//print_line("not (p_format & RS::ARRAY_FLAG_USE_2D_VERTICES) if (p_format & ARRAY_FLAG_COMPRESS_ATTRIBUTES) create - aabb: " + aabb);
							} else {
00007FF72F2B74A5  movaps      xmm0,xmm1  
00007FF72F2B74A8  movsd       mmword ptr [rbp-48h],xmm1  
00007FF72F2B74AD  shufps      xmm0,xmm0,55h  
00007FF72F2B74B1  movaps      xmm10,xmm6  
00007FF72F2B74B5  movaps      xmm12,xmm0  
00007FF72F2B74B9  shufps      xmm10,xmm6,0FFh  
00007FF72F2B74BE  movdqa      xmm0,xmm6  
00007FF72F2B74C2  movaps      xmm13,xmm1  
00007FF72F2B74C6  psrldq      xmm0,8  
00007FF72F2B74CB  movaps      xmm8,xmm6  
00007FF72F2B74CF  movd        esi,xmm0  
00007FF72F2B74D3  movups      xmmword ptr [rbp+4Ch],xmm6  
00007FF72F2B74D7  jmp         RenderingServer::_surface_set_data+6F8h (07FF72F2B75F8h)  
								aabb3.expand_to(src[i]);
00007FF72F2B74DC  comiss      xmm7,xmm10  
00007FF72F2B74E0  ja          RenderingServer::_surface_set_data+5EEh (07FF72F2B74EEh)  
00007FF72F2B74E2  comiss      xmm7,xmm13  
00007FF72F2B74E6  ja          RenderingServer::_surface_set_data+5EEh (07FF72F2B74EEh)  
00007FF72F2B74E8  comiss      xmm7,xmm12  
00007FF72F2B74EC  jbe         RenderingServer::_surface_set_data+61Bh (07FF72F2B751Bh)  
00007FF72F2B74EE  mov         dword ptr [rsp+28h],r13d  
00007FF72F2B74F3  lea         r9,[string "AABB size is negative, this is @"... (07FF730567C00h)]  
00007FF72F2B74FA  mov         r8d,153h  
00007FF72F2B7500  mov         byte ptr [rsp+20h],r13b  
00007FF72F2B7505  lea         rdx,[string "C:\\src\\GodotProjects\\godot\\core@"... (07FF730567C68h)]  
00007FF72F2B750C  lea         rcx,[string "AABB::expand_to" (07FF730567C98h)]  
00007FF72F2B7513  call        _err_print_error (07FF72FB9DAC0h)  
00007FF72F2B7518  movsxd      r10,r15d  
00007FF72F2B751B  movss       xmm1,dword ptr [rbp-58h]  
00007FF72F2B7520  movaps      xmm0,xmm8  
00007FF72F2B7524  movss       xmm2,dword ptr [r14+rbx*4]  
00007FF72F2B752A  addss       xmm10,xmm1  
00007FF72F2B752F  comiss      xmm1,xmm2  
00007FF72F2B7532  addss       xmm13,dword ptr [rbp-54h]  
00007FF72F2B7538  movaps      xmm4,xmm0  
00007FF72F2B753B  movsd       mmword ptr [rbp+80h],xmm0  
00007FF72F2B7543  movd        xmm0,esi  
00007FF72F2B7547  addss       xmm12,xmm0  
00007FF72F2B754C  jbe         RenderingServer::_surface_set_data+666h (07FF72F2B7566h)  
00007FF72F2B754E  movaps      xmm0,xmm8  
00007FF72F2B7552  movaps      xmm6,xmm2  
00007FF72F2B7555  movss       xmm0,xmm2  
00007FF72F2B7559  movsd       mmword ptr [rbp+80h],xmm0  
00007FF72F2B7561  movaps      xmm4,xmm0  
00007FF72F2B7564  jmp         RenderingServer::_surface_set_data+66Eh (07FF72F2B756Eh)  
00007FF72F2B7566  movss       xmm6,dword ptr [rbp+80h]  
00007FF72F2B756E  movss       xmm0,dword ptr [r14+rbx*4+4]  
00007FF72F2B7575  movss       xmm5,dword ptr [rbp+84h]  
00007FF72F2B757D  comiss      xmm5,xmm0  
00007FF72F2B7580  jbe         RenderingServer::_surface_set_data+68Bh (07FF72F2B758Bh)  
00007FF72F2B7582  movaps      xmm4,xmm6  
00007FF72F2B7585  movaps      xmm5,xmm0  
00007FF72F2B7588  unpcklps    xmm4,xmm0  
00007FF72F2B758B  movss       xmm1,dword ptr [r14+rbx*4+8]  
00007FF72F2B7592  movd        xmm3,esi  
00007FF72F2B7596  comiss      xmm3,xmm1  
00007FF72F2B7599  jbe         RenderingServer::_surface_set_data+6A2h (07FF72F2B75A2h)  
00007FF72F2B759B  movaps      xmm3,xmm1  
00007FF72F2B759E  movd        esi,xmm1  
00007FF72F2B75A2  comiss      xmm2,xmm10  
00007FF72F2B75A6  jbe         RenderingServer::_surface_set_data+6ACh (07FF72F2B75ACh)  
00007FF72F2B75A8  movaps      xmm10,xmm2  
00007FF72F2B75AC  comiss      xmm0,xmm13  
00007FF72F2B75B0  jbe         RenderingServer::_surface_set_data+6B6h (07FF72F2B75B6h)  
00007FF72F2B75B2  movaps      xmm13,xmm0  
00007FF72F2B75B6  comiss      xmm1,xmm12  
00007FF72F2B75BA  jbe         RenderingServer::_surface_set_data+6C0h (07FF72F2B75C0h)  
00007FF72F2B75BC  movaps      xmm12,xmm1  
00007FF72F2B75C0  subss       xmm10,xmm6  
00007FF72F2B75C5  mov         dword ptr [rbp-50h],esi  
00007FF72F2B75C8  movups      xmm6,xmmword ptr [rbp-58h]  
00007FF72F2B75CC  subss       xmm13,xmm5  
00007FF72F2B75D1  movaps      xmm8,xmm4  
00007FF72F2B75D5  movsd       xmm6,xmm4  
00007FF72F2B75D9  subss       xmm12,xmm3  
00007FF72F2B75DE  shufps      xmm6,xmm6,93h  
00007FF72F2B75E2  movss       xmm6,xmm10  
00007FF72F2B75E7  shufps      xmm6,xmm6,39h  
00007FF72F2B75EB  movaps      xmm0,xmm13  
00007FF72F2B75EF  unpcklps    xmm0,xmm12  
00007FF72F2B75F3  movss       dword ptr [rbp-48h],xmm0  
						// First we need to generate the AABB for the entire surface.
						for (int i = 0; i < p_vertex_array_len; i++) {
00007FF72F2B75F8  inc         rdi  
00007FF72F2B75FB  movups      xmmword ptr [rbp-58h],xmm6  
00007FF72F2B75FF  cmp         rdi,r10  
00007FF72F2B7602  jl          RenderingServer::_surface_set_data+567h (07FF72F2B7467h)  
00007FF72F2B7608  mov         r13,qword ptr [rsp+78h]  
00007FF72F2B760D  xor         r11d,r11d  
00007FF72F2B7610  mov         r12,qword ptr [rbp+30h]  
00007FF72F2B7614  movss       xmm8,dword ptr [__real@3f000000 (07FF7304C9BD8h)]  
00007FF72F2B761D  mov         esi,dword ptr [ai]  
00007FF72F2B7621  jmp         RenderingServer::_surface_set_data+73Bh (07FF72F2B763Bh)  
00007FF72F2B7623  movaps      xmm0,xmm13  
00007FF72F2B7627  xorps       xmm1,xmm1  
00007FF72F2B762A  unpcklps    xmm0,xmm12  
00007FF72F2B762E  shufps      xmm1,xmm6,0FFh  
00007FF72F2B7632  movss       dword ptr [rbp-48h],xmm0  
00007FF72F2B7637  movaps      xmm10,xmm1  
								//print_line("not (p_format & RS::ARRAY_FLAG_USE_2D_VERTICES) if (p_format & ARRAY_FLAG_COMPRESS_ATTRIBUTES) expand_to - aabb: " + aabb);
							}
						}

aabb not optmized out
Edit: by using print aabb once instead of compiling with optimize=debug to better compare optimization difference):

					// Setting vertices means regenerating the AABB.
					AABB aabb3;
00007FF6F8D9743A  movups      xmmword ptr [rbp-78h],xmm0  
00007FF6F8D9743E  mov         qword ptr [rbp-68h],rax  
					print_line("aab3: " + aabb3);
00007FF6F8D97442  call        AABB::operator String (07FF6F9967740h)  
00007FF6F8D97447  lea         r8,[rbp+0F8h]  
00007FF6F8D9744E  lea         rdx,[string "aab3: " (07FF6FB29A7D4h)]  
00007FF6F8D97455  lea         rcx,[rbp+200h]  
00007FF6F8D9745C  call        operator+ (07FF6F9697230h)  
00007FF6F8D97461  mov         rdx,rax  
00007FF6F8D97464  lea         rcx,[rbp+340h]  
00007FF6F8D9746B  call        Variant::Variant (07FF6F96CA970h)  
00007FF6F8D97470  mov         rdx,rax  
00007FF6F8D97473  lea         rcx,[rbp+358h]  
00007FF6F8D9747A  mov         rbx,rax  
00007FF6F8D9747D  call        Variant::Variant (07FF6F96C9870h)  
00007FF6F8D97482  mov         rdx,rax  
00007FF6F8D97485  lea         rcx,[rbp+328h]  
00007FF6F8D9748C  call        stringify_variants (07FF6F96F3460h)  
00007FF6F8D97491  mov         rcx,rax  
00007FF6F8D97494  call        __print_line (07FF6F96F1E20h)  
00007FF6F8D97499  movsxd      rcx,dword ptr [rbx]  
00007FF6F8D9749C  lea         rax,[_wpgmptr (07FF6F6020000h)]  
00007FF6F8D974A3  cmp         byte ptr [rcx+rax+3F65C28h],0  
00007FF6F8D974AB  je          RenderingServer::_surface_set_data+5B5h (07FF6F8D974B5h)  
00007FF6F8D974AD  mov         rcx,rbx  
00007FF6F8D974B0  call        Variant::_clear_internal (07FF6F96CDC70h)  
00007FF6F8D974B5  mov         dword ptr [rbx],edi  
00007FF6F8D974B7  mov         rcx,qword ptr [rbp+200h]  
00007FF6F8D974BE  test        rcx,rcx  
00007FF6F8D974C1  je          RenderingServer::_surface_set_data+5D8h (07FF6F8D974D8h)  
00007FF6F8D974C3  mov         eax,r14d  
00007FF6F8D974C6  lock xadd   dword ptr [rcx-8],eax  
00007FF6F8D974CB  cmp         eax,1  
00007FF6F8D974CE  jne         RenderingServer::_surface_set_data+5D8h (07FF6F8D974D8h)  
00007FF6F8D974D0  movzx       edx,al  
00007FF6F8D974D3  call        Memory::free_static (07FF6F967E240h)  
00007FF6F8D974D8  mov         rcx,qword ptr [rbp+0F8h]  
00007FF6F8D974DF  test        rcx,rcx  
00007FF6F8D974E2  je          RenderingServer::_surface_set_data+5F9h (07FF6F8D974F9h)  
00007FF6F8D974E4  mov         eax,r14d  
00007FF6F8D974E7  lock xadd   dword ptr [rcx-8],eax  
00007FF6F8D974EC  cmp         eax,1  
00007FF6F8D974EF  jne         RenderingServer::_surface_set_data+5F9h (07FF6F8D974F9h)  
00007FF6F8D974F1  movzx       edx,al  
00007FF6F8D974F4  call        Memory::free_static (07FF6F967E240h)  
					if (p_format & ARRAY_FLAG_COMPRESS_ATTRIBUTES) {
00007FF6F8D974F9  cmp         qword ptr [rbp-80h],0  
00007FF6F8D974FE  je          RenderingServer::_surface_set_data+0E5Ch (07FF6F8D97D5Ch)  
						// First we need to generate the AABB for the entire surface.
						for (int i = 0; i < p_vertex_array_len; i++) {
00007FF6F8D97504  movss       xmm3,dword ptr [rbp-6Ch]  
00007FF6F8D97509  xor         r11d,r11d  
00007FF6F8D9750C  movsxd      r10,r15d  
00007FF6F8D9750F  mov         edi,r11d  
00007FF6F8D97512  test        r15d,r15d  
00007FF6F8D97515  jle         RenderingServer::_surface_set_data+7C2h (07FF6F8D976C2h)  
00007FF6F8D9751B  mov         eax,dword ptr [rbp-70h]  
00007FF6F8D9751E  xor         r13d,r13d  
00007FF6F8D97521  movsd       xmm0,mmword ptr [rbp-78h]  
00007FF6F8D97526  mov         r12,qword ptr [rbp+0A8h]  
00007FF6F8D9752D  nop         dword ptr [rax]  
							//print_line("i: " + i);
							//print_line("src[i]: " + src[i]);
							if (i == 0) {
00007FF6F8D97530  lea         rbx,[rdi+rdi*2]  
00007FF6F8D97534  test        rdi,rdi  
00007FF6F8D97537  jne         RenderingServer::_surface_set_data+695h (07FF6F8D97595h)  
								aabb3 = AABB(src[i], SMALL_VEC3);
00007FF6F8D97539  mov         eax,dword ptr [rsi+rbx*4+8]  
00007FF6F8D9753D  movsd       xmm1,mmword ptr [rsi+rbx*4]  
00007FF6F8D97542  movsd       xmm0,mmword ptr [r12+1A8h]  
00007FF6F8D9754C  movsd       mmword ptr [rbp+5Ch],xmm0  
00007FF6F8D97551  mov         dword ptr [rbp+58h],eax  
00007FF6F8D97554  movups      xmm2,xmmword ptr [rbp+50h]  
00007FF6F8D97558  mov         eax,dword ptr [r12+1B0h]  
00007FF6F8D97560  mov         dword ptr [rbp+64h],eax  
00007FF6F8D97563  movsd       xmm0,mmword ptr [rbp+60h]  
00007FF6F8D97568  movsd       xmm2,xmm1  
00007FF6F8D9756C  movsd       mmword ptr [rbp-68h],xmm0  
								//print_line("not (p_format & RS::ARRAY_FLAG_USE_2D_VERTICES) if (p_format & ARRAY_FLAG_COMPRESS_ATTRIBUTES) create - aabb: " + aabb);
							} else {
00007FF6F8D97571  movaps      xmm3,xmm2  
00007FF6F8D97574  movdqa      xmm0,xmm2  
00007FF6F8D97578  shufps      xmm3,xmm2,0FFh  
00007FF6F8D9757C  psrldq      xmm0,8  
00007FF6F8D97581  movd        eax,xmm0  
00007FF6F8D97585  movaps      xmm0,xmm2  
00007FF6F8D97588  movups      xmmword ptr [rbp+50h],xmm2  
00007FF6F8D9758C  movups      xmmword ptr [rbp-78h],xmm2  
00007FF6F8D97590  jmp         RenderingServer::_surface_set_data+7AAh (07FF6F8D976AAh)  
								aabb3.expand_to(src[i]);
00007FF6F8D97595  comiss      xmm6,xmm3  
00007FF6F8D97598  ja          RenderingServer::_surface_set_data+6A6h (07FF6F8D975A6h)  
00007FF6F8D9759A  comiss      xmm6,dword ptr [rbp-68h]  
00007FF6F8D9759E  ja          RenderingServer::_surface_set_data+6A6h (07FF6F8D975A6h)  
00007FF6F8D975A0  comiss      xmm6,dword ptr [rbp-64h]  
00007FF6F8D975A4  jbe         RenderingServer::_surface_set_data+6E0h (07FF6F8D975E0h)  
00007FF6F8D975A6  mov         dword ptr [rsp+28h],r13d  
00007FF6F8D975AB  lea         r9,[string "AABB size is negative, this is @"... (07FF6FA047C00h)]  
00007FF6F8D975B2  mov         r8d,153h  
00007FF6F8D975B8  mov         byte ptr [rsp+20h],r13b  
00007FF6F8D975BD  lea         rdx,[string "C:\\src\\GodotProjects\\godot\\core@"... (07FF6FA047C68h)]  
00007FF6F8D975C4  lea         rcx,[string "AABB::expand_to" (07FF6FA047C98h)]  
00007FF6F8D975CB  call        _err_print_error (07FF6F967DAF0h)  
00007FF6F8D975D0  movss       xmm3,dword ptr [rbp-6Ch]  
00007FF6F8D975D5  mov         eax,dword ptr [rbp-70h]  
00007FF6F8D975D8  movsd       xmm0,mmword ptr [rbp-78h]  
00007FF6F8D975DD  movsxd      r10,r15d  
00007FF6F8D975E0  movss       xmm1,dword ptr [rbp-78h]  
00007FF6F8D975E5  movaps      xmm8,xmm0  
00007FF6F8D975E9  movss       xmm5,dword ptr [rbp-68h]  
00007FF6F8D975EE  addss       xmm3,xmm1  
00007FF6F8D975F2  movss       xmm2,dword ptr [rbp-64h]  
00007FF6F8D975F7  addss       xmm5,dword ptr [rbp-74h]  
00007FF6F8D975FC  movsd       mmword ptr [rbp+70h],xmm0  
00007FF6F8D97601  movd        xmm0,eax  
00007FF6F8D97605  movss       dword ptr [rbp-70h],xmm0  
00007FF6F8D9760A  addss       xmm2,xmm0  
00007FF6F8D9760E  movss       xmm4,dword ptr [rsi+rbx*4]  
00007FF6F8D97613  comiss      xmm1,xmm4  
00007FF6F8D97616  jbe         RenderingServer::_surface_set_data+72Fh (07FF6F8D9762Fh)  
00007FF6F8D97618  movaps      xmm0,xmm8  
00007FF6F8D9761C  movaps      xmm10,xmm4  
00007FF6F8D97620  movss       xmm0,xmm4  
00007FF6F8D97624  movsd       mmword ptr [rbp+70h],xmm0  
00007FF6F8D97629  movaps      xmm8,xmm0  
00007FF6F8D9762D  jmp         RenderingServer::_surface_set_data+735h (07FF6F8D97635h)  
00007FF6F8D9762F  movss       xmm10,dword ptr [rbp+70h]  
00007FF6F8D97635  movss       xmm0,dword ptr [rsi+rbx*4+4]  
00007FF6F8D9763B  movss       xmm9,dword ptr [rbp+74h]  
00007FF6F8D97641  comiss      xmm9,xmm0  
00007FF6F8D97645  jbe         RenderingServer::_surface_set_data+753h (07FF6F8D97653h)  
00007FF6F8D97647  movaps      xmm8,xmm10  
00007FF6F8D9764B  movaps      xmm9,xmm0  
00007FF6F8D9764F  unpcklps    xmm8,xmm0  
00007FF6F8D97653  movss       xmm1,dword ptr [rsi+rbx*4+8]  
00007FF6F8D97659  movd        xmm7,eax  
00007FF6F8D9765D  comiss      xmm7,xmm1  
00007FF6F8D97660  jbe         RenderingServer::_surface_set_data+769h (07FF6F8D97669h)  
00007FF6F8D97662  movaps      xmm7,xmm1  
00007FF6F8D97665  movd        eax,xmm1  
00007FF6F8D97669  comiss      xmm4,xmm3  
00007FF6F8D9766C  jbe         RenderingServer::_surface_set_data+771h (07FF6F8D97671h)  
00007FF6F8D9766E  movaps      xmm3,xmm4  
00007FF6F8D97671  comiss      xmm0,xmm5  
00007FF6F8D97674  jbe         RenderingServer::_surface_set_data+779h (07FF6F8D97679h)  
00007FF6F8D97676  movaps      xmm5,xmm0  
00007FF6F8D97679  comiss      xmm1,xmm2  
00007FF6F8D9767C  jbe         RenderingServer::_surface_set_data+781h (07FF6F8D97681h)  
00007FF6F8D9767E  movaps      xmm2,xmm1  
00007FF6F8D97681  subss       xmm3,xmm10  
00007FF6F8D97686  mov         dword ptr [rbp-70h],eax  
00007FF6F8D97689  subss       xmm5,xmm9  
00007FF6F8D9768E  movaps      xmm0,xmm8  
00007FF6F8D97692  subss       xmm2,xmm7  
00007FF6F8D97696  movsd       mmword ptr [rbp-78h],xmm0  
00007FF6F8D9769B  movss       dword ptr [rbp-6Ch],xmm3  
00007FF6F8D976A0  movss       dword ptr [rbp-68h],xmm5  
00007FF6F8D976A5  movss       dword ptr [rbp-64h],xmm2  
						// First we need to generate the AABB for the entire surface.
						for (int i = 0; i < p_vertex_array_len; i++) {
00007FF6F8D976AA  inc         rdi  
00007FF6F8D976AD  cmp         rdi,r10  
00007FF6F8D976B0  jl          RenderingServer::_surface_set_data+630h (07FF6F8D97530h)  
00007FF6F8D976B6  mov         r13,qword ptr [rsp+70h]  
00007FF6F8D976BB  xor         r11d,r11d  
00007FF6F8D976BE  mov         r12,qword ptr [rbp+48h]  
								//print_line("not (p_format & RS::ARRAY_FLAG_USE_2D_VERTICES) if (p_format & ARRAY_FLAG_COMPRESS_ATTRIBUTES) expand_to - aabb: " + aabb);
							}
						}

I will continue this direction
But let me know any thoughts, idea, things I should check for, or if im not going in the right direction etc.
thanks

clayjohn · 2023-10-17T07:30:47Z

@kevinkuo52 That seems very likely. It must be a compiler-specific issue which is why AaronFranke and myself can't reproduce.

Looking at the code with your post in mind. I am struck that there is a situation where the AABB may not get set at all: namely, when the mesh does not use normals or tangents. In that case, we fill the vertex array with positions only and then move on and r_aabb ends up uninitialized.

godot/servers/rendering_server.cpp

Lines 459 to 473 in fd33c7b

 if (!using_normals_tangents) { 

 // Early out if we are only setting vertex positions. 

 for (int i = 0; i < p_vertex_array_len; i++) { 

 Vector3 pos = (src[i] - aabb.position) / aabb.size; 

 uint16_t vector[4] = { 

 (uint16_t)CLAMP(pos.x * 65535, 0, 65535), 

 (uint16_t)CLAMP(pos.y * 65535, 0, 65535), 

 (uint16_t)CLAMP(pos.z * 65535, 0, 65535), 

 (uint16_t)0 

 }; 

 memcpy(&vw[p_offsets[ai] + i * p_vertex_stride], vector, sizeof(uint16_t) * 4); 

 } 

 continue; 

 }

Instead of operating on aabb and then assigning to r_aabb. Can you try modifying the code to operate on r_aabb directly? I.e.:

if (i == 0) {
	r_aabb = AABB(src[i], SMALL_VEC3);
} else {
	r_aabb.expand_to(src[i]);
}

clayjohn · 2023-10-19T22:32:43Z

Looks like I am able to reproduce the issue now, but only on Windows and only when using compiled builds from our Github Actions (I tested https://github.com/godotengine/godot/actions/runs/6576288429/job/17865371862 for reference). Building locally with the exact same options does not reproduce the issue.

My exact compiler version is MSVC 14.36.32532. For those of you who can reproduce the issue when building locally, can you also post what compiler version you are using?

KnightNine · 2023-10-21T21:35:19Z

@clayjohn
I am using MSVC 14.34.31933 (from what I can find).
And just to note: The imported models are still flat using the most recent version of the master branch.

kevinkuo52 · 2023-10-22T00:23:01Z

Looking at the code with your post in mind. I am struck that there is a situation where the AABB may not get set at all: namely, when the mesh does not use normals or tangents. In that case, we fill the vertex array with positions only and then move on and r_aabb ends up uninitialized.

Just to clarify, using_normals_tangents is true when i step thru it, so it does not skip and will set r_aabb in the end with an incorrect z value for size like this when aabb is optimized out by compiler:
[P: (-1, -1, -1), S: (2, 2.00001, 0.00001)]

Instead of operating on aabb and then assigning to r_aabb. Can you try modifying the code to operate on r_aabb directly? I.e.:
if (i == 0) {
	r_aabb = AABB(src[i], SMALL_VEC3);
} else {
	r_aabb.expand_to(src[i]);
}

Tried it, it does seem to mitigate the issue. Tho not sure if its an actual fix, suspect there's some underlying issue that cause aabb to have weird behavior when optimized out?
(i should have time this weekend, will see if i can figure something out... no promise tho..)

Side note:
when step thru code i notice for 64 build, it will hit this condition here where tangent_type == Variant::PACKED_FLOAT32_ARRAY
Want to mentioned it just in case it seems off? or is it expected
https://github.com/godotengine/godot/blob/f71f4b80e32f6e98a4cd3cb1c06071223297e8fc/servers/rendering_server.cpp#L485C7-L486C66

I see it's set here
https://github.com/godotengine/godot/blob/f71f4b80e32f6e98a4cd3cb1c06071223297e8fc/scene/resources/surface_tool.cpp#L474C4-L475C25
always seems to be type Vector<float> so not sure how it will ever be PACKED_FLOAT64_ARRAY, maybe some other flow.

clayjohn · 2023-10-23T12:05:53Z

Side note: when step thru code i notice for 64 build, it will hit this condition here where tangent_type == Variant::PACKED_FLOAT32_ARRAY Want to mentioned it just in case it seems off? or is it expected https://github.com/godotengine/godot/blob/f71f4b80e32f6e98a4cd3cb1c06071223297e8fc/servers/rendering_server.cpp#L485C7-L486C66

I see it's set here https://github.com/godotengine/godot/blob/f71f4b80e32f6e98a4cd3cb1c06071223297e8fc/scene/resources/surface_tool.cpp#L474C4-L475C25 always seems to be type Vector<float> so not sure how it will ever be PACKED_FLOAT64_ARRAY, maybe some other flow.

Arrays can also be created by users directly with the ArrayMesh. In which case both floats and doubles are accepted. SurfaceTool always uses floats though and that is fine.

clayjohn · 2023-10-23T12:07:47Z

Just to clarify, using_normals_tangents is true when i step thru it, so it does not skip and will set r_aabb in the end with an incorrect z value for size like this when aabb is optimized out by compiler: [P: (-1, -1, -1), S: (2, 2.00001, 0.00001)]
Instead of operating on aabb and then assigning to r_aabb. Can you try modifying the code to operate on r_aabb directly? I.e.:
if (i == 0) {
	r_aabb = AABB(src[i], SMALL_VEC3);
} else {
	r_aabb.expand_to(src[i]);
}
Tried it, it does seem to mitigate the issue. Tho not sure if its an actual fix, suspect there's some underlying issue that cause aabb to have weird behavior when optimized out? (i should have time this weekend, will see if i can figure something out... no promise tho..)

My working theory is that the compiler sees that r_aabb is only guaranteed to be written out in the 2D case so it thinks that it is okay to optimize out the z component.

jsjtxietian mentioned this issue Oct 6, 2023

Support updating tooltip immediately after editor description change #82916

Merged

AThousandShips added bug topic:import topic:3d labels Oct 6, 2023

akien-mga added this to the 4.2 milestone Oct 6, 2023

akien-mga added the regression label Oct 6, 2023

akien-mga assigned clayjohn Oct 6, 2023

clayjohn mentioned this issue Oct 11, 2023

Fix a few cases where surface format is still 32 bits #83169

Merged

akien-mga closed this as completed in #83169 Oct 11, 2023

clayjohn reopened this Oct 11, 2023

Calinou mentioned this issue Oct 12, 2023

TorusMesh exhibits a visible line due to incorrect mesh normals #83175

Closed

clayjohn mentioned this issue Oct 12, 2023

Cleanup instances of using uint32_t for mesh formats #83211

Merged

atirut-w mentioned this issue Oct 13, 2023

Mesh compression breaks normal #83240

Closed

clayjohn mentioned this issue Oct 23, 2023

Ensure r_aabb is always used when creating surfaces through the RenderingServer #83840

Merged

akien-mga closed this as completed in #83840 Oct 26, 2023

AThousandShips mentioned this issue Oct 27, 2023

Upgrading meshes with mesh versioning tools yields different result than manually re-importing them. #84079

Closed

Imported models are flat. #82890

Imported models are flat. #82890

Comments

KnightNine commented Oct 6, 2023 • edited Loading

Godot version

System information

Issue description

IT'S FLATT!?!?

Steps to reproduce

Minimal reproduction project

aaronfranke commented Oct 6, 2023 • edited Loading

KnightNine commented Oct 6, 2023 • edited Loading

aaronfranke commented Oct 6, 2023 • edited Loading

LiveTrower commented Oct 6, 2023

clayjohn commented Oct 6, 2023

clayjohn commented Oct 6, 2023

KnightNine commented Oct 6, 2023

clayjohn commented Oct 6, 2023

KnightNine commented Oct 6, 2023

clayjohn commented Oct 6, 2023

LiveTrower commented Oct 6, 2023

KnightNine commented Oct 6, 2023 • edited Loading

LiveTrower commented Oct 7, 2023

KnightNine commented Oct 8, 2023 • edited Loading

kevinkuo52 commented Oct 8, 2023 • edited Loading

kevinkuo52 commented Oct 9, 2023 • edited Loading

KnightNine commented Oct 9, 2023 • edited Loading

kevinkuo52 commented Oct 9, 2023 • edited Loading

KnightNine commented Oct 9, 2023 • edited Loading

clayjohn commented Oct 11, 2023 • edited Loading

clayjohn commented Oct 11, 2023

LiveTrower commented Oct 11, 2023

clayjohn commented Oct 11, 2023

LiveTrower commented Oct 12, 2023

kevinkuo52 commented Oct 12, 2023 • edited Loading

KnightNine commented Oct 12, 2023 • edited Loading

clayjohn commented Oct 12, 2023

clayjohn commented Oct 12, 2023

LiveTrower commented Oct 12, 2023

clayjohn commented Oct 12, 2023

LiveTrower commented Oct 12, 2023

kevinkuo52 commented Oct 13, 2023 • edited Loading

kevinkuo52 commented Oct 13, 2023 • edited Loading

LiveTrower commented Oct 13, 2023

kevinkuo52 commented Oct 16, 2023 • edited Loading

clayjohn commented Oct 17, 2023

clayjohn commented Oct 19, 2023

KnightNine commented Oct 21, 2023 • edited Loading

kevinkuo52 commented Oct 22, 2023 • edited Loading

clayjohn commented Oct 23, 2023

clayjohn commented Oct 23, 2023

KnightNine commented Oct 6, 2023 •

edited

Loading

aaronfranke commented Oct 6, 2023 •

edited

Loading

KnightNine commented Oct 6, 2023 •

edited

Loading

aaronfranke commented Oct 6, 2023 •

edited

Loading

KnightNine commented Oct 6, 2023 •

edited

Loading

KnightNine commented Oct 8, 2023 •

edited

Loading

kevinkuo52 commented Oct 8, 2023 •

edited

Loading

kevinkuo52 commented Oct 9, 2023 •

edited

Loading

KnightNine commented Oct 9, 2023 •

edited

Loading

kevinkuo52 commented Oct 9, 2023 •

edited

Loading

KnightNine commented Oct 9, 2023 •

edited

Loading

clayjohn commented Oct 11, 2023 •

edited

Loading

kevinkuo52 commented Oct 12, 2023 •

edited

Loading

KnightNine commented Oct 12, 2023 •

edited

Loading

kevinkuo52 commented Oct 13, 2023 •

edited

Loading

kevinkuo52 commented Oct 13, 2023 •

edited

Loading

kevinkuo52 commented Oct 16, 2023 •

edited

Loading

KnightNine commented Oct 21, 2023 •

edited

Loading

kevinkuo52 commented Oct 22, 2023 •

edited

Loading