A big thing that strikes me here is the variable bool Node::Visible. There should not be such cache variable or stored data of a visible node. It might be the case for now that you make only single camera queries for rendering purposes, but later on you could have game logic-specific code making queries to the same acceleration structure, i.e. using the structure to do raycasting queries, or "give me objects inside this bounding sphere/box", and so on. It doesn't make sense to hardcode the Octree to contain state pertaining to the world camera.
Instead, I'd make a separate QueryResult object, which contains a vector/hash table of collected visible nodes. Then, when you are about to render:
QueryResult visibleObjects;
octree.FindVisibleObjects(cameraFrustum, &visibleObjects)
sort(visibleObjects, sceneObjectsSortPredicate);
for (object in visibleObjects)
render(object);
This collecting pass of scene objects allows you to sort, which an important optimization, i.e. it gives the ability to sort front-to-back for depth, and sort to group objects that use the same material/shader/texture, and so on.
At minimum, if you need to keep that boolean, strongly document the semantics that the bool Node::Visible refers to either the most recent query (effectively making it a cache for the most recent visibility query operation) or rename it to bool Node::VisibleToMainCamera; to signify that the boolean refers to whether the node is visible to the certain specific god camera that the player views the scene through.
Then, I'd replace Node *children[8]; with a Node *children, which I'd allocate with a children = new Node[8]; statement. Also, remove the IsLeaf boolean and replace it with a function IsLeaf() const { return children == nullptr; }
Also, Octree::AddObject does 9 calls to a generic IntersectAABBAABB function. That is a bit excessive, and instead I'd do something like:
Octree::AddObject(Node *node, Object *object, const AABB &objectAABB)
{
vec center = node->CenterPoint();
if (objectAABB.maxX < center.x)
{
// left half
if (objectAABB.maxY < center.y)
{
// top half
// test z ...
}
else if (objectAABB.minY > center.y)
{
// bottom half
// test z...
}
}
else if (objectAABB.minX > center.x)
{
// test y and z.
}
}
Octree::AddObject(Node *node, Object *object, const AABB &objectAABB)
{
return AddObject(node, object, ComputeObjectAABB(object));
}
this form makes one efficient AABB intersection test to place the object AABB in the proper octant, and manage the case where it straddles multiple octants. As was suggested above, there are different ways to handle this:
- support having objects up the tree, and not just in leaves. Objects will be placed in bottommost node they fully fit
- add to multiple children
- add to fixed child e.g. with the top-left rule that was presented above. Then when querying, see the top-left neighbors.
- make the octree what is known as a "loose octree" - there the neighboring octree nodes overlap in volume.
There is no single best way to handle the straddling issue, but they are a bit of "what works best" decision in my experience.