Texture border/margin generation by copying pixels from the adjacent polygon

@brecht I’m trying to pass a margin_type from the UI to the baking code. Something I don’t understand is: in struct RenderData (in DNA_scene_types.h) there’s a block of members containing bake_mode,bake_filter,bake_flag etc. It’s marked as ‘bake render options’ in a comment. However slightly below there’s also a member ‘struct BakeData bake’ which containes more or less those same settings again. This is marked ‘cycles baking’ in a comment. Which is which? Do I need to add my margin_type param to both sets? :confounded: If you’re too busy now with the 3.0 release, no worries. It can wait. I’ll ping you again after the release.

edit: I found out I need them both. I’m still not sure why the same data is available twice in the RenderData struct. Backwards compatability? Historical cruft? Should I try to refactor it?

New question, probably for @brecht . I’m trying to add the new margin generation to multires baking as well, but could use some pointers in the right direction.

  • I assume I need to use ‘DerivedMesh *lores_dm’ to get the UV-data?
  • the MultiresBakeRender struct doesn’t contain a UV-layer name, like the normal BakeRender struct?

It’s for historical reasons. It might be nice to refactor once, but I would not change it as part of working on something like this, best to keep such refactoring separated.

Yes, you should be able to get equivalent arrays from the derivedmesh. Really this legacy and we want to convert this to Mesh but have not gotten around to it.

Not sure which UV layer name you are referring to in which structure (BakeRender does not exist). From what I remember we always bake to the active UV layer.

Yes, that’s certainly true.

Once this is done I’ll look into it, I’m starting to know my way around this part of the code (a bit).

Now it’s the follwing day I’m not sure myself either… I’ll look into it again later with fresh eyes. All I know is I was rather lost as soon as I started to look into the Multiresolution Baking stuff.

I updated the patch. It now supports multiresolution baking and calling the border generation as an operator (so you can add a border to an already existing texture).

Now that 3.0 is out I’d very much appreciate some feedback from @brecht or maybe @dfelinto on (especially) the code in source/blender/render/intern/texture_margin.cc . It would be nice to get this done before 3.1 I think.

problems remaining:

  • I added the operator to the image-editor menu, because that seems the most logical place. However, you still also need an active object selected for the algorithm to work. It currently uses the active object’s default uv map to generate a margin for the image shown in the image editor when called from the image editor menu, the margin size is taken from the baking panel. But ff you call the operator from python you can pass a margin size and an UV-layer name. It then goes through the material list of the active object and generates margins for the active image in each material (like it works when baking).

I don’t really like how this works right now, and would very much like some input from someone knowledgeable of UI stuff , maybe @julianeisel knows who to ask?
a menu?

  • the operator code (in uvedit_texturemargin.c) is hacked together by changing an existing operator, and I’m not really sure I did it right.
    Should I add the object and image as parameters? I’m not really sure how to do that.

  • I’m not sure about the naming. I called the original filter ‘extend’ and the new one ‘copypixels’. But those are more placeholder names.

  • I renamed bake_filter to bake_margin, but someone on the chat reminded me that those DNA properties are script-accesible and this can possibly break existing scripts. Maybe rename it back? having bake_filter together with bake_margin_type is bit strange though… can I ping @ideasman42 to give some input on this? Or anyone else from the API module?

meta-question: I think arc-diff squashes everything to a single diff, loosing all the commits contained in the feature branch? If that’s true, would it be a good idea to split this in a few parts to review separately? Can that be done, can I create an (arc) patch which sits on top of another arc patch?

edit: another meta question. I still need to write documentation for this, where to put that?

edit3: updated this post to reflect the last version of the patch.

3 Likes

I will try to review this for 3.1, but it will likely be in January as I’m going on holidays in a week from now and have many things to finish.

I don’t think we should add an operator for doing this on an existing image.

You can rename bake_filter to bake_margin in dna_rename_defs.h, then compatibility is preserved.

It is helpful to split the review in multiple parts. You can manually create dependencies between revisions. It’s a bit of a pain to do updates then, so if things are in flux a lot I would wait with that.

For documentation, you can add it in the code review description.

2 Likes

Ok, have a nice holiday. In preparation I’ll take out the operator for now. Though I do think it is very useful to have as an operator, that’s maybe best left for a separate diff.

That would make the reviewing job somewhat smaller as well.

I don’t understand exactly what you mean with this. But I’ll just write something locally and I can copy-paste it wherever :wink:

Should I ping you again sometime in januari? So you can just forget about this for now?

I uploaded a new version of the patch. Added the backwards compatibility rename and merged with current master. Also added a diff of the docs.

@brecht If you could have a look if you have time I would be grateful.

If it helps with the reviewing I could add ‘overkill mode’ comments describing exactly what happens everywhere. I’m generally not really a fan of adding too much comments because there’s always a risk the comments get out of sync with the actual code over time, but for reviewing it might be useful to spare you the time of too much context switching?

By the way: best wishes for 2022 and I hope you had a nice holiday.

8 Likes

Thanks a lot to Brecht & Hans for their feedback. I hope to have time this weekend to incorporate it.

It has landed in master! \o/

Hopefully this topic won’t turn into a topic to complain about the bugs :wink:

13 Likes