Details

Reviewers

Commits

rB449db0ab1e34: Baking: new method to generate margin, based on adjacent faces

Summary

Add a new way of generating texture margins by copying pixels from adjacent polygons instead of repeating border pixels. Improves T92943.

Though there's still stuff that needs to happen (mainly UI) I'm open for comments.

Though I guess you want to wait till after v3.0 is released and the chaos has died down ;-)

Diff Detail

Repository

rB Blender

Branch

texture-margin (branched from master)

Build Status

Buildable 18958
Build 18958: arc lint + arc unit

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Harbormaster completed remote builds in B19016: Diff 45370.Nov 28 2021, 6:23 PM

Steffen Hartmann (Harti) awarded a token.Nov 28 2021, 7:48 PM

Changing bake_filter to bake_margin might break scripts? I need to make sure that I can do that, or else rename it back.

clang-format
Move bake_margin_type to a more logical place and fix padding.
Handle generating a new margin for textures without alpha or mask.
Add an operator to call the margin generation for any bitmap.
Add the margin_copypixels operator to the image menu of the imaged editor.
Todo comment updated.
More explanatory comments, and a small clarification the the code.
Codestyle.
Merge branch 'master' into texture-margin
Merge branch 'master' into texture-margin
Add forgotten source file to git.
Put pointers to the MLoop and MPoly lists we actually need in the Map class, instead of a single pointer to Mesh.
Add support for copypixel margins to multiresolution baking.
Update todolist comment.
Merge branch 'master' into texture-margin
Rename generate_border -> generate_margin, to be more consistent.
Remove the debugfile output code.
Clang-format

Harbormaster completed remote builds in B19187: Diff 45617.Dec 3 2021, 9:34 PM

Start of implementation of new texture margin generation.
Reimplement (with lots of copypasting) the prototype inside blender
Fix a typo, lots of debugging help start with xtending the old way to help linear interpolation.
Only mirror on edges which are facing towards us.
Look up pixels in multiple polygons, if the margin is wider than the next polygon
Do all calculations in pixel space instead of uv space.
Use the IMB_filter_extend filter to fill in the missing parts.
Code cleanup. Get rid of static globals.
Move system includes to after blender includes.
Change all doubles to floats.
Use only unsigned ints where they really need to be unsigned.
Change the namespace to be in line with the codestyle guidelines.
Add some comments.
Replace the custom datatypes from the prototype with a single mapping array.
Make sure the final extend step doesn't mess up the interior of our texture.
Replace std::vector with blender::Vector .
Renamed functions to follow the blender stylle guide.
Renamed variables to be in line with the blender style guide.
Convert from uv to pixel coordinates in a consitent way.
Add some comments.
Layout.
Some more variable renaming and an extra comment added.
Todolist comment amended.
Document the entrypoint.
clang-format
Add a gui dropdown to select the new margin algoirithm.
Run clang-format on the changed files.
Fix comments spelling/formatting. Thanks Garek for checking them!
Update top comment todo list.
Clang format.
Remove unused variable to silence gcc warning.
clang-format
Move bake_margin_type to a more logical place and fix padding.
Handle generating a new margin for textures without alpha or mask.
Add an operator to call the margin generation for any bitmap.
Add the margin_copypixels operator to the image menu of the imaged editor.
Todo comment updated.
More explanatory comments, and a small clarification the the code.
Codestyle.
Add forgotten source file to git.
Put pointers to the MLoop and MPoly lists we actually need in the Map class, instead of a single pointer to Mesh.
Add support for copypixel margins to multiresolution baking.
Update todolist comment.
Rename generate_border -> generate_margin, to be more consistent.
Remove the debugfile output code.
Clang-format
Todo comment.
Remove todo comment, add licence comment.
When the operator is called from an image editor, use the image form the editor instead of the selected object's material.
clang-format

Harbormaster completed remote builds in B19324: Diff 45854.Dec 9 2021, 1:18 PM

Rebased onto master

Start of implementation of new texture margin generation.
Reimplement (with lots of copypasting) the prototype inside blender
Fix a typo, lots of debugging help start with xtending the old way to help linear interpolation.
Only mirror on edges which are facing towards us.
Look up pixels in multiple polygons, if the margin is wider than the next polygon
Do all calculations in pixel space instead of uv space.
Use the IMB_filter_extend filter to fill in the missing parts.
Code cleanup. Get rid of static globals.
Move system includes to after blender includes.
Change all doubles to floats.
Use only unsigned ints where they really need to be unsigned.
Change the namespace to be in line with the codestyle guidelines.
Add some comments.
Replace the custom datatypes from the prototype with a single mapping array.
Make sure the final extend step doesn't mess up the interior of our texture.
Replace std::vector with blender::Vector .
Renamed functions to follow the blender stylle guide.
Renamed variables to be in line with the blender style guide.
Convert from uv to pixel coordinates in a consitent way.
Add some comments.
Layout.
Some more variable renaming and an extra comment added.
Todolist comment amended.
Document the entrypoint.
clang-format
Add a gui dropdown to select the new margin algoirithm.
Run clang-format on the changed files.
Fix comments spelling/formatting. Thanks Garek for checking them!
Update top comment todo list.
Clang format.
Remove unused variable to silence gcc warning.
clang-format
Move bake_margin_type to a more logical place and fix padding.
Handle generating a new margin for textures without alpha or mask.
Add an operator to call the margin generation for any bitmap.
Add the margin_copypixels operator to the image menu of the imaged editor.
Todo comment updated.
More explanatory comments, and a small clarification the the code.
Codestyle.
Add forgotten source file to git.
Put pointers to the MLoop and MPoly lists we actually need in the Map class, instead of a single pointer to Mesh.
Add support for copypixel margins to multiresolution baking.
Update todolist comment.
Rename generate_border -> generate_margin, to be more consistent.
Remove the debugfile output code.
Clang-format
Todo comment.
Remove todo comment, add licence comment.
When the operator is called from an image editor, use the image form the editor instead of the selected object's material.
clang-format

Harbormaster completed remote builds in B19326: Diff 45857.Dec 9 2021, 1:28 PM

Add forgotten source file to git.
Start of implementation of new texture margin generation.
Reimplement (with lots of copypasting) the prototype inside blender
Fix a typo, lots of debugging help start with xtending the old way to help linear interpolation.
Only mirror on edges which are facing towards us.
Look up pixels in multiple polygons, if the margin is wider than the next polygon
Do all calculations in pixel space instead of uv space.
Use the IMB_filter_extend filter to fill in the missing parts.
Code cleanup. Get rid of static globals.
Move system includes to after blender includes.
Change all doubles to floats.
Use only unsigned ints where they really need to be unsigned.
Change the namespace to be in line with the codestyle guidelines.
Add some comments.
Replace the custom datatypes from the prototype with a single mapping array.
Make sure the final extend step doesn't mess up the interior of our texture.
Replace std::vector with blender::Vector .
Renamed functions to follow the blender stylle guide.
Renamed variables to be in line with the blender style guide.
Convert from uv to pixel coordinates in a consitent way.
Add some comments.
Layout.
Some more variable renaming and an extra comment added.
Todolist comment amended.
Document the entrypoint.
clang-format
Add a gui dropdown to select the new margin algoirithm.
Run clang-format on the changed files.
Fix comments spelling/formatting. Thanks Garek for checking them!
Update top comment todo list.
Clang format.
Remove unused variable to silence gcc warning.
clang-format
Move bake_margin_type to a more logical place and fix padding.
Handle generating a new margin for textures without alpha or mask.
Add an operator to call the margin generation for any bitmap.
Add the margin_copypixels operator to the image menu of the imaged editor.
Todo comment updated.
More explanatory comments, and a small clarification the the code.
Codestyle.
Put pointers to the MLoop and MPoly lists we actually need in the Map class, instead of a single pointer to Mesh.
Add support for copypixel margins to multiresolution baking.
Update todolist comment.
Rename generate_border -> generate_margin, to be more consistent.
Remove the debugfile output code.
Clang-format
Todo comment.
Remove todo comment, add licence comment.
When the operator is called from an image editor, use the image form the editor instead of the selected object's material.
clang-format

Harbormaster completed remote builds in B19327: Diff 45860.Dec 9 2021, 1:39 PM

Remove accidentally added include.

Harbormaster completed remote builds in B19328: Diff 45861.Dec 9 2021, 2:11 PM

Some quick comments from glancing over the code, no time to review the specifics of the algorithm right now, that may have to wait until January.

I don't think we should have the operator, even if it may be useful in some cases. It seems like a poor fit to have something so tightly coupled to baking logic but still separate from it.

source/blender/render/intern/texture_margin.cc
51	Use float2, float3, .. instead.
77	Remove this scanline pointer array mechanism, just pixel data directly.
134–138	Use less obscure names for macros.
337–338	Don't abbreviate already short terms like `edge_point` to `ep`.
548–550	This is only needed in the header file.

This revision now requires changes to proceed.Dec 9 2021, 8:07 PM

In D13303#357506, @Brecht Van Lommel (brecht) wrote:

Some quick comments from glancing over the code, no time to review the specifics of the algorithm right now, that may have to wait until January.

Ok, have a nice holiday ;-)

I don't think we should have the operator, even if it may be useful in some cases. It seems like a poor fit to have something so tightly coupled to baking logic but still separate from it.

it's not really tightly coupled to the baking logic though. Just to the UV map. IMHO it's quite handy to have this as an operator (a menu entry is probably overkill, but callable from python) to generate margins for existing models. Quite a few models you can download from various online sources have textures with bad (or none at all) margins. I myself already used it to improve a minetest model for my kids ;-)

But to keep it simple I've removed the operator parts in my local branch for now, and I'll look into the rest of the changes you requested.

Adding an operator can be done later if it turns out to be desirable.

Gavriel Lavi (Gavriel5578) awarded a token.Dec 11 2021, 12:25 AM

Did the requested changes

Martijn Versteegh (Baardaap) marked an inline comment as done.Dec 11 2021, 12:06 PM

Remove the operator code.
Replaced fvec with float2 and renamed some short variables.
(Hopefully) clearer macro names.
Removed scanline access pointers.
Remove unneeded 'extern "C"'

Harbormaster completed remote builds in B19363: Diff 45934.Dec 11 2021, 12:08 PM

Merge branch 'master' into texture-margin

Harbormaster completed remote builds in B19364: Diff 45935.Dec 11 2021, 12:12 PM

Renamed variable to match styleguide.

Harbormaster completed remote builds in B19365: Diff 45936.Dec 11 2021, 12:15 PM

Merge branch 'master' into texture-margin
Add the renaming of bake_filter to bake margin to dna_rename_defs.h

Harbormaster completed remote builds in B19900: Diff 46902.Jan 11 2022, 4:02 PM

Patch for the docs:

Index: manual/render/cycles/baking.rst
===================================================================
--- manual/render/cycles/baking.rst     (revision 8808)
+++ manual/render/cycles/baking.rst     (working copy)
@@ -234,6 +234,17 @@
          Baked result is extended this many pixels beyond the border of each UV "island",
          to soften seams in the texture.
 
+      .. _bpy.types.BakeSettings.margin_type:
+
+      Margin Type
+         How to generate the margin around the UV "islands".
+
+         :Extend:
+            Generate a margin by repeating the border pixels outwards.
+
+         :CopyPixels:
+            Generate a margin by copying pixels from the adjacent UV island.
+
       .. _bpy.types.BakeSettings.use_clear:
 
       Clear Image

This seems to work fine in my tests with a few meshes. Only relatively minor comments on the code, since this already has gone through many iterations.

Have you verified this works with selected to active baking?
Suggest to rename this from "CopyPixels" to "Adjacent Faces", I think that describes it more clearly. Also the terminology we generally use in Blender is "Adjacent Faces" rather than "Neighboring Polygons" (see Mesh edit mode menus).
Unless there are many failure cases, probably this should be the default?
UI could be tweaked to make a "Margin" subpanel with "Type" and "Size" settings. Generally we put "type" settings go on top.

source/blender/editors/object/object_bake_api.c
781–787	Can this code be moved into `generate_margin`, and also take into account the `uv_layer` name for the derivedmesh case there?
source/blender/render/intern/texture_margin.cc
55	I'd give this a more specific name like `TextureMarginMap`. It's in a namespace but still feels confusing to me to call this `Map` just like `BLI_map.`.
205	I guess it's easy enough to reuse this by making it `static const` in the class?
268	Rename `uv_to_xy`.
474–490	Would move this loop tri code a little higher to got with accessing other mesh data layers.
493	Remove unused code.

This revision now requires changes to proceed.Jan 13 2022, 6:38 PM

I don't even really know what the patch does, but I was curious so I read through the patch. Noticed some small things and figured I would comment

source/blender/editors/object/object_bake_api.c
2046	`What` -> `Which`
source/blender/render/RE_texture_margin.h
36–45	These shouldn't need to be declared with `extern`
source/blender/render/intern/texture_margin.cc
288	No need to use `blender::` here.
409–425	Think it would be nice to stick to the class layout from the style guide: https://wiki.blender.org/wiki/Style_Guide/C_Cpp#Class_Layout Also don't have a strong opinion on this, but I think the convention in Blender code is west const.
480–484	Just curious, why not use `BKE_mesh_runtime_looptri_ensure`?

Martijn Versteegh (Baardaap) added inline comments.Jan 13 2022, 9:18 PM

source/blender/render/intern/texture_margin.cc
205	woops, I meant to do that, but I forgot ! ;-)
409–425	I can do it with west const. I like east const better because it's more consistent (the const always protects the thing left of it) . To me MPoly const * const variable; is more consistent than const MPoly * const variable; But I don't have a strong opinion either way, and if the rest of blender uses west const than I'll change it. I'll have a look at that Class Layout guide, I must admit I hadn't studied that one yet.
480–484	Because I didn't know that existed ;-) I copy-pasted this from somewhere else.

Vyacheslav (hitrpr) awarded a token.Jan 14 2022, 12:18 AM

Brecht Van Lommel (brecht) added inline comments.Jan 14 2022, 12:37 PM

source/blender/render/intern/texture_margin.cc
480–484	Using `ensure` will permanently store the looptris and increases memory usage that way. In this case I think the time taken by baking is significant anyway, so would rather not increases memory usage to potentially save a bit of time somewhere.

Moved lookup of MLoopUV data to generate_margin( )
Fixed description string. (What algorithm -> Which algorithm)
Renamed uvtoxy to uv_to_xy(). Removed blender:: namespace spcifier from Vector. Made the direcions a static onct member of the class.
Moved looptri generation together with other data layer accesses.
Removed extraneous 'extern's and unused code.

Harbormaster completed remote builds in B20002: Diff 47080.Jan 14 2022, 4:24 PM

Martijn Versteegh (Baardaap) added inline comments.Jan 14 2022, 4:24 PM

source/blender/editors/object/object_bake_api.c
781–787	I moved it there. That is slightly more logical. However I still need to pass the char const *uv_layer in, and the MultiRes baking doesn't have that, so it doesn't really simplify the code as much as I'd like. Still more clear this way I think.
source/blender/render/intern/texture_margin.cc
288	hm. It feels somewhat scary to me to use a generic name like Vector without explicit namespace , but ok.
496–505	@Brecht Van Lommel (brecht) I copied this code from where the baking pixels are generated. It feels slightly dodgy to duplicate this code here, especially with the added workaround. Should I create a utility function for this?

Martijn Versteegh (Baardaap) marked an inline comment as done.Jan 14 2022, 5:00 PM

Martijn Versteegh (Baardaap) added inline comments.

source/blender/render/intern/texture_margin.cc
409–425	Oh, now I marked this as done, but I didn't yet change the const positions.... Tomorrow maybe. I feel a slight mental resistance :-) ...

Lopo Isaac (lopoIsaac) awarded a token.Jan 14 2022, 5:21 PM

Have you verified this works with selected to active baking?

I did a few quick tests some time ago and it seemed to work ok. I can't really think of a reason why it wouldn't. But more thorough testing might be good.

Suggest to rename this from "CopyPixels" to "Adjacent Faces", I think that describes it more clearly. Also the terminology we generally use in Blender is "Adjacent Faces" rather than "Neighboring Polygons" (see Mesh edit mode menus).

Adjacent Faces is better indeed. The CopyPixels was more of a placeholder because I couldn't think of a better name.

Unless there are many failure cases, probably this should be the default?

I was a bit scared to do that. But I *think* it would be a good default. I need to run some more tests with meshes with very small polygons though, I'm not really sure how ell it works in that case (polygons narrower than ~2-3 pixels)

UI could be tweaked to make a "Margin" subpanel with "Type" and "Size" settings. Generally we put "type" settings go on top.

I'll have a look at it. The gui code still slightly scares me ;-)

Follow the blender style guide for class layout.
Rename CopyPixels to AdjacentFaces
Put the margin type above the margin size in the UI.

Harbormaster completed remote builds in B20008: Diff 47090.Jan 14 2022, 11:24 PM

Put the margin setting in their own panel.
Set the default margin generation to ADJACENTFACES.

Harbormaster completed remote builds in B20014: Diff 47097.Jan 15 2022, 9:41 PM

I put the margin settings into their own sub-panel. Though I'm not really sure I like the way that works out. To me it feels more like adding clutter than removing clutter.

I also set the default to AdjacentFaces, but I need to look over all places where the default is set carefully, because I don't really trust I did it right. Just setting the default to adjacentfaces was not enough, it only started working when I also put AdjacentFaces in the top spot in the gui (and swapped ther integer values in the enum, though I'm not sure if that was actually needed).

Merge branch 'master' into texture-margin

Harbormaster completed remote builds in B20021: Diff 47108.Jan 16 2022, 7:51 PM

Brecht Van Lommel (brecht) mentioned this in rBM8817: Render: update for new baking margin type.Jan 17 2022, 7:39 PM

Brecht Van Lommel (brecht) mentioned this in rBL62791: Tests: update for new bake margin type.

Thanks for all the work. I'll commit this with some naming tweaks and a memory leak fix. Will also commit an update to the manual.

This revision is now accepted and ready to land.Jan 17 2022, 7:41 PM

Closed by commit rB449db0ab1e34: Baking: new method to generate margin, based on adjacent faces (authored by Martijn Versteegh (Baardaap), committed by Brecht Van Lommel (brecht)). · Explain WhyJan 17 2022, 7:41 PM

This revision was automatically updated to reflect the committed changes.

Brecht Van Lommel (brecht) added a commit: rB449db0ab1e34: Baking: new method to generate margin, based on adjacent faces.

Paul Larson (GeorgiaPacific) added a subscriber: Paul Larson (GeorgiaPacific).Jan 17 2022, 9:26 PM

In D13303#367731, @Brecht Van Lommel (brecht) wrote:

Thanks for all the work. I'll commit this with some naming tweaks and a memory leak fix. Will also commit an update to the manual.

Ah. I tried to push a new diff because I found a memory leak. But it bounced because the diff was already closed ;-)
hopefully it was the same leak. I'll check.

(edit: yes, it was the same one ;-) )

Omar Emara (OmarSquircleArt) mentioned this in T92904: Blender cannot bake from box blended mapping without seams.Mar 23 2022, 9:01 AM

Start of implementation of new texture margin generation.
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 45257

source/blender/editors/object/object_bake_api.c

source/blender/render/CMakeLists.txt

source/blender/render/RE_bake.h

source/blender/render/RE_texture_margin.h

source/blender/render/intern/bake.c

source/blender/render/intern/texture_margin.cc

Start of implementation of new texture margin generation.ClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 45257

source/blender/editors/object/object_bake_api.c

source/blender/render/CMakeLists.txt

source/blender/render/RE_bake.h

source/blender/render/RE_texture_margin.h

source/blender/render/intern/bake.c

source/blender/render/intern/texture_margin.cc

Start of implementation of new texture margin generation.
ClosedPublic
Actions

Revision Contents
Changeset List