Ah, I completely missed your comment and the code that explained that. Might be worth a small comment in the ensure_cache
function header or on the variable inside SculptSession
I'll make a separate PR for extracting this
Minor styling question - do we have a standardized way of writing struct initialization? For SubdivCCGCoord
in general I think I've seen this, struct member assignment, and the named initializer list.
Related to the above comment, I think if we keep this method as returning only integers, it makes more sense keep it returning -1
as a not found indicator instead of changing this.
With this change, the updated code does crash in non-debug builds if topology_island_cache
hasn't been set. Probably safer to still exit early with -1
or change this signature to std::optional<int>
Being a bit nitpicky here, but maybe this BLI change should be a separate PR? I don't think it'll have odd side effects, but it seems better to isolate it.