Page MenuHome

Loop normals display not working if have custom split normals
Closed, ResolvedPublic

Description

System Information
MacOS 10.14. Intel HD Graphics 530 1536 MB

Blender Version
Broken: 2.8 hash 642fe9e0f2a95 Nov 4

Short description of error
If one sets up custom normals on a mesh and then modifies them, and enables loop normals in the overlay options, then the displayed normals stay at whatever the loop normals would be just for the face that the loop is for.

Exact steps for others to reproduce the error
This blend


was made by setting autosmooth on the cube, making the top edges sharp, setting the object to smooth shading, then using alt-L to enter normal editing mode, then using m (point at mouse) and moving the mouse a bit, and then left clicking. The matcap shows that normals are affected when doing this.

The commit 478899dee771 appears to have added loop normal display support so I'll assign to fclem but maybe the problem is due to a number of TODOs in draw_cache_impl_mesh.c about "real loop normals".

Event Timeline

I'm not sure if this should be assigned to Clement, Campbell, or someone else. Also not sure if this is a bug that is intended to be fixed before the beta.

Philipp Oeser (lichtwerk) triaged this task as Confirmed, Medium priority.Nov 5 2018, 2:33 PM

Seems like a TODO in add_overlay_tri.

/* TODO real loop normal */

I might have a look if @Campbell Barton (campbellbarton) is too busy.

the overlay also displays bad amount of normals, in 2.79 it works as it should by my opinion:

2.79
2.8

Campbell, your recent commit rBff3601b9 seems to have broken some of the code that was written to fix this, or maybe just broke the code that fetches and uses custom normals in shading. Now when autosmooth is on it doesn't seem like the workbench display uses custom normals at all (e.g., trying to modify them with the mesh -> normals -> point normals to target command, using m for mouse -- nothing happens now).

Can you have a look please?

I put this in my draw_cache_impl_mesh.c so that I could turn on #define of USE_BM_MAPPED_LOOPNORMAL and get it to use custom loop normals properly in display.

			const int l_orig = rdata->mapped.l_origindex ? rdata->mapped.l_origindex[mlt->tri[i]] : mlt->tri[i];

but I'm pretty sure this is not the right fix, else you likely would have done this in the first place.
Is the issue that mlt->tri[] is just wrong if there's a modifier that sets things on the CD_ORIGINDEX layer?

People will likely start filing bugs about "custom split normals don't work at all" if we don't do something to use the CD_CUSTOMLOOPNORMAL layer in shading.

No idea about bmesh stuff, but with regular meshes the ORIGINDEX layer doesn't exist if the object only has deform modifiers and thus the topology is unchanged from base, and you have to account for that.