This patch adds a new operator to convert a black and white image into
grease pencil strokes.
If the image is not B/W, an internal conversion is done.
For more details see T79877
Details
Details
Diff Detail
Diff Detail
- Repository
- rB Blender
- Branch
- temp-potrace-patch (branched from master)
- Build Status
Buildable 10305 Build 10305: arc lint + arc unit
Event Timeline
Comment Actions
@Ray Molenkamp (LazyDodo) When this patch will be accepted, the option WITH-POTRACEwould be enabled by default, no?
Comment Actions
That is correct, looking at T80818 all platforms have updated, so we can turn it on by default
Comment Actions
@Hans Goudey (HooglyBoogly) I set you as reviewer to recheck the code to verify I didn't write something totally wrong. The main part of the code is the connection with Potrace lib and what I did was adapt the code of some functions to Blender.
| source/blender/blenkernel/intern/gpencil.c | ||
|---|---|---|
| 2750 | Todo: Fix Typo error | |
Comment Actions
Here are some warnings I got before properly setting up potrace:
/home/hans/Documents/Blender-Git/blender/source/blender/editors/gpencil/gpencil_trace_ops.c:222:6: warning: no previous prototype for ‘GPENCIL_OT_trace_image’ [-Wmissing-prototypes]
222 | void GPENCIL_OT_trace_image(wmOperatorType *ot)
| ^~~~~~~~~~~~~~~~~~~~~~
/home/hans/Documents/Blender-Git/blender/source/blender/editors/gpencil/gpencil_trace_utils.c:191:61: error: pointer parameter 'offset' can be pointer to const [readability-non-const-parameter,-warnings-as-errors]
static void add_point(bGPDstroke *gps, float scale, int32_t offset[2], float x, float y)
^
constFor testing it, I think I must be missing something about how to use it, because it was always grayed out in the object menu. I tried adding an empty object, a blank and stroke grease pencil object. I had an image open in the image editor to the left of the 3D viewport
I guess the poll was always failing.
Other than that confusion just a few smaller comments, nothing totally wrong that I could see : )
| source/blender/editors/gpencil/gpencil_trace_ops.c | ||
|---|---|---|
| 127 | Maybe there should be a comment about why "640" is used here, it seems pretty arbitrary. | |
| 231 | Looks like you dropped a capital here | |
| 243 | This sounds more natural: frequently in a local -> frequently in the local | |
| source/blender/editors/gpencil/gpencil_trace_utils.c | ||
| 236–244 | This is all just personal style I guess, but I don't find these sorts of comments very helpful, especially the very simple ones, like "Main pointer" and "Thickness of the stroke" where the explanation is evident from the variable's name. Personally I would rather have a shorter comment explaining anything that's unexpected or not intuitive | |
| 262 | Small comment here, this variable name seems a bit odd. The r_ prefix should only be used for variables from the caller function, so I think something like new_idx makes more sense here. And since this variable is only used in the two if statements below, it can be declared inside each of them so it's not accessible afterwards. | |
| 291 | There's another magic number here, could use an explanation. | |
Comment Actions
Found an easy crash too: If you add the object that the operator adds as a target, instant crash.
Comment Actions
Looks great!
On second thought, I think the operator poll message should be phrased to say why the item is grayed out rather than as instructions for the user, so "No image empty selected" might be better.