WIP: Feature bbox improvements
Merge request reports
Activity
920 920 { 921 921 forAll(bbs[procI], i) 922 922 { 923 reduce(bbs[procI][i].min(), minOp<point>()); 924 reduce(bbs[procI][i].max(), maxOp<point>()); 923 bbs[procI][i].reduce(); 244 244 subGeom_.setSize(surfI); 245 245 indexOffset_.setSize(surfI+1); 246 246 247 // Bounds is the overall bounds 248 bounds() = boundBox(point::max, point::min); 247 // Bounds is the overall bounds - prepare for min/max ops 248 bounds().invalidate(); 249 249 
I guess this is the reason for great vs. vgreat in treeboundbox vs boundbox. For an inverted box, probably no reason not to use great for both. The
invalidate()uses great for tree and vgreat for normal bbox (same as current implementation). In fact,invalidate()just uses the same values as the respectiveinvertedBox.Edited by Mark OLESEN
117 116 } 118 117 119 118 119 inline void Foam::boundBox::clear() 120 { I do not think that setting the boundBox to zero warrants a separate function.
I do not see what clear() in the context of a bounding box should do. The centre of the boundBox is still valid. Why clear to zero? What is it different from invertexBox? Also it will still 'hold' (0 0 0) so is not really empty. I would get rid of this function.
-
Partially agreed. Clear is not really empty, but in the current the null-constructor version implicitly contains a (0,0,0) too. I think it would be more consistent to always initialize with an invalid (inverted) box - even for the null constructor. This make much more sense to me. It would then be immediately possible to add a point to such a box and it would just work. It is were redefined like that, could have a single reset(), invalidate(), clear() to revert to an initial (inverted) bounding box state. With this type of change, clear() doesn't sound so bad again, and we can ditch invalidate() as being too obscure.
Edited by Mark OLESEN
