Skip to content

Commit

Permalink
2754: Revert Entity::bounds() to returning the definition bounds. (#2761
Browse files Browse the repository at this point in the history 
)

* 2754: Revert Entity::bounds() to returning the definition bounds.

Add a new Node::cullingBounds() that includes the model bounds.

Fixes #2754

* 2754: fix initializer order

* 2754: fix a few bounds() calls that should be cullingBounds()

* 2754: add docs

* 2754: ComputeNodeBoundsVisitor: use bbox::builder

* 2754: don't add Group to node tree

* 2754: cullingBounds->physicalBounds refactor

* 2754: Node::bounds->logicalBounds refactor

* 2754: more bounds refactoring

* 2754: rename *BoundsDidChange functions to *PhysicalBoundsDidChange

* 2754: fix comment

* 2754: minor naming adjustments / comments
  • Loading branch information
@ericwa
ericwa committed May 16, 2019
1 parent c8e2b5f commit 649b9b6
Show file tree
Hide file tree
Showing 36 changed files with 337 additions and 204 deletions.
2 changes: 1 addition & 1 deletion benchmark/src/AABBTreeBenchmark.cpp
View file
Expand Up @@ -57,7 +57,7 @@ namespace TrenchBroom {
}

void doInsert(Model::Node* node) {
m_tree.insert(node->bounds(), node);
m_tree.insert(node->physicalBounds(), node);
}
};

Expand Down
8 changes: 4 additions & 4 deletions common/src/Model/AttributableNode.cpp
View file
Expand Up @@ -270,20 +270,20 @@ namespace TrenchBroom {
AttributableNode::NotifyAttributeChange::NotifyAttributeChange(AttributableNode* node) :
m_nodeChange(node),
m_node(node),
m_oldBounds(node->bounds()) {
m_oldPhysicalBounds(node->physicalBounds()) {
ensure(m_node != nullptr, "node is null");
m_node->attributesWillChange();
}

AttributableNode::NotifyAttributeChange::~NotifyAttributeChange() {
m_node->attributesDidChange(m_oldBounds);
m_node->attributesDidChange(m_oldPhysicalBounds);
}

void AttributableNode::attributesWillChange() {}

void AttributableNode::attributesDidChange(const vm::bbox3& oldBounds) {
void AttributableNode::attributesDidChange(const vm::bbox3& oldPhysicalBounds) {
updateClassname();
doAttributesDidChange(oldBounds);
doAttributesDidChange(oldPhysicalBounds);
}

void AttributableNode::updateClassname() {
Expand Down
4 changes: 2 additions & 2 deletions common/src/Model/AttributableNode.h
View file
Expand Up @@ -116,14 +116,14 @@ namespace TrenchBroom {
private:
NotifyNodeChange m_nodeChange;
AttributableNode* m_node;
vm::bbox3 m_oldBounds;
vm::bbox3 m_oldPhysicalBounds;
public:
NotifyAttributeChange(AttributableNode* node);
~NotifyAttributeChange();
};

void attributesWillChange();
void attributesDidChange(const vm::bbox3& oldBounds);
void attributesDidChange(const vm::bbox3& oldPhysicalBounds);

void updateClassname();
private: // search index management
Expand Down
6 changes: 3 additions & 3 deletions common/src/Model/BoundsContainsNodeVisitor.cpp
View file
Expand Up @@ -29,8 +29,8 @@ namespace TrenchBroom {

void BoundsContainsNodeVisitor::doVisit(const World* world) { setResult(false); }
void BoundsContainsNodeVisitor::doVisit(const Layer* layer) { setResult(false); }
void BoundsContainsNodeVisitor::doVisit(const Group* group) { setResult(m_bounds.contains(group->bounds())); }
void BoundsContainsNodeVisitor::doVisit(const Entity* entity) { setResult(m_bounds.contains(entity->bounds())); }
void BoundsContainsNodeVisitor::doVisit(const Brush* brush) { setResult(m_bounds.contains(brush->bounds())); }
void BoundsContainsNodeVisitor::doVisit(const Group* group) { setResult(m_bounds.contains(group->logicalBounds())); }
void BoundsContainsNodeVisitor::doVisit(const Entity* entity) { setResult(m_bounds.contains(entity->logicalBounds())); }
void BoundsContainsNodeVisitor::doVisit(const Brush* brush) { setResult(m_bounds.contains(brush->logicalBounds())); }
}
}
4 changes: 2 additions & 2 deletions common/src/Model/BoundsIntersectsNodeVisitor.cpp
View file
Expand Up @@ -30,8 +30,8 @@ namespace TrenchBroom {

void BoundsIntersectsNodeVisitor::doVisit(const World* world) { setResult(false); }
void BoundsIntersectsNodeVisitor::doVisit(const Layer* layer) { setResult(false); }
void BoundsIntersectsNodeVisitor::doVisit(const Group* group) { setResult(m_bounds.intersects(group->bounds())); }
void BoundsIntersectsNodeVisitor::doVisit(const Entity* entity) { setResult(m_bounds.intersects(entity->bounds())); }
void BoundsIntersectsNodeVisitor::doVisit(const Group* group) { setResult(m_bounds.intersects(group->logicalBounds())); }
void BoundsIntersectsNodeVisitor::doVisit(const Entity* entity) { setResult(m_bounds.intersects(entity->logicalBounds())); }
void BoundsIntersectsNodeVisitor::doVisit(const Brush* brush) {
for (const BrushVertex* vertex : brush->vertices()) {
if (m_bounds.contains(vertex->position())) {
Expand Down
32 changes: 18 additions & 14 deletions common/src/Model/Brush.cpp
View file
Expand Up @@ -406,15 +406,15 @@ namespace TrenchBroom {
void Brush::setFaces(const vm::bbox3& worldBounds, const BrushFaceList& faces) {
const NotifyNodeChange nodeChange(this);

const vm::bbox3 oldBounds = bounds();
const vm::bbox3 oldBounds = physicalBounds();
deleteGeometry();

detachFaces(m_faces);
VectorUtils::clearAndDelete(m_faces);
addFaces(faces);

buildGeometry(worldBounds);
nodeBoundsDidChange(oldBounds);
nodePhysicalBoundsDidChange(oldBounds);
}

bool Brush::closed() const {
Expand Down Expand Up @@ -552,7 +552,7 @@ namespace TrenchBroom {

try {
const auto testBrush = Brush(worldBounds, testFaces);
const auto inWorldBounds = worldBounds.contains(testBrush.bounds());
const auto inWorldBounds = worldBounds.contains(testBrush.logicalBounds());
const auto closed = testBrush.closed();
const auto allFaces = testBrush.faceCount() == testFaces.size();

Expand Down Expand Up @@ -685,7 +685,7 @@ namespace TrenchBroom {
}

bool Brush::containsPoint(const vm::vec3& point) const {
if (!bounds().contains(point)) {
if (!logicalBounds().contains(point)) {
return false;
} else {
for (const auto* face : m_faces) {
Expand Down Expand Up @@ -1273,10 +1273,10 @@ namespace TrenchBroom {
}

void Brush::rebuildGeometry(const vm::bbox3& worldBounds) {
const vm::bbox3 oldBounds = bounds();
const vm::bbox3 oldBounds = physicalBounds();
deleteGeometry();
buildGeometry(worldBounds);
nodeBoundsDidChange(oldBounds);
nodePhysicalBoundsDidChange(oldBounds);
}

void Brush::buildGeometry(const vm::bbox3& worldBounds) {
Expand Down Expand Up @@ -1343,11 +1343,15 @@ namespace TrenchBroom {
return name;
}

const vm::bbox3& Brush::doGetBounds() const {
const vm::bbox3& Brush::doGetLogicalBounds() const {
ensure(m_geometry != nullptr, "geometry is null");
return m_geometry->bounds();
}

const vm::bbox3& Brush::doGetPhysicalBounds() const {
return logicalBounds();
}

Node* Brush::doClone(const vm::bbox3& worldBounds) const {
BrushFaceList faceClones;
faceClones.reserve(m_faces.size());
Expand Down Expand Up @@ -1413,7 +1417,7 @@ namespace TrenchBroom {
Brush::BrushFaceHit::BrushFaceHit(BrushFace* i_face, const FloatType i_distance) : face(i_face), distance(i_distance) {}

Brush::BrushFaceHit Brush::findFaceHit(const vm::ray3& ray) const {
if (vm::isnan(vm::intersectRayAndBBox(ray, bounds()))) {
if (vm::isnan(vm::intersectRayAndBBox(ray, logicalBounds()))) {
return BrushFaceHit();
}

Expand Down Expand Up @@ -1463,12 +1467,12 @@ namespace TrenchBroom {
private:
void doVisit(const World* world) override { setResult(false); }
void doVisit(const Layer* layer) override { setResult(false); }
void doVisit(const Group* group) override { setResult(contains(group->bounds())); }
void doVisit(const Entity* entity) override { setResult(contains(entity->bounds())); }
void doVisit(const Group* group) override { setResult(contains(group->logicalBounds())); }
void doVisit(const Entity* entity) override { setResult(contains(entity->logicalBounds())); }
void doVisit(const Brush* brush) override { setResult(contains(brush)); }

bool contains(const vm::bbox3& bounds) const {
if (m_this->bounds().contains(bounds)) {
if (m_this->logicalBounds().contains(bounds)) {
return true;
}

Expand Down Expand Up @@ -1502,12 +1506,12 @@ namespace TrenchBroom {
private:
void doVisit(const World* world) override { setResult(false); }
void doVisit(const Layer* layer) override { setResult(false); }
void doVisit(const Group* group) override { setResult(intersects(group->bounds())); }
void doVisit(const Entity* entity) override { setResult(intersects(entity->bounds())); }
void doVisit(const Group* group) override { setResult(intersects(group->logicalBounds())); }
void doVisit(const Entity* entity) override { setResult(intersects(entity->logicalBounds())); }
void doVisit(const Brush* brush) override { setResult(intersects(brush)); }

bool intersects(const vm::bbox3& bounds) const {
return m_this->bounds().intersects(bounds);
return m_this->logicalBounds().intersects(bounds);
}

bool intersects(const Brush* brush) {
Expand Down
3 changes: 2 additions & 1 deletion common/src/Model/Brush.h
View file
Expand Up @@ -281,7 +281,8 @@ namespace TrenchBroom {
void findIntegerPlanePoints(const vm::bbox3& worldBounds);
private: // implement Node interface
const String& doGetName() const override;
const vm::bbox3& doGetBounds() const override;
const vm::bbox3& doGetLogicalBounds() const override;
const vm::bbox3& doGetPhysicalBounds() const override;

Node* doClone(const vm::bbox3& worldBounds) const override;
NodeSnapshot* doTakeSnapshot() override;
Expand Down
42 changes: 27 additions & 15 deletions common/src/Model/ComputeNodeBoundsVisitor.cpp
View file
Expand Up @@ -25,40 +25,52 @@

namespace TrenchBroom {
namespace Model {
ComputeNodeBoundsVisitor::ComputeNodeBoundsVisitor(const vm::bbox3& defaultBounds) :
ComputeNodeBoundsVisitor::ComputeNodeBoundsVisitor(const BoundsType type, const vm::bbox3& defaultBounds) :
m_initialized(false),
m_bounds(defaultBounds) {}
m_boundsType(type),
m_defaultBounds(defaultBounds) {}

const vm::bbox3& ComputeNodeBoundsVisitor::bounds() const {
return m_bounds;
if (m_builder.initialized()) {
return m_builder.bounds();
} else {
return m_defaultBounds;
}
}

void ComputeNodeBoundsVisitor::doVisit(const World* world) {}
void ComputeNodeBoundsVisitor::doVisit(const Layer* layer) {}

void ComputeNodeBoundsVisitor::doVisit(const Group* group) {
mergeWith(group->bounds());
if (m_boundsType == BoundsType::Physical) {
m_builder.add(group->physicalBounds());
} else {
m_builder.add(group->logicalBounds());
}
}

void ComputeNodeBoundsVisitor::doVisit(const Entity* entity) {
mergeWith(entity->bounds());
if (m_boundsType == BoundsType::Physical) {
m_builder.add(entity->physicalBounds());
} else {
m_builder.add(entity->logicalBounds());
}
}

void ComputeNodeBoundsVisitor::doVisit(const Brush* brush) {
mergeWith(brush->bounds());
}

void ComputeNodeBoundsVisitor::mergeWith(const vm::bbox3& bounds) {
if (!m_initialized) {
m_bounds = bounds;
m_initialized = true;
if (m_boundsType == BoundsType::Physical) {
m_builder.add(brush->physicalBounds());
} else {
m_bounds = merge(m_bounds, bounds);
m_builder.add(brush->logicalBounds());
}
}

vm::bbox3 computeBounds(const Model::NodeList& nodes) {
return computeBounds(std::begin(nodes), std::end(nodes));
vm::bbox3 computeLogicalBounds(const Model::NodeList& nodes) {
return computeLogicalBounds(std::begin(nodes), std::end(nodes));
}

vm::bbox3 computePhysicalBounds(const Model::NodeList& nodes) {
return computePhysicalBounds(std::begin(nodes), std::end(nodes));
}
}
}
32 changes: 27 additions & 5 deletions common/src/Model/ComputeNodeBoundsVisitor.h
View file
Expand Up @@ -28,12 +28,25 @@

namespace TrenchBroom {
namespace Model {
enum class BoundsType {
/**
* See Node::logicalBounds()
*/
Logical,
/**
* See Node::physicalBounds()
*/
Physical
};

class ComputeNodeBoundsVisitor : public ConstNodeVisitor {
private:
bool m_initialized;
BoundsType m_boundsType;
vm::bbox3 m_defaultBounds;
vm::bbox3::builder m_builder;
public:
vm::bbox3 m_bounds;
explicit ComputeNodeBoundsVisitor(const vm::bbox3& defaultBounds = vm::bbox3());
explicit ComputeNodeBoundsVisitor(BoundsType type, const vm::bbox3& defaultBounds = vm::bbox3());
const vm::bbox3& bounds() const;
private:
void doVisit(const World* world) override;
Expand All @@ -44,11 +57,20 @@ namespace TrenchBroom {
void mergeWith(const vm::bbox3& bounds);
};

vm::bbox3 computeBounds(const Model::NodeList& nodes);
vm::bbox3 computeLogicalBounds(const Model::NodeList& nodes);

template <typename I>
vm::bbox3 computeLogicalBounds(I cur, I end) {
auto visitor = ComputeNodeBoundsVisitor(BoundsType::Logical);
Node::accept(cur, end, visitor);
return visitor.bounds();
}

vm::bbox3 computePhysicalBounds(const Model::NodeList& nodes);

template <typename I>
vm::bbox3 computeBounds(I cur, I end) {
ComputeNodeBoundsVisitor visitor;
vm::bbox3 computePhysicalBounds(I cur, I end) {
auto visitor = ComputeNodeBoundsVisitor(BoundsType::Physical);
Node::accept(cur, end, visitor);
return visitor.bounds();
}
Expand Down

0 comments on commit 649b9b6

Please sign in to comment.