Skip to content

Commit

Permalink
Replace usage of Vector3::isEqual with math::near
Browse files Browse the repository at this point in the history
  • Loading branch information
Matthew Mott committed Apr 10, 2021
1 parent 6a841b4 commit 257bfc0
Show file tree
Hide file tree
Showing 11 changed files with 50 additions and 57 deletions.
2 changes: 1 addition & 1 deletion libs/math/Plane3.h
View file
Expand Up @@ -81,7 +81,7 @@ class Plane3
*/
bool operator== (const Plane3& other) const
{
return _normal.isEqual(other._normal, EPSILON_NORMAL) &&
return math::near(_normal, other._normal, EPSILON_NORMAL) &&
float_equal_epsilon(_dist, other._dist, EPSILON_DIST);
}

Expand Down
9 changes: 0 additions & 9 deletions libs/math/Vector3.h
View file
Expand Up @@ -223,15 +223,6 @@ class BasicVector3
return (*this + other) * 0.5f;
}

// Returns true if this vector is equal to the other one, considering the given tolerance.
template<typename OtherElement, typename Epsilon>
bool isEqual(const BasicVector3<OtherElement>& other, Epsilon epsilon) const
{
return float_equal_epsilon(x(), other.x(), epsilon) &&
float_equal_epsilon(y(), other.y(), epsilon) &&
float_equal_epsilon(z(), other.z(), epsilon);
}

/**
* Returns a "snapped" copy of this Vector, each component rounded to integers.
*/
Expand Down
4 changes: 2 additions & 2 deletions libs/texturelib.h
View file
Expand Up @@ -220,12 +220,12 @@ inline void ComputeAxisBase(const Vector3& normal, Vector3& texS, Vector3& texT)
const Vector3 up(0, 0, 1);
const Vector3 down(0, 0, -1);

if (normal.isEqual(up, 1e-6))
if (math::near(normal, up, 1e-6))
{
texS = Vector3(0, 1, 0);
texT = Vector3(1, 0, 0);
}
else if (normal.isEqual(down, 1e-6))
else if (math::near(normal, down, 1e-6))
{
texS = Vector3(0, 1, 0);
texT = Vector3(-1, 0, 0);
Expand Down
10 changes: 5 additions & 5 deletions radiantcore/brush/Brush.h
View file
Expand Up @@ -16,7 +16,7 @@ class Ray;
/// \brief Returns true if 'self' takes priority when building brush b-rep.
inline bool plane3_inside(const Plane3& self, const Plane3& other)
{
if (self.normal().isEqual(other.normal(), 0.001))
if (math::near(self.normal(), other.normal(), 0.001))
{
return self.dist() < other.dist();
}
Expand Down Expand Up @@ -125,15 +125,15 @@ class Brush :
// ----

DetailFlag _detailFlag;

public:
/// \brief The undo memento for a brush stores only the list of face references - the faces are not copied.
class BrushUndoMemento :
class BrushUndoMemento :
public IUndoMemento
{
public:
BrushUndoMemento(const Faces& faces, DetailFlag detailFlag) :
_faces(faces),
BrushUndoMemento(const Faces& faces, DetailFlag detailFlag) :
_faces(faces),
_detailFlag(detailFlag)
{}

Expand Down
10 changes: 5 additions & 5 deletions radiantcore/brush/Face.cpp
View file
Expand Up @@ -252,7 +252,7 @@ void Face::translate(const Vector3& translation)
{
if (GlobalBrush().textureLockEnabled())
{
m_texdefTransformed.transformLocked(_shader.getWidth(), _shader.getHeight(),
m_texdefTransformed.transformLocked(_shader.getWidth(), _shader.getHeight(),
m_plane.getPlane(), Matrix4::getTranslation(translation));
}

Expand All @@ -263,7 +263,7 @@ void Face::translate(const Vector3& translation)

void Face::transform(const Matrix4& matrix)
{
if (GlobalBrush().textureLockEnabled())
if (GlobalBrush().textureLockEnabled())
{
m_texdefTransformed.transformLocked(_shader.getWidth(), _shader.getHeight(), m_plane.getPlane(), matrix);
}
Expand Down Expand Up @@ -424,7 +424,7 @@ void Face::SetTexdef(const TextureProjection& projection)
void Face::setTexdef(const TexDef& texDef)
{
TextureProjection projection;

// Construct the BPTexDef out of the TexDef by using the according constructor
projection.matrix = TextureMatrix(texDef);

Expand All @@ -446,7 +446,7 @@ ShiftScaleRotation Face::getShiftScaleRotation()
{
TextureProjection curProjection = _texdef;

// Multiply the texture dimensions to the projection matrix such that
// Multiply the texture dimensions to the projection matrix such that
// the shift/scale/rotation represent pixel values within the image.
Vector2 shaderDims(_shader.getWidth(), _shader.getHeight());

Expand Down Expand Up @@ -486,7 +486,7 @@ void Face::applyShaderFromFace(const Face& other)
for (Winding::const_iterator i = other.m_winding.begin(); i != other.m_winding.end(); ++i) {
for (Winding::const_iterator j = m_winding.begin(); j != m_winding.end(); ++j) {
// Check if the vertices are matching
if (j->vertex.isEqual(i->vertex, 0.001))
if (math::near(j->vertex, i->vertex, 0.001))
{
// Match found, add to list
thisVerts.push_back(j);
Expand Down
2 changes: 1 addition & 1 deletion radiantcore/patch/Patch.cpp
View file
Expand Up @@ -506,7 +506,7 @@ bool Patch::isDegenerate() const {
for (PatchControlConstIter i = _ctrl.begin(); i != _ctrl.end(); ++i) {

// Skip the first comparison
if (i != _ctrl.begin() && !i->vertex.isEqual(prev, 0.0001)) {
if (i != _ctrl.begin() && !math::near(i->vertex, prev, 0.0001)) {
return false;
}

Expand Down
16 changes: 8 additions & 8 deletions radiantcore/patch/algorithm/General.cpp
View file
Expand Up @@ -26,7 +26,7 @@ namespace algorithm
void thicken(const PatchNodePtr& sourcePatch, float thickness, bool createSeams, int axis)
{
if (axis < 0 || axis > 3) throw cmd::ExecutionFailure(fmt::format(_("Invalid axis value: {0}"), string::to_string(axis)));

// Get a shortcut to the patchcreator
auto& patchCreator = GlobalPatchModule();

Expand Down Expand Up @@ -112,7 +112,7 @@ void stitchTextures(const cmd::ArgumentList& args)
// Stitch the texture leaving the source patch intact
target->stitchTextureFrom(*source);
}
else
else
{
throw cmd::ExecutionFailure(_("Cannot stitch textures. \nCould not cast nodes to patches."));
}
Expand Down Expand Up @@ -171,7 +171,7 @@ namespace
constexpr double WELD_EPSILON = 0.001;

// Returns true if <num> elements in the given sequences are equal
inline bool firstNItemsAreEqual(const PatchControlIterator& sequence1,
inline bool firstNItemsAreEqual(const PatchControlIterator& sequence1,
const PatchControlIterator& sequence2, std::size_t num, double epsilon)
{
// If the iterators are invalid from the start, return false
Expand All @@ -185,7 +185,7 @@ inline bool firstNItemsAreEqual(const PatchControlIterator& sequence1,

for (std::size_t n = 0; n < num && p1.isValid() && p2.isValid(); ++n, ++p1, ++p2)
{
if (!p1->vertex.isEqual(p2->vertex, epsilon))
if (!math::near(p1->vertex, p2->vertex, epsilon))
{
return false;
}
Expand Down Expand Up @@ -226,7 +226,7 @@ inline bool meshesAreFacingOppositeDirections(const PatchMesh& mesh1, const Patc
{
for (const auto& v2 : mesh2.vertices)
{
if (v1.vertex.isEqual(v2.vertex, 0.01))
if (math::near(v1.vertex, v2.vertex, 0.01))
{
return std::abs(v1.normal.angle(v2.normal)) > c_half_pi;
}
Expand Down Expand Up @@ -271,15 +271,15 @@ scene::INodePtr createdMergedPatch(const PatchNodePtr& patchNode1, const PatchNo
{ ColumnWisePatchIterator(patch2), patch2.getHeight(), EdgeType::Column },
{ RowWisePatchIterator(patch2, patch2.getHeight() - 1, 0), patch2.getWidth(), EdgeType::Row },
{ ColumnWisePatchIterator(patch2, patch2.getWidth() - 1, 0), patch2.getHeight(), EdgeType::Column },

{ RowWisePatchReverseIterator(patch2), patch2.getWidth(), EdgeType::Row },
{ ColumnWisePatchReverseIterator(patch2), patch2.getHeight(), EdgeType::Column },
{ RowWisePatchReverseIterator(patch2, patch2.getHeight() - 1, 0), patch2.getWidth(), EdgeType::Row },
{ ColumnWisePatchReverseIterator(patch2, patch2.getWidth() - 1, 0), patch2.getHeight(), EdgeType::Column }
};

// As soon as we've found a matching edge, we know exactly how the resulting merged patch
// should look like. We know the dimensions and we know whether we need to expand the
// should look like. We know the dimensions and we know whether we need to expand the
// patch row-wise or column-wise.
for (const auto& p1Iter : patch1Edges)
{
Expand Down Expand Up @@ -397,7 +397,7 @@ void weldPatchPool()
for (auto p2 = p1 + 1; p2 != pair.second.end(); ++p2)
{
if (!(*p2)->getParent()) continue;// patch has been merged already

try
{
weldPatches(*p1, *p2);
Expand Down
6 changes: 3 additions & 3 deletions radiantcore/selection/manipulators/RotateManipulator.cpp
View file
Expand Up @@ -96,7 +96,7 @@ void RotateManipulator::updateCircleTransforms()
_pivot2World._worldSpace.getTransposed().transformDirection(_pivot2World._viewpointSpace.zCol().getVector3())
);

_circleX_visible = !g_vector3_axis_x.isEqual(localViewpoint, 1e-6);
_circleX_visible = !math::near(g_vector3_axis_x, localViewpoint, 1e-6);
if(_circleX_visible)
{
_local2worldX = Matrix4::getIdentity();
Expand All @@ -106,7 +106,7 @@ void RotateManipulator::updateCircleTransforms()
_local2worldX.premultiplyBy(_pivot2World._worldSpace);
}

_circleY_visible = !g_vector3_axis_y.isEqual(localViewpoint, 1e-6);
_circleY_visible = !math::near(g_vector3_axis_y, localViewpoint, 1e-6);
if(_circleY_visible)
{
_local2worldY = Matrix4::getIdentity();
Expand All @@ -116,7 +116,7 @@ void RotateManipulator::updateCircleTransforms()
_local2worldY.premultiplyBy(_pivot2World._worldSpace);
}

_circleZ_visible = !g_vector3_axis_z.isEqual(localViewpoint, 1e-6);
_circleZ_visible = !math::near(g_vector3_axis_z, localViewpoint, 1e-6);
if(_circleZ_visible)
{
_local2worldZ = Matrix4::getIdentity();
Expand Down
34 changes: 17 additions & 17 deletions test/PatchWelding.cpp
View file
Expand Up @@ -32,7 +32,7 @@ inline void compareNormalOfFirstSharedVertex(const PatchMesh& mesh1, const Patch
{
for (const auto& v2 : mesh2.vertices)
{
if (v1.vertex.isEqual(v2.vertex, 0.01))
if (math::near(v1.vertex, v2.vertex, 0.01))
{
EXPECT_LT(std::abs(v1.normal.angle(v2.normal)), c_half_pi);
return;
Expand Down Expand Up @@ -114,10 +114,10 @@ TEST_P(PatchWelding3x3, WeldWithOther3x3Patch)
}

// Patch 1 is sharing its first row
INSTANTIATE_TEST_CASE_P(WeldPatch1WithOther3x3, PatchWelding3x3,
testing::Values(std::tuple{ "1", "2", 5, 3 },
std::tuple{ "1", "3", 5, 3 },
std::tuple{ "1", "4", 5, 3 },
INSTANTIATE_TEST_CASE_P(WeldPatch1WithOther3x3, PatchWelding3x3,
testing::Values(std::tuple{ "1", "2", 5, 3 },
std::tuple{ "1", "3", 5, 3 },
std::tuple{ "1", "4", 5, 3 },
std::tuple{ "1", "5", 5, 3 },
std::tuple{ "2", "1", 5, 3 },
std::tuple{ "3", "1", 5, 3 },
Expand All @@ -126,9 +126,9 @@ INSTANTIATE_TEST_CASE_P(WeldPatch1WithOther3x3, PatchWelding3x3,

// Patch 6 is sharing its last row
INSTANTIATE_TEST_CASE_P(WeldPatch6WithOther3x3, PatchWelding3x3,
testing::Values(std::tuple{ "6", "7", 5, 3 },
std::tuple{ "6", "8", 5, 3 },
std::tuple{ "6", "9", 5, 3 },
testing::Values(std::tuple{ "6", "7", 5, 3 },
std::tuple{ "6", "8", 5, 3 },
std::tuple{ "6", "9", 5, 3 },
std::tuple{ "6", "10", 5, 3 },
std::tuple{ "7", "6", 5, 3 },
std::tuple{ "8", "6", 5, 3 },
Expand All @@ -137,9 +137,9 @@ INSTANTIATE_TEST_CASE_P(WeldPatch6WithOther3x3, PatchWelding3x3,

// Patch 11 is sharing a row
INSTANTIATE_TEST_CASE_P(WeldPatch11WithOther3x3, PatchWelding3x3,
testing::Values(std::tuple{ "11", "12", 3, 5 },
std::tuple{ "11", "13", 3, 5 },
std::tuple{ "11", "14", 3, 5 },
testing::Values(std::tuple{ "11", "12", 3, 5 },
std::tuple{ "11", "13", 3, 5 },
std::tuple{ "11", "14", 3, 5 },
std::tuple{ "11", "15", 3, 5 },
std::tuple{ "12", "11", 5, 3 },
std::tuple{ "13", "11", 5, 3 },
Expand All @@ -148,16 +148,16 @@ INSTANTIATE_TEST_CASE_P(WeldPatch11WithOther3x3, PatchWelding3x3,

// Patch 16 is sharing a column
INSTANTIATE_TEST_CASE_P(WeldPatch16WithOther3x3, PatchWelding3x3,
testing::Values(std::tuple{ "16", "17", 3, 5 },
std::tuple{ "16", "18", 3, 5 },
std::tuple{ "16", "19", 3, 5 },
testing::Values(std::tuple{ "16", "17", 3, 5 },
std::tuple{ "16", "18", 3, 5 },
std::tuple{ "16", "19", 3, 5 },
std::tuple{ "16", "20", 3, 5 },
std::tuple{ "17", "16", 5, 3 },
std::tuple{ "18", "16", 5, 3 },
std::tuple{ "19", "16", 5, 3 },
std::tuple{ "20", "16", 5, 3 }));

// Patch 4 = worldspawn, Patch 7 = func_static
// Patch 4 = worldspawn, Patch 7 = func_static
TEST_F(PatchWeldingTest, TryToWeldPatchesOfDifferentParents)
{
loadMap("weld_patches.mapx");
Expand Down Expand Up @@ -223,7 +223,7 @@ TEST_F(PatchWeldingTest, WeldedPatchInheritsSelectionGroups)
// After merging we expect the merged patch to have the same groups as patch 1 had
auto mergedNode = performPatchWelding("1", "2");
auto mergedNodeSelectionGroup = std::dynamic_pointer_cast<IGroupSelectable>(firstPatch);

EXPECT_EQ(mergedNodeSelectionGroup->getGroupIds(), firstPatchGroups);
}

Expand Down Expand Up @@ -337,7 +337,7 @@ TEST_F(PatchWeldingTest, WeldingIsUndoable)
}

GlobalCommandSystem().executeCommand("WeldSelectedPatches");

// Patch 2 should be gone now
EXPECT_TRUE(findPatchWithNumber("1")); // this is the merged patch
EXPECT_FALSE(findPatchWithNumber("2"));
Expand Down
4 changes: 2 additions & 2 deletions test/Selection.cpp
View file
Expand Up @@ -137,15 +137,15 @@ TEST_F(SelectionTest, LightBoundsChangedAfterRadiusChange)

auto defaultBounds = string::convert<Vector3>(entity->getKeyValue("light_radius"));

EXPECT_TRUE(GlobalSelectionSystem().getWorkZone().bounds.getExtents().isEqual(defaultBounds, 0.01));
EXPECT_TRUE(math::near(GlobalSelectionSystem().getWorkZone().bounds.getExtents(), defaultBounds, 0.01));

// Modify just the light_radius spawnarg
entity->setKeyValue("light_radius", "30 20 10");

// The work zone should have adapted itself to the new bounds
// assuming that the LightNode recalculates its AABB
auto changedBounds = string::convert<Vector3>(entity->getKeyValue("light_radius"));
EXPECT_TRUE(GlobalSelectionSystem().getWorkZone().bounds.getExtents().isEqual(changedBounds, 0.01));
EXPECT_TRUE(math::near(GlobalSelectionSystem().getWorkZone().bounds.getExtents(), changedBounds, 0.01));
}

// #5484: Projected lights don't rotate around their origin anymore
Expand Down
10 changes: 6 additions & 4 deletions test/math/Vector.cpp
View file
Expand Up @@ -77,10 +77,12 @@ TEST(MathTest, VectorEpsilonComparison)
const Vector3 v(1, 8, 320);
const Vector3 increment(1e-8, 1e-7, 1e-8);

EXPECT_TRUE(v.isEqual(v, 1e-8));
EXPECT_TRUE(v.isEqual(v + increment, 1e-6));
EXPECT_FALSE(v.isEqual(v + increment, 1e-9));
EXPECT_TRUE(v.isEqual(v + Vector3(1000, 10000, 20000), 1e6));
EXPECT_TRUE(math::near(v, v, 1e-8));
EXPECT_TRUE(math::near(v, v + increment, 1e-6));
EXPECT_TRUE(math::near(v, v - increment, 1e-6));
EXPECT_FALSE(math::near(v, v + increment, 1e-9));
EXPECT_FALSE(math::near(v, v - increment, 1e-9));
EXPECT_TRUE(math::near(v, v + Vector3(1000, 10000, 20000), 1e6));
}

TEST(MathTest, NegateVector3)
Expand Down

0 comments on commit 257bfc0

Please sign in to comment.