Details
Diff Detail
- Repository
- rB Blender
Event Timeline
Thanks for splitting this out of the other patch (D14389 for reference).
Thinking about this again, I'm wondering if it makes more sense to move this to the geometry module instead of blenlib, since it's a higher level operation.
@Campbell Barton (campbellbarton) @Jacques Lucke (JacquesLucke) do either of you have an opinion between the two?
| source/blender/editors/uvedit/uvedit_parametrizer.c | ||
|---|---|---|
| 31 | This is a local macro, so it probably shouldn't have the BLI prefix. | |
Moving to BLI seems fine, since it doesn't depend on DNA/BKE.
It could be moved into the geometry module as @Hans Goudey (HooglyBoogly) suggests.
Unless there is an intention to depend on other geometry/BKE calls, I don't think it's needed.
Accepting with minor requests.
| source/blender/blenlib/BLI_uv_parametrizer.h | ||
|---|---|---|
| 22–31 ↗ | (On Diff #49700) | Use doxy groups: /** \name Chart Construction:
*
* - Dot points.
* \{ */
... code ...
/** \} */Dot points should also be proper sentences. |
| 29–30 ↗ | (On Diff #49700) | Are these meant to be indented? |
| source/blender/editors/uvedit/uvedit_parametrizer.c | ||
| 31 | RIght, since it's local to the file it can be left as-is. | |
As the maintainer of this code, I think it should be in the geometry module.
I think the dependencies are irrelevant, logically this is not a low level API but a modifier/operator implementation.
Is the code in the geometry module expected to be in the blender::geometry namespace? If I move this to a namespace, the code has to be compiled as C++, which in turn also means porting uvedit_unwrap_ops.c to C++. This is trivial, but a large change for essentially just moving some code around. Is this what you would like me to do?
I think it's best to keep it as C for now and move it to C++ as a separate step. Functions can use the GEO_ prefix.