Skip to content

Commit 501695f

Browse files
nebraszkaprybicki
andauthored
Minor updates to comments, tests, and naming (#249)
* Minor updates to comments, tests, and naming - Fix documentation in core.h (rgl_node_t) - Ensure non-zero field values in testPointCloud == operator - Rename test from '...should_warn...' to '...should_work...' for accuracy * Review fixes Co-authored-by: Piotr Rybicki <[email protected]> --------- Co-authored-by: Piotr Rybicki <[email protected]>
1 parent 09dba15 commit 501695f

File tree

3 files changed

+42
-10
lines changed

3 files changed

+42
-10
lines changed

include/rgl/api/core.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,9 @@ typedef struct Entity* rgl_entity_t;
109109
typedef struct Texture* rgl_texture_t;
110110

111111
/**
112-
* TODO(prybicki)
112+
* Opaque handle for a computational graph node in RGL.
113+
* Represents a single computational step, e.g. introducing gaussian noise, raytracing or downsampling.
114+
* Nodes form a directed acyclic graph, dictating execution order.
113115
*/
114116
typedef struct Node* rgl_node_t;
115117

test/include/helpers/testPointCloud.hpp

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
#include <RGLFields.hpp>
1010
#include <math/Mat3x4f.hpp>
11+
#include <Logger.hpp>
1112

1213
inline std::vector<rgl_field_t>& getAllRealFieldsVector()
1314
{
@@ -188,7 +189,32 @@ class TestPointCloud
188189

189190
bool operator==(const TestPointCloud& other) const
190191
{
191-
return getPointCount() == other.getPointCount() && fields == other.fields && data == other.data;
192+
if (getPointCount() != other.getPointCount() || fields != other.fields || data != other.data) {
193+
return false;
194+
}
195+
196+
// If all values for a field (which is not a padding) are equal to zero then warn about this
197+
for (size_t fieldIndex = 0; fieldIndex < fields.size(); ++fieldIndex) {
198+
const auto& field = fields[fieldIndex];
199+
200+
if (isDummy(field)) {
201+
continue;
202+
}
203+
204+
int fieldOffset = offsets.at(fieldIndex);
205+
bool allZeros = true;
206+
for (int i = 0; i < getPointCount(); ++i) {
207+
if (std::memcmp(data.data() + i * getPointByteSize() + fieldOffset, std::vector<char>(getFieldSize(field), 0).data(),
208+
getFieldSize(field)) != 0) {
209+
allZeros = false;
210+
break;
211+
}
212+
}
213+
if (allZeros) {
214+
RGL_WARN(fmt::format("TestPointCloud::operator==: all field values are 0.0 for field {}", toString(field)));
215+
}
216+
}
217+
return true;
192218
}
193219

194220
template<rgl_field_t T>

test/src/graph/nodes/CompactByFieldPointsNodeTest.cpp

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,8 @@ class CompactByFieldPointsNodeTest : public RGLTestWithParam<int>
4141
}
4242
};
4343

44-
INSTANTIATE_TEST_SUITE_P(CompactByFieldPointsNodeTest, CompactByFieldPointsNodeTest, testing::Values(1, 100, maxGPUCoresTestCount),
44+
INSTANTIATE_TEST_SUITE_P(CompactByFieldPointsNodeTest, CompactByFieldPointsNodeTest,
45+
testing::Values(1, 100, maxGPUCoresTestCount),
4546
[](const auto& info) { return "pointsCount_" + std::to_string(info.param); });
4647

4748
TEST_F(CompactByFieldPointsNodeTest, invalid_argument_node)
@@ -90,15 +91,16 @@ TEST_P(CompactByFieldPointsNodeTest, points_all_non_hit)
9091

9192
runGraphWithAssertions(inNode);
9293

93-
auto&& outPointCloud = std::make_unique<TestPointCloud>(TestPointCloud::createFromNode(compactByFieldPointsNode, pointFields));
94+
auto&& outPointCloud = std::make_unique<TestPointCloud>(
95+
TestPointCloud::createFromNode(compactByFieldPointsNode, pointFields));
9496
EXPECT_EQ(outPointCloud->getPointCount(), 0);
9597

9698
// Check if the contents of outData have changed (they should not have)
9799
std::vector<::Field<XYZ_VEC3_F32>::type> outData{
98100
{ 2.0f, 3.0f, 5.0f},
99-
{ 7.0f, 11.0f, 13.0f},
100-
{17.0f, 19.0f, 23.0f}
101-
};
101+
{ 7.0f, 11.0f, 13.0f},
102+
{17.0f, 19.0f, 23.0f}
103+
};
102104
auto outDataCopy = outData;
103105

104106
int32_t outCount, outSize;
@@ -124,7 +126,8 @@ TEST_P(CompactByFieldPointsNodeTest, points_all_hit)
124126

125127
runGraphWithAssertions(inNode);
126128

127-
auto&& outPointCloud = std::make_unique<TestPointCloud>(TestPointCloud::createFromNode(compactByFieldPointsNode, pointFields));
129+
auto&& outPointCloud = std::make_unique<TestPointCloud>(
130+
TestPointCloud::createFromNode(compactByFieldPointsNode, pointFields));
128131

129132
EXPECT_EQ(*outPointCloud, *inPointCloud);
130133
}
@@ -140,7 +143,8 @@ TEST_P(CompactByFieldPointsNodeTest, points_random_hit)
140143
auto&& inPointCloudWithoutNonHits = inPointCloud;
141144
inPointCloudWithoutNonHits->removeNonHitPoints();
142145

143-
auto&& outPointCloud = std::make_unique<TestPointCloud>(TestPointCloud::createFromNode(compactByFieldPointsNode, pointFields));
146+
auto&& outPointCloud = std::make_unique<TestPointCloud>(
147+
TestPointCloud::createFromNode(compactByFieldPointsNode, pointFields));
144148

145149
EXPECT_EQ(*outPointCloud, *inPointCloudWithoutNonHits);
146150
}
@@ -172,7 +176,7 @@ TEST_P(CompactByFieldPointsNodeTest, multiple_compactions_applied_to_same_data)
172176
EXPECT_EQ(*outPointCloud, *inPointCloudWithoutNonHits);
173177
}
174178

175-
TEST_F(CompactByFieldPointsNodeTest, should_warn_when_empty_point_cloud)
179+
TEST_F(CompactByFieldPointsNodeTest, should_work_when_empty_point_cloud)
176180
{
177181
rgl_node_t emptyPointCloudOutputNode = nullptr;
178182
createOrUpdateNode<EmptyNode>(&emptyPointCloudOutputNode);

0 commit comments

Comments
 (0)