Skip to content

Correctly export vertex normals #434

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

Merged
merged 2 commits into from
Jan 12, 2025
Merged

Conversation

Jrius
Copy link
Collaborator

@Jrius Jrius commented Jan 9, 2025

As it is now, Korman does not correctly export vertex normals as they appear in Blender.

Blender computes actual mesh normals (=what is used by the viewport for shading) according to (non exhaustive list):

  • shared vertices between polygons
  • polygon smoothness (use_smooth)
  • edge sharpness (use_edge_sharp)
  • mesh auto smooth+angle (mesh use_auto_smooth & auto_smooth_angle). Very useful for modelling machines with both smooth and sharp angles !
  • potential custom split normals data (not sure how it works to be honest)

On export, Korman currently tries to reuse common vertices between polygons (which is good), but is a bit too eager and reuses vertices which are part of polygons with different normals. It also doesn't use the right property to get the actual normal that respects all of the above (edge sharpness, auto smooth angle, etc). There are some times where it kinda works and others where it fails, but I won't get into the details.

What this MR does is retrieve the true normal (again, what is used by Blender's viewport, more or less) and use that when looking for an existing vertex / exporting a new vertex.

Pros:

  • Lighting effects that involve vertex normals now behave closer to the Blender counterparts. Off the top of my head, this means runtime lighting, specular highlights, and reflection cubemaps. Very important IMHO, I'm a sucker for properly split normals.

Cons:

  • Slightly heavier DrawableSpan due to more vertices being generated.
  • May hit the vertex number limit earlier, even for people who don't want or need normal-based effects.

Backwards compatibility: previous files will see improvements if re-exported, if they relied on any of the previously mentioned effects (RT light, specular, cubemaps). No effect on baked lighting. May cause big meshes that previously exported fine to hit the vertex limit and fail.

Example of properly exported normals:
MR_normals_image
Left: Plasma result of a cubemapped object. Middle: mesh normals, seen using Blender's matcap. Right: edit mode displaying sharp edges.

Copy link
Member

@Hoikas Hoikas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a good improvement. I have one minor concern with the list comprehension added on line 451, though. In Python 3.11 and lower, list comprehensions are implemented as Python function calls under the hood. These are very expensive, and this is a fairly tight loop. I'd like to avoid this, if possible. The suggestion I'm attaching is what I think would do that.

@Jrius
Copy link
Collaborator Author

Jrius commented Jan 12, 2025

Good catch, I did not know about that. On a light scene it does shave off a few dozen milliseconds - not much but may be useful for iterating on bigger scenes.

@Hoikas Hoikas merged commit 8b67d62 into H-uru:master Jan 12, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants