Implementation of T47438 (levels, bright/contrast, HSV, invert for vertex paint) as C operators.
Details
Details
Diff Detail
Diff Detail
- Repository
- rB Blender
Event Timeline
Comment Actions
Refactor operators into one file, as they all follow the same pattern: "set the color of each vertex in the active vertex layer to some function of the current color".
Comment Actions
Apologies for not stating this explicitly, but would prefer color operations be written in Blender's C code (updated task).
While color operations aren't too slow in Python, they can be near realtime, even on high-poly meshes when done in C.
Comment Actions
Okay. So unless otherwise stated, is it typically best practice to write new operators in C for performance reasons?
Comment Actions
Refactor ED_vpaint_color_transform to take a pointer to a color transformation function (separates the logic for looping over the mesh from the logic for changing a vertex's color).
| source/blender/editors/sculpt_paint/paint_vertex.c | ||
|---|---|---|
| 505–507 ↗ | (On Diff #6876) | Using varargs here is unnecessarily complicated, would rather each function has an associated struct which can be passed as a void * pointer (we normally call this void *user_data), this was as long as the right struct is paired up with the callback, different data types can be used (not just floats), and theres less chance of errors by some accidental missing argument. |
| 560–564 ↗ | (On Diff #6876) | Would rather this follow brightness/contrast of other areas of Blender. Best double check the code in the compositor's brightness/contrast node. |
| 583 ↗ | (On Diff #6876) | style (function braces on newlines), see: https://wiki.blender.org/index.php/Dev:Doc/Code_Style |
Comment Actions
Use void *user_data instead of varargs, put function braces on newlines, make bright/contrast and hue/sat/val functions identical to corresponding node operations
| source/blender/editors/sculpt_paint/paint_vertex.c | ||
|---|---|---|
| 560–564 ↗ | (On Diff #6908) | Is it okay that evaluating 1.0f / a leads to division by 0 when contrast equals 100? |
Comment Actions
Committed rB42e2398ae35da2b499d18ea589767108bd093687
Made some minor edits.
- Avoid divide by zero.
- Localize structs and color manipulation functions.
- Pre-calculate brigthtness/context offset+gain.
- Use const for user_data arg.
Comment Actions
Thanks for the code review. You may also want to apply the divide by zero fix to other usages of the bright/contrast algorithm (namely source/blender/blenkernel/intern/seqmodifier.c and source/blender/compositor/operations/COM_BrightnessOperation.cpp.