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

Investigate minifig head lighting #641

Closed
leozide opened this issue Mar 7, 2021 · 24 comments
Closed

Investigate minifig head lighting #641

leozide opened this issue Mar 7, 2021 · 24 comments
Labels

Comments

@leozide
Copy link
Owner

leozide commented Mar 7, 2021

image

@leozide leozide added the bug label Mar 7, 2021
@leozide
Copy link
Owner Author

leozide commented Mar 13, 2021

Also on heads:

image

3626cpx3.dat in ldrawunf.zip

@rsbx
Copy link
Contributor

rsbx commented Mar 14, 2021

Looking at torso 973 and head 3626a using wireframe shading with LC_DEBUG_NORMALS enabled, it appears that the normals of the affected areas don't point where I'd expect them to. BTW, the torso also has the issue on the "neck".

@rsbx
Copy link
Contributor

rsbx commented Mar 14, 2021

Hmmm; if I disable my BFC code, 3626a and the neck of 973 are fine. But, 973 still has issues where the arms go.

@rsbx
Copy link
Contributor

rsbx commented Mar 14, 2021

Another OFFICIAL part with normal issues: 14769, Tile 2x2 Round with Round Underside Stud. Specifically the areas by the notches.

@rsbx
Copy link
Contributor

rsbx commented Mar 15, 2021

Attached is a detail image of part 14769, Tile 2x2 Round with Round Underside Stud, showing the vertex normals in wireframe mode using orthographic projection from above. There are a number of triangles & quads with vertex normals that do match the normal of their triangle/quad in the area that I highlighted. I suspect the issue affecting minifig torsos, e,g, 973.dat, is related to what is going on here.

tile_14769

To get this view, I made the following changes to the code at commit b10d895:

  • Defined LC_DEBUG_NORMALS to 1 in common/lc_scene.cpp
  • Changed the calls to MeshLoader() in common/lc_library.cpp and pieceinf.cpp to disable vertex optimization and normal averaging
  • Deleted all of the cache files via rm -rf

@leozide
Copy link
Owner Author

leozide commented Mar 16, 2021

These are a pain to track down but you need to do it with smoothing enabled

@rsbx
Copy link
Contributor

rsbx commented Mar 16, 2021 via email

@rsbx
Copy link
Contributor

rsbx commented Mar 16, 2021

I'm pretty sure the bad input normals are caused by shear components in the transformation matrices in the definitions of 973 (torso) and 14769 (round tile). For example, the follow matrix, from the outer, left side, 4-4ndis.dat primitive of 973, appears to ONLY have shear and scale components.

-0.85 0 0
4.925 1 0
0 0 5

I have not investigated 3626cpx3 since it's an unofficial part and I don't have them loaded but, from the image, it appears that the normals are inverted for 1/2 the head.

@rsbx
Copy link
Contributor

rsbx commented Mar 17, 2021

Here's the patch to fix the issue:

normalfix.patch.txt

For those interested: normals need to be transformed by the transpose of the inverse of the transform matrix.

@rsbx
Copy link
Contributor

rsbx commented Mar 17, 2021

It looks like a couple of the shaders need to be fixed also but that's beyond me.

@leozide
Copy link
Owner Author

leozide commented Mar 17, 2021

I think you're on the right track, I tried to orthonormalize instead and it looks like the Minifig torso is fixed. I don't think transposing the inverse is really needed, it may be fixing the matrix as a consequence of the inverse operation.

@ghost
Copy link

ghost commented Mar 17, 2021

Also happen with eyes on many heads parts:

pic.1

pic.2

@rsbx
Copy link
Contributor

rsbx commented Mar 17, 2021 via email

@leozide
Copy link
Owner Author

leozide commented Mar 17, 2021

Both methods are removing non uniform scaling so the results will be equivalent for orthogonal matrices.

Shaders don't need to change because the world matrices are affine transforms.

@rsbx
Copy link
Contributor

rsbx commented Mar 17, 2021 via email

@leozide
Copy link
Owner Author

leozide commented Mar 17, 2021

Ok, the transforms are orthonormal

@rsbx
Copy link
Contributor

rsbx commented Mar 17, 2021

The cost of the matrix inverse could be lessened by inverting only the top-left 3x3 matrix of the 4x4 transform matrix since the normal is a vector. But I've not noticed a performance degradation with the normal fix so it may not be worth the effort.

@leozide leozide changed the title Investigate minifig torso lighting Investigate minifig head lighting Mar 18, 2021
@rsbx
Copy link
Contributor

rsbx commented Mar 18, 2021

@Symbian9 what are the part IDs for those heads you found with issues?

@ghost
Copy link

ghost commented Mar 18, 2021

what are the part IDs for those heads you found with issues?

That was 3626bp61.dat and 3626bpa1.dat.

FTR, In latest LeoCAD b548e1f build (I use LeoCAD-Linux-b548e1f-x86_64.AppImage) it seems like issue already fixed:

pic.1

@rsbx
Copy link
Contributor

rsbx commented Mar 18, 2021

Great! Likely caused by the normals calculation issue that was just fixed.

I see you found the High Contrast stud style option.

@rsbx
Copy link
Contributor

rsbx commented Mar 20, 2021

Can this issue be closed?

@rsbx
Copy link
Contributor

rsbx commented Mar 25, 2021

The shading discontinuity with 3626cpx3 is cause by the texture coordinates not matching where the front and back textures meet. Since the compare fails, a new vertex in add and the there's no normal smoothing.

@rsbx
Copy link
Contributor

rsbx commented Mar 26, 2021

That should fix 3626cpx3 & friends:
addvertex.patch.txt

@rsbx
Copy link
Contributor

rsbx commented Mar 26, 2021

Improved patch:

addvertex.patch.txt

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants