Skip to content
Snippets Groups Projects

WIP: Feature bbox improvements

Closed Mark OLESEN requested to merge feature-bbox-improvements into develop

Merge request reports

Closed by avatar (Jul 26, 2025 6:42am UTC)

Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
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
    • Have you been careful with the distinction between VGREAT (point::max) and GREAT? VGREAT is the exact greatest number and e.g. if you use it in a magEqOp() it will bomb out.

    • Author Maintainer

      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 respective invertedBox.

      Edited by Mark OLESEN
  • 196 208 }
    197 209
    198 210
    211 // * * * * * * * * * * * * * * * Member Operators * * * * * * * * * * * * * //
    212
    213 inline void Foam::boundBox::operator|=(const boundBox& bb)
    214 {
  • 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.

    • Author Maintainer

      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
  • Andrew Heather Marked this merge request as a Work In Progress

    Marked this merge request as a Work In Progress

  • Mark OLESEN Status changed to closed

    Status changed to closed

  • Author Maintainer

    To be re-opened after the 1612 release.

  • Please register or sign in to reply
    Loading