Page MenuHome

Cycle Bake - Average Normals for projection
Needs RevisionPublic

Authored by Marco Alamia (Macro) on Tue, Nov 27, 6:04 AM.

Details

Summary

The issue I am trying to solve happens when baking a texture along a hard edge.
I have attached an explanation with pictures and a blend file.


Diff Detail

Event Timeline

Marco Alamia (Macro) edited the summary of this revision. (Show Details)
Lukas Stockner (lukasstockner97) requested changes to this revision.Wed, Nov 28, 4:18 AM

Thanks for the patch, this looks really good!

I'm not exactly a baking expert, but the functionality looks reasonable to me.

I've got a few notes inline, but most of it is just nitpicking, no real concerns (except for the pointer cast).

General note on the code style (more nitpicking, sorry): As far as I can see this file uses spaces after if/for and opening braces on the same line with a space before them, so new code should stick to that as well. Also, the existing code unfortunately tends to skip braces around single statements, but for new code that should be avoided.

source/blender/render/intern/source/bake_api.c
188

Please use regular comments instead of pseudocode that looks like valid C to explain the algorithm.

Actually in this case, a binary search should be common enough to not need any explanation...

189

When you're calling vertex_to_sort_compare here, the pointer to the float array sorted_positions is cast to VertexToSort* and it only works because you only access the first element which happens to be a float array as well. Just calling position_compare would be a lot cleaner.

595

We're using C99 now, so you can move those declarations to where they're actually needed (e.g. normal[3] to line 645). Loop indices make sense here though.

The existing code is mostly still written for C89.

609

You can shorten this code a lot by using copy_v3_v3 etc. and doing a loop over the three vertices per tri.

634

Those two arrays are only references once each, I don't think the additional shortcut variable is really needed.

679

I think you should be able to get rid of this pointer by always passing &average_normal_data and checking average_normal_data->count > 0 in calc_point_from_barycentric_extrusion.

691

You can shorten this initialization to just AverageNormalData average_normal_data = {0};

This revision now requires changes to proceed.Wed, Nov 28, 4:18 AM

Thanks Lukas for taking the time to code review my patch.

I have applied all the changes. The compare one was a legit bug, that function was operating on vertices rather then just position originally and I failed to change the compare function when I moved to positions. Thanks for catching that!

Marco Alamia (Macro) marked 7 inline comments as done.

No changes from the previous revision - I have a feeling that I did something wrong when submitting the previous time since I have flagged some of the review as "Done" but the tool insist they are "Unsubmitted". I am not clear if it's my fault but it should be harmless to resubmit the same review.

Brecht Van Lommel (brecht) requested changes to this revision.Wed, Dec 5, 5:40 PM

Is this different than just setting smooth normals on the low poly base mesh? Is the way the averaging works better in some way? Is it a matter of usability so user don't need to modify the low poly meshes?

To me it seems odd that we are using binary search to figure out which face corners share a vertex, when that information is already defined in the mesh. Or is this for modifiers like edge split that split the mesh, where that connectivity information is lost?

source/blender/editors/object/object_bake_api.c
1488

This should have an explanation of why this is needed.

Do you think this should be enabled by default?

source/blender/makesrna/intern/rna_scene.c
4341

Same as above.

source/blender/render/intern/source/bake_api.c
124–125

Code style convention:

typedef struct VertexToSort {
176

spcified -> specified.

This revision now requires changes to proceed.Wed, Dec 5, 5:40 PM

To me it seems odd that we are using binary search to figure out which face corners share a vertex, when that information is already defined in the mesh. Or is this for modifiers like edge split that split the mesh, where that connectivity information is lost?

Unless it is actually trying to support edge split, is there any reason this can't just use the actual Mesh vertex normals?

Marco Alamia (Macro) marked 2 inline comments as done.

Thank you for your time and for the review.

To me it seems odd that we are using binary search to figure out which face corners share a vertex, when that information is already defined in the mesh. Or is this for modifiers like edge split that split the mesh, where that connectivity information is lost?

Yes, it is for modifiers like edge splits or the autosmooth feature. Once the normals are split I don't think the mesh has a way to recover the "averaged version" anymore. If there is a way to do it that I am not aware of I wouldn't mind switching to that; I ended up with that binary search because I had to create the avg normal data from the split normals.

Is this different than just setting smooth normals on the low poly base mesh? Is the way the averaging works better in some way? Is it a matter of usability so user don't need to modify the low poly meshes?
Unless it is actually trying to support edge split, is there any reason this can't just use the actual Mesh vertex normals?

It is exactly for edge splits and autosmooth.
You are correct that if you were to use smooth normals you would get exactly the same result as far as the projection is concerned; the issue though is that the TBN would also be using that normal, which means that the resulting normal map would not be "flat" on flat regions, making it impossible to manually paint it for cases where you might want to add in details (such as screws or grooves or whatnot).
This feature doesn't change the geometrical normals so the TBN remains flat, the normal map is flat, but the projection has used a "bent" normal fixing the seam that would appeared using the geometrical flat normal.
I can draw the various cases if this is not clear, it's not the easiest thing to explain with words :)

In summary:

  • Not smoothed mesh + Bake = incorrect projection
  • Smoothed mesh + Bake = correct projection but yields normal maps that are impossible to edit
  • Not smoothed mesh + Bake + Avg Normal = correct projection, good normal map
  • Smoothed mesh + Bake + Avg Normal = same as Smoothed mesh + Bake
source/blender/editors/object/object_bake_api.c
1488

Are you sure you want an explanation of why is needed there?
All other properties seem to have an explanation of what it does rather than why. If you feel strongly about it I can try to put an explanation of why but it might become a bit verbose. Let me know.
I just checked Painter, their help reads: "Compute rays directions based on averaged normals"

Do you think this should be enabled by default?
Good question, I don't know. It's enabled by default in Painter, but I am not 100% sure what's best.

It is exactly for edge splits and autosmooth.

Don't mix those two together, they are completely different. Edge split actually splits the mesh completely, while autosmooth only splits the normals. Thus with autosmooth there are still shared vertices and averaged vertex normals.

It is exactly for edge splits and autosmooth.

Don't mix those two together, they are completely different. Edge split actually splits the mesh completely, while autosmooth only splits the normals. Thus with autosmooth there are still shared vertices and averaged vertex normals.

I see, so I could technically get rid of the extra calculation for avg normals in the autosmooth case but I will still need it for edge splits? Is it worth treating them differently though? Is it easier to let them both flow through the same code path?

The Edge Split modifier is from before Autosmooth worked in the viewport and supported sharp edges and custom normal editing. There are not a lot of people using the modifier anymore, as far as I know. It is mostly there for legacy reasons.

Personally I think it would be fine if this only worked with Autosmooth, with simplified code.

source/blender/editors/object/object_bake_api.c
1488

The explanation could be as short as: "Use smooth averaged normals for projection, to avoid continuity artifacts around sharp edges."

Brecht Van Lommel (brecht) requested changes to this revision.Tue, Dec 11, 9:00 PM
This revision now requires changes to proceed.Tue, Dec 11, 9:00 PM

My new Target Normal Projection mode in the shrinkwrap modifier (basically intended to address a vaguely similar mapping continuity problem to this) simply uses the vertex normals and thus won't be smooth with edge split either.

Don't mix those two together, they are completely different. Edge split actually splits the mesh completely, while autosmooth only splits the normals. Thus with autosmooth there are still shared vertices and averaged vertex normals.

@Alexander Gavrilov (angavrilov)
Are you sure about the averaged normals being there when using autosmooth? I have created a simple mesh that I have autosmoothed but the mesh (struct Mesh in code) doesn't seem to have them. Am I looking at the wrong data structure? Is there another way to access per vertex information that has averaged normals?
I have attached an image to detail what the mesh looks like and the Mesh vertex data that goes with it.

@Brecht Van Lommel (brecht)
Is the sort the main concern? While I was searching for the averaged normals that Alexander mentioned I realized that the bake mechanism is actually creating the triangles by reading the Mesh data. I could change the triangle creation code to store the average normal so that it would be readily accessible during baking. The only issue is that I would probably still need to sort and search if the average normals aren't stored somewhere already.

Would this fix this T59362?

Yes, it gives you a tool to fix that.

The way the original asset is setup will generate those results, that can't be fixed. If the geometrical normal is bent your TBN is bent and the colors are not flat.
If you straighten the normals though you get flat colors, but in the current blender you also get an incorrect projection and bad seams in the normal map. With the average normal for projection you fix the projection.
I have attached what I did to get the desired results.