Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add AxisAlignedBox::fromPoints #1114

Merged
merged 7 commits into from
Feb 25, 2025
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
- Added support for `3DTILES_ellipsoid` in `Cesium3DTiles`, `Cesium3DTilesReader`, and `Cesium3DTilesWriter`.
- Added support for `3DTILES_content_voxels` in `Cesium3DTiles`, `Cesium3DTilesReader`, and `Cesium3DTilesWriter`.
- Added generated classes for `EXT_primitive_voxels` and its dependencies in `CesiumGltf`, `CesiumGltfReader`, and `CesiumGltfWriter`.
- Added `AxisAlignedBox::fromPositions`, which creates an `AxisAlignedBox` from an input vector of positions.

##### Fixes :wrench:

Expand Down
29 changes: 28 additions & 1 deletion CesiumGeometry/include/CesiumGeometry/AxisAlignedBox.h
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
#pragma once

#include <CesiumGeometry/Library.h>
#include "Library.h"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We used to do this for headers in the same directory, but lately we've just been using the qualified name in angle brackets for consistency.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay! I thought that went against our style guide, but I'll revert it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be worth adding a section about #include style to the style guide? Especially since this is a change from SF.12.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reopened #991 noting this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see doing this for #includes in headers, but a small peeve in .cpp files is that the angle brackets cause the relevant .h to get mixed with the other includes. In other words, if I have "MyFile.h", doing <MyFile.h> in MyFile.cpp potentially mixes it with the other includes due to sorting alphabetically. It's not the biggest deal in the world but I guess the first option feels more "right" :')

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That might be fixable with .clang-format

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm maybe not: llvm/llvm-project#39735


#include <glm/common.hpp>
#include <glm/vec3.hpp>

#include <vector>

namespace CesiumGeometry {

/**
Expand Down Expand Up @@ -120,6 +123,30 @@ struct CESIUMGEOMETRY_API AxisAlignedBox final {
position.y >= this->minimumY && position.y <= this->maximumY &&
position.z >= this->minimumZ && position.z <= this->maximumZ;
}

/**
* @brief Creates a tight-fitting, axis-aligned bounding box that contains all
* of the input positions.
*
* @param positions The positions.
* @returns An axis-aligned bounding box derived from the input positions.
*/
static AxisAlignedBox
fromPositions(const std::vector<glm::dvec3>& positions) {
if (positions.size() == 0) {
return AxisAlignedBox();
}

glm::dvec3 min = positions[0];
glm::dvec3 max = positions[0];

for (size_t i = 1; i < positions.size(); i++) {
min = glm::min(min, positions[i]);
max = glm::max(max, positions[i]);
}

return AxisAlignedBox(min.x, min.y, min.z, max.x, max.y, max.z);
}
};

} // namespace CesiumGeometry
41 changes: 41 additions & 0 deletions CesiumGeometry/test/TestAxisAlignedBox.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,44 @@ TEST_CASE("AxisAlignedBox constructor") {
CHECK(aabb.lengthY == 5.0 - 2.0);
CHECK(aabb.lengthZ == 6.0 - 3.0);
}

TEST_CASE("AxisAlignedBox fromPositions") {
SUBCASE("returns default box for empty vector") {
AxisAlignedBox aabb = AxisAlignedBox::fromPositions({});

CHECK(aabb.minimumX == 0.0);
CHECK(aabb.minimumY == 0.0);
CHECK(aabb.minimumZ == 0.0);
CHECK(aabb.maximumX == 0.0);
CHECK(aabb.maximumY == 0.0);
CHECK(aabb.maximumZ == 0.0);
}

SUBCASE("works for single position") {
glm::dvec3 position(1.0, 2.0, 3.0);
AxisAlignedBox aabb = AxisAlignedBox::fromPositions({position});

CHECK(aabb.minimumX == position.x);
CHECK(aabb.minimumY == position.y);
CHECK(aabb.minimumZ == position.z);
CHECK(aabb.maximumX == position.x);
CHECK(aabb.maximumY == position.y);
CHECK(aabb.maximumZ == position.z);
}

SUBCASE("works for multiple positions") {
std::vector<glm::dvec3> positions{
glm::dvec3(1.0, 2.0, 3.0),
glm::dvec3(-2.0, 0.4, -10.0),
glm::dvec3(0.1, 4.3, 11.0),
glm::dvec3(0.5, 0.5, 2.7)};

AxisAlignedBox aabb = AxisAlignedBox::fromPositions(positions);
CHECK(aabb.minimumX == -2.0);
CHECK(aabb.minimumY == 0.4);
CHECK(aabb.minimumZ == -10.0);
CHECK(aabb.maximumX == 1.0);
CHECK(aabb.maximumY == 4.3);
CHECK(aabb.maximumZ == 11.0);
}
}
Loading