Skip to content

Commit

Permalink
Fixed|BSP Builder: Space partitioner logic error resulting in mismatc…
Browse files Browse the repository at this point in the history
…hed sectors

When building line segments along the partitioning half-plane it is
imperative that the EdgeTip sets for the respective vertexes are
updated by inserting tips for the new line segments. Otherwise errors
will result if any of these segments are later split by subsequent
partitioning.

Closed sectors are no longer erroneously reported as "unclosed".
  • Loading branch information
danij-deng committed May 17, 2013
1 parent d1dc1d9 commit 265287f
Show file tree
Hide file tree
Showing 4 changed files with 97 additions and 43 deletions.
79 changes: 52 additions & 27 deletions doomsday/client/include/map/bsp/edgetip.h
Expand Up @@ -44,59 +44,66 @@ class EdgeTip
public:
enum Side
{
Front = 0,
Front,
Back
};

public:
explicit EdgeTip(coord_t angle = 0, LineSegment::Side *front = 0,
LineSegment::Side *back = 0)
explicit EdgeTip(coord_t angle = 0, LineSegment::Side *front = 0, LineSegment::Side *back = 0)
: _angle(angle), _front(front), _back(back)
{}

inline coord_t angle() const { return _angle; }

inline EdgeTip &setAngle(coord_t newAngle)
inline void setAngle(coord_t newAngle)
{
_angle = newAngle;
return *this;
}

inline LineSegment::Side &front() const { return *_front; }

inline LineSegment::Side &back() const { return *_back; }

inline LineSegment::Side &side(Side sid) const
bool hasSide(Side sid) const
{
return sid == Front? front() : back();
return sid == Front? _front != 0 : _back != 0;
}

inline bool hasFront() const { return _front != 0; }
inline bool hasFront() const { return hasSide(Front); }

inline bool hasBack() const { return _back != 0; }
inline bool hasBack() const { return hasSide(Back); }

inline bool hasSide(Side sid) const
LineSegment::Side &side(Side sid) const
{
return sid == Front? hasFront() : hasBack();
if(sid == Front)
{
DENG_ASSERT(_front != 0);
return *_front;
}
else
{
DENG_ASSERT(_back != 0);
return *_back;
}
}

inline EdgeTip &setFront(LineSegment::Side *lineSeg)
{
_front = lineSeg;
return *this;
}
inline LineSegment::Side &front() const { return side(Front); }
inline LineSegment::Side &back() const { return side(Back); }

inline EdgeTip &setBack(LineSegment::Side *lineSeg)
{
_back = lineSeg;
return *this;
}
inline LineSegment::Side *frontPtr() const { return hasFront()? &front() : 0; }
inline LineSegment::Side *backPtr() const { return hasBack()? &back() : 0; }

inline EdgeTip &setSide(Side sid, LineSegment::Side *lineSeg)
inline void setSide(Side sid, LineSegment::Side *lineSeg)
{
return sid == Front? setFront(lineSeg) : setBack(lineSeg);
if(sid == Front)
{
_front = lineSeg;
}
else
{
_back = lineSeg;
}
}

inline void setFront(LineSegment::Side *lineSeg) { setSide(Front, lineSeg); }
inline void setBack(LineSegment::Side *lineSeg) { setSide(Back, lineSeg); }

private:
/// Angle that line makes at vertex (degrees; 0 is E, 90 is N).
coord_t _angle;
Expand Down Expand Up @@ -137,6 +144,24 @@ class EdgeTips
return *_tips.insert(after.base(), EdgeTip(angle, front, back));
}

void clearByLineSegment(LineSegment &seg)
{
All::iterator i = _tips.begin();
while(i != _tips.end())
{
EdgeTip &tip = *i;
if(tip.hasFront() && &tip.front().line() == &seg ||
tip.hasBack() && &tip.back().line() == &seg)
{
i = _tips.erase(i);
}
else
{
++i;
}
}
}

All const &all() const { return _tips; }

private:
Expand Down
2 changes: 1 addition & 1 deletion doomsday/client/src/map/bsp/hplane.cpp
Expand Up @@ -235,7 +235,7 @@ HPlane::Intercept *HPlane::intercept(LineSegment::Side const &lineSeg, int edge,
d->intercepts.append(Intercept(distToVertex, const_cast<LineSegment::Side &>(lineSeg), edge));
Intercept *newIntercept = &d->intercepts.last();

newIntercept->selfRef = (lineSeg.hasMapSide() && lineSeg.mapLine().isSelfReferencing());
newIntercept->selfRef = lineSeg.sectorPtr() == lineSeg.back().sectorPtr(); //(lineSeg.hasMapSide() && lineSeg.mapLine().isSelfReferencing());

newIntercept->before = openSectorAtAngle(edgeTips, inverseAngle());
newIntercept->after = openSectorAtAngle(edgeTips, angle());
Expand Down
55 changes: 42 additions & 13 deletions doomsday/client/src/map/bsp/partitioner.cpp
Expand Up @@ -881,10 +881,6 @@ DENG2_PIMPL(Partitioner)

Vertex *newVert = newVertex(point);

// First, create tips for the new vertex.
edgeTips(*newVert).add(frontLeft.inverseAngle(), frontLeft.back().hasSector()? &frontLeft.back() : 0, &frontLeft);
edgeTips(*newVert).add(frontLeft.angle(), &frontLeft, frontLeft.back().hasSector()? &frontLeft.back() : 0);

LineSegment &newSegment = cloneLineSegment(frontLeft.line());

// Now perform the split, updating vertex and relative segment links.
Expand Down Expand Up @@ -930,6 +926,34 @@ DENG2_PIMPL(Partitioner)
}
}

/**
* @todo Optimize: Avoid clearing tips by implementing update logic.
*/
LineSegment &left = frontLeft.line();
LineSegment &right = frontRight.line();

edgeTips(left.from()).clearByLineSegment(left);
edgeTips(left.to() ).clearByLineSegment(left);

edgeTips(right.from()).clearByLineSegment(right);
edgeTips(right.to() ).clearByLineSegment(right);

edgeTips(left.from() ).add(left.front().angle(),
left.front().hasSector()? &left.front() : 0,
left.back().hasSector()? &left.back() : 0);

edgeTips(left.to() ).add(left.back().angle(),
left.back().hasSector()? &left.back() : 0,
left.front().hasSector()? &left.front() : 0);

edgeTips(right.from()).add(right.front().angle(),
right.front().hasSector()? &right.front() : 0,
right.back().hasSector()? &right.back() : 0);

edgeTips(right.to() ).add(right.back().angle(),
right.back().hasSector()? &right.back() : 0,
right.front().hasSector()? &right.front() : 0);

return frontRight;
}

Expand Down Expand Up @@ -1062,7 +1086,7 @@ DENG2_PIMPL(Partitioner)
* @param rights Set of line segments on the right side of the partition.
* @param lefts Set of line segments on the left side of the partition.
*/
void partitionLineSegmentSides(SuperBlock &lineSegs, SuperBlock &rights,
void partitionLineSegments(SuperBlock &lineSegs, SuperBlock &rights,
SuperBlock &lefts)
{
// Iterative pre-order traversal of SuperBlock.
Expand Down Expand Up @@ -1111,10 +1135,10 @@ DENG2_PIMPL(Partitioner)

// Sanity checks...
if(!rights.totalLineSegmentSideCount())
throw Error("Partitioner::partitionLineSegmentSides", "Right set is empty");
throw Error("Partitioner::partitionLineSegments", "Right set is empty");

if(!lefts.totalLineSegmentSideCount())
throw Error("Partitioner::partitionLineSegmentSides", "Left set is empty");
throw Error("Partitioner::partitionLineSegments", "Left set is empty");
}

/**
Expand All @@ -1125,7 +1149,7 @@ DENG2_PIMPL(Partitioner)
* @param rights Set of line segments on the right of the partition.
* @param lefts Set of line segments on the left of the partition.
*/
void addPartitionLineSegmentSides(SuperBlock &rights, SuperBlock &lefts)
void addPartitionLineSegments(SuperBlock &rights, SuperBlock &lefts)
{
LOG_TRACE("Building line segments along partition %s")
<< hplane.partition().asText();
Expand Down Expand Up @@ -1182,11 +1206,16 @@ DENG2_PIMPL(Partitioner)
sector = next.before;
}

DENG_ASSERT(sector != 0);

LineSegment *newSeg =
buildLineSegmentBetweenVertexes(cur.vertex(), next.vertex(),
sector, sector, 0 /*no line*/,
sector, sector, 0 /*no map line*/,
hplane.mapLine());

edgeTips(newSeg->from() ).add(newSeg->front().angle(), &newSeg->front(), &newSeg->back());
edgeTips(newSeg->to() ).add(newSeg->back().angle(), &newSeg->back(), &newSeg->front());

// Add each new line segment to the appropriate set.
linkLineSegmentSideInSuperBlockmap(rights, newSeg->front());
linkLineSegmentSideInSuperBlockmap(lefts, newSeg->back());
Expand Down Expand Up @@ -1396,9 +1425,9 @@ DENG2_PIMPL(Partitioner)
// Partition the line segements into two subsets according to their
// spacial relationship with the half-plane (splitting any which
// intersect).
partitionLineSegmentSides(lineSegs, rightLineSegs, leftLineSegs);
partitionLineSegments(lineSegs, rightLineSegs, leftLineSegs);
lineSegs.clear();
addPartitionLineSegmentSides(rightLineSegs, leftLineSegs);
addPartitionLineSegments(rightLineSegs, leftLineSegs);
hplane.clearIntercepts();

// Take a copy of the geometry bounds for each child/sub space
Expand Down Expand Up @@ -1688,8 +1717,8 @@ DENG2_PIMPL(Partitioner)
HEdge &hedge = *hedgeIt;
if(!hedge.hasTwin())
{
//DENG_ASSERT(&hedge.next() != &hedge);
//DENG_ASSERT(&hedge.next().vertex() != &hedge.vertex());
DENG_ASSERT(&hedge.next() != &hedge);
DENG_ASSERT(&hedge.next().vertex() != &hedge.vertex());

hedge._twin = new HEdge(hedge.next().vertex());
hedge._twin->_twin = &hedge;
Expand Down
4 changes: 2 additions & 2 deletions doomsday/client/src/map/bspbuilder.cpp
Expand Up @@ -144,7 +144,7 @@ class Reporter : DENG2_OBSERVES(Partitioner, UnclosedSectorFound),
<< hedge->lineSide().sector().indexInMap()
<< hedge->line().indexInMap();
else
LOG_WARNING("Sector #%d has migrant \"mini\" half-edge.")
LOG_WARNING("Sector #%d has migrant partition line half-edge.")
<< facingSector->indexInMap();
}

Expand All @@ -157,7 +157,7 @@ class Reporter : DENG2_OBSERVES(Partitioner, UnclosedSectorFound),
PartialBspLeafMap::const_iterator it = _partialBspLeafs.begin();
for(int i = 0; i < numToLog; ++i, ++it)
{
LOG_WARNING("Half-edge list for BSP leaf %p has %u gaps (%u hedges).")
LOG_WARNING("Half-edge list for BSP leaf %p has %u gaps (%i half-edges).")
<< de::dintptr(it->first) << it->second << it->first->hedgeCount();
}

Expand Down

0 comments on commit 265287f

Please sign in to comment.