This is only a small detail, but: I'd prefer to only have the radius in the UI. Perform the division here, then only handle u/v_min/max in the rest of the Cycles code.
The padding is needed for alignment reasons, this is all over the DNA files.
I think these should be hard limits, not UI limits? Same goes for u_max and radius (that one doesn't need an upper hard limit, but it shouldn't be negative).
"this object" -> "the center of the objects in this collection"
Overall the functionality seems reasonable, but there's a bunch of code comments. Also, I think the collection needs to be handled in DepsgraphRelationBuilder::build_camera
as well.
Using FOREACH_COLLECTION_OBJECT_RECURSIVE_BEGIN
would be cleaner here, and would avoid recursing explicitly.
This file is already C++, so I'd prefer to use the new float3
consistently instead of C-style arrays.
Should be removed (same below).
Same as above, maybe just disable them.
There's a bunch of duplication here compared to the focus-on-object case. I think we could just have two branches to find pos
and then share the rest of the code?
No need to do this so explicitly, doing pos / count
below should automatically perform the cast and apply to all components.
pos += transform_get_column(&dofmat, 3);
should work fine here, no need for an intermediate variable or componentwise addition.
I think using coll.all_objects
here removes the need to recurse over child collections, so this could just be done in blender_camera_focal_distance
.
Instead of hiding the UI elements here, I think it would be better to disable (gray out) the unavailable ones.
Cycles also seems fine to me, but I don't know what the policy on compatibility is here. Personally, I think it's fine to just fix it.
I've implemented an initial version of this in !123278, no support for color picker yet.