Skip to content

Commit c9a4bb6

Browse files
azrogersazrogers
committed
More review comments
1 parent e8aa593 commit c9a4bb6

File tree

5 files changed

+72
-46
lines changed

5 files changed

+72
-46
lines changed

‎Source/CesiumRuntime/Private/CesiumGeoJsonDocument.cpp‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,12 @@
77

88
#include <span>
99

10+
FCesiumGeoJsonDocument::FCesiumGeoJsonDocument() : _pDocument(nullptr) {}
11+
12+
FCesiumGeoJsonDocument::FCesiumGeoJsonDocument(
13+
std::shared_ptr<CesiumVectorData::GeoJsonDocument>&& document)
14+
: _pDocument(std::move(document)) {}
15+
1016
bool UCesiumGeoJsonDocumentBlueprintLibrary::LoadGeoJsonFromString(
1117
const FString& InString,
1218
FCesiumGeoJsonDocument& OutVectorDocument) {
@@ -41,13 +47,13 @@ bool UCesiumGeoJsonDocumentBlueprintLibrary::LoadGeoJsonFromString(
4147

4248
FCesiumGeoJsonObject UCesiumGeoJsonDocumentBlueprintLibrary::GetRootObject(
4349
const FCesiumGeoJsonDocument& InVectorDocument) {
44-
if (!InVectorDocument._document) {
50+
if (!InVectorDocument._pDocument) {
4551
return FCesiumGeoJsonObject();
4652
}
4753

4854
return FCesiumGeoJsonObject(
49-
InVectorDocument._document,
50-
&InVectorDocument._document->rootObject);
55+
InVectorDocument._pDocument,
56+
&InVectorDocument._pDocument->rootObject);
5157
}
5258

5359
UCesiumLoadVectorDocumentFromIonAsyncAction*

‎Source/CesiumRuntime/Private/CesiumGeoJsonObject.cpp‎

Lines changed: 50 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2,29 +2,38 @@
22

33
#include "CesiumGeospatial/Cartographic.h"
44
#include "Dom/JsonObject.h"
5+
#include "VecMath.h"
56

67
#include <utility>
78
#include <variant>
89
#include <vector>
910

11+
FCesiumGeoJsonFeature::FCesiumGeoJsonFeature()
12+
: _pDocument(nullptr), _pFeature(nullptr) {}
13+
14+
FCesiumGeoJsonFeature::FCesiumGeoJsonFeature(
15+
const std::shared_ptr<CesiumVectorData::GeoJsonDocument>& document,
16+
const CesiumVectorData::GeoJsonFeature* feature)
17+
: _pDocument(document), _pFeature(feature) {}
18+
1019
ECesiumGeoJsonFeatureIdType UCesiumGeoJsonFeatureBlueprintLibrary::GetIdType(
1120
const FCesiumGeoJsonFeature& InFeature) {
12-
if (!InFeature._document || !InFeature._feature ||
13-
std::holds_alternative<std::monostate>(InFeature._feature->id)) {
21+
if (!InFeature._pDocument || !InFeature._pFeature ||
22+
std::holds_alternative<std::monostate>(InFeature._pFeature->id)) {
1423
return ECesiumGeoJsonFeatureIdType::None;
1524
}
1625

17-
return std::holds_alternative<int64_t>(InFeature._feature->id)
26+
return std::holds_alternative<int64_t>(InFeature._pFeature->id)
1827
? ECesiumGeoJsonFeatureIdType::Integer
1928
: ECesiumGeoJsonFeatureIdType::String;
2029
}
2130

2231
int64 UCesiumGeoJsonFeatureBlueprintLibrary::GetIdAsInteger(
2332
const FCesiumGeoJsonFeature& InFeature) {
24-
if (!InFeature._document || !InFeature._feature) {
33+
if (!InFeature._pDocument || !InFeature._pFeature) {
2534
return -1;
2635
}
27-
const int64_t* pId = std::get_if<int64_t>(&InFeature._feature->id);
36+
const int64_t* pId = std::get_if<int64_t>(&InFeature._pFeature->id);
2837
if (!pId) {
2938
return -1;
3039
}
@@ -34,7 +43,7 @@ int64 UCesiumGeoJsonFeatureBlueprintLibrary::GetIdAsInteger(
3443

3544
FString UCesiumGeoJsonFeatureBlueprintLibrary::GetIdAsString(
3645
const FCesiumGeoJsonFeature& InFeature) {
37-
if (!InFeature._document || !InFeature._feature) {
46+
if (!InFeature._pDocument || !InFeature._pFeature) {
3847
return {};
3948
}
4049
struct GetIdVisitor {
@@ -45,7 +54,7 @@ FString UCesiumGeoJsonFeatureBlueprintLibrary::GetIdAsString(
4554
FString operator()(const std::monostate& id) { return {}; }
4655
};
4756

48-
return std::visit(GetIdVisitor{}, InFeature._feature->id);
57+
return std::visit(GetIdVisitor{}, InFeature._pFeature->id);
4958
}
5059

5160
namespace {
@@ -66,9 +75,27 @@ jsonValueToUnrealJsonValue(const CesiumUtility::JsonValue& value) {
6675
return MakeShared<FJsonValueNumber>(value);
6776
}
6877
TSharedPtr<FJsonValue> operator()(const std::uint64_t& value) {
78+
// If this value will fit into a double losslessly, we return it as a JSON
79+
// number.
80+
const std::optional<double> doubleOpt =
81+
CesiumUtility::losslessNarrow<double, std::uint64_t>(value);
82+
if (doubleOpt) {
83+
return MakeShared<FJsonValueNumber>(*doubleOpt);
84+
}
85+
86+
// Otherwise, we return it as a number string.
6987
return MakeShared<FJsonValueNumberString>(FString::FromInt(value));
7088
}
7189
TSharedPtr<FJsonValue> operator()(const std::int64_t& value) {
90+
// If this value will fit into a double losslessly, we return it as a JSON
91+
// number.
92+
const std::optional<double> doubleOpt =
93+
CesiumUtility::losslessNarrow<double, std::int64_t>(value);
94+
if (doubleOpt) {
95+
return MakeShared<FJsonValueNumber>(*doubleOpt);
96+
}
97+
98+
// Otherwise, we return it as a number string.
7299
return MakeShared<FJsonValueNumberString>(FString::FromInt(value));
73100
}
74101
TSharedPtr<FJsonValue>
@@ -96,36 +123,36 @@ jsonValueToUnrealJsonValue(const CesiumUtility::JsonValue& value) {
96123

97124
FJsonObjectWrapper UCesiumGeoJsonFeatureBlueprintLibrary::GetProperties(
98125
const FCesiumGeoJsonFeature& InFeature) {
99-
if (!InFeature._document || !InFeature._feature) {
126+
if (!InFeature._pDocument || !InFeature._pFeature) {
100127
return {};
101128
}
102129
TSharedPtr<FJsonObject> object = MakeShared<FJsonObject>();
103-
if (InFeature._feature->properties) {
104-
for (const auto& [k, v] : *InFeature._feature->properties) {
130+
if (InFeature._pFeature->properties) {
131+
for (const auto& [k, v] : *InFeature._pFeature->properties) {
105132
object->SetField(UTF8_TO_TCHAR(k.c_str()), jsonValueToUnrealJsonValue(v));
106133
}
107134
}
108135

109136
FJsonObjectWrapper wrapper;
110-
wrapper.JsonObject = object;
137+
wrapper.JsonObject = MoveTemp(object);
111138
return wrapper;
112139
}
113140

114141
FCesiumGeoJsonObject UCesiumGeoJsonFeatureBlueprintLibrary::GetGeometry(
115142
const FCesiumGeoJsonFeature& InFeature) {
116-
if (!InFeature._document || !InFeature._feature ||
117-
!InFeature._feature->geometry) {
143+
if (!InFeature._pDocument || !InFeature._pFeature ||
144+
!InFeature._pFeature->geometry) {
118145
return {};
119146
}
120147

121148
return FCesiumGeoJsonObject(
122-
InFeature._document,
123-
InFeature._feature->geometry.get());
149+
InFeature._pDocument,
150+
InFeature._pFeature->geometry.get());
124151
}
125152

126153
bool UCesiumGeoJsonFeatureBlueprintLibrary::IsValid(
127154
const FCesiumGeoJsonFeature& InFeature) {
128-
return InFeature._document != nullptr && InFeature._feature != nullptr;
155+
return InFeature._pDocument != nullptr && InFeature._pFeature != nullptr;
129156
}
130157

131158
bool UCesiumGeoJsonObjectBlueprintLibrary::IsValid(
@@ -181,16 +208,10 @@ FJsonObjectWrapper UCesiumGeoJsonObjectBlueprintLibrary::GetForeignMembers(
181208
}
182209

183210
FJsonObjectWrapper wrapper;
184-
wrapper.JsonObject = object;
211+
wrapper.JsonObject = MoveTemp(object);
185212
return wrapper;
186213
}
187214

188-
namespace {
189-
FVector degreesToVector(const glm::dvec3& coordinates) {
190-
return FVector(coordinates.x, coordinates.y, coordinates.z);
191-
}
192-
} // namespace
193-
194215
FVector UCesiumGeoJsonObjectBlueprintLibrary::GetObjectAsPoint(
195216
const FCesiumGeoJsonObject& InObject) {
196217
if (!InObject._pDocument || !InObject._pObject) {
@@ -204,7 +225,7 @@ FVector UCesiumGeoJsonObjectBlueprintLibrary::GetObjectAsPoint(
204225
return FVector::ZeroVector;
205226
}
206227

207-
return degreesToVector(pPoint->coordinates);
228+
return VecMath::createVector(pPoint->coordinates);
208229
}
209230

210231
TArray<FVector> UCesiumGeoJsonObjectBlueprintLibrary::GetObjectAsMultiPoint(
@@ -225,7 +246,7 @@ TArray<FVector> UCesiumGeoJsonObjectBlueprintLibrary::GetObjectAsMultiPoint(
225246
Points.Reserve(pMultiPoint->coordinates.size());
226247

227248
for (size_t i = 0; i < pMultiPoint->coordinates.size(); i++) {
228-
Points.Emplace(degreesToVector(pMultiPoint->coordinates[i]));
249+
Points.Emplace(VecMath::createVector(pMultiPoint->coordinates[i]));
229250
}
230251

231252
return Points;
@@ -250,7 +271,7 @@ UCesiumGeoJsonObjectBlueprintLibrary::GetObjectAsLineString(
250271
Points.Reserve(pLineString->coordinates.size());
251272

252273
for (size_t i = 0; i < pLineString->coordinates.size(); i++) {
253-
Points.Emplace(degreesToVector(pLineString->coordinates[i]));
274+
Points.Emplace(VecMath::createVector(pLineString->coordinates[i]));
254275
}
255276

256277
return FCesiumGeoJsonLineString(MoveTemp(Points));
@@ -279,7 +300,8 @@ UCesiumGeoJsonObjectBlueprintLibrary::GetObjectAsMultiLineString(
279300
Points.Reserve(pMultiLineString->coordinates[i].size());
280301

281302
for (size_t j = 0; j < pMultiLineString->coordinates[i].size(); j++) {
282-
Points.Emplace(degreesToVector(pMultiLineString->coordinates[i][j]));
303+
Points.Emplace(
304+
VecMath::createVector(pMultiLineString->coordinates[i][j]));
283305
}
284306

285307
Lines.Emplace(MoveTemp(Points));
@@ -414,7 +436,7 @@ UCesiumGeoJsonPolygonBlueprintFunctionLibrary::GetPolygonRings(
414436
Points.Reserve((*InPolygon._rings)[i].size());
415437

416438
for (size_t j = 0; j < (*InPolygon._rings)[i].size(); j++) {
417-
Points.Emplace(degreesToVector((*InPolygon._rings)[i][j]));
439+
Points.Emplace(VecMath::createVector((*InPolygon._rings)[i][j]));
418440
}
419441

420442
Rings.Emplace(MoveTemp(Points));

‎Source/CesiumRuntime/Private/Tests/CesiumVectorDocument.spec.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,13 @@
55
#include "Misc/AutomationTest.h"
66

77
BEGIN_DEFINE_SPEC(
8-
FCesiumVectorDocumentSpec,
9-
"Cesium.Unit.CesiumVectorDocument",
8+
FCesiumGeoJsonDocumentSpec,
9+
"Cesium.Unit.CesiumGeoJsonDocument",
1010
EAutomationTestFlags::EditorContext | EAutomationTestFlags::ClientContext |
1111
EAutomationTestFlags::ProductFilter | EAutomationTestFlags::NonNullRHI)
12-
END_DEFINE_SPEC(FCesiumVectorDocumentSpec)
12+
END_DEFINE_SPEC(FCesiumGeoJsonDocumentSpec)
1313

14-
void FCesiumVectorDocumentSpec::Define() {
14+
void FCesiumGeoJsonDocumentSpec::Define() {
1515
Describe("UCesiumGeoJsonDocumentBlueprintLibrary::LoadGeoJsonFromString", [this]() {
1616
It("loads a valid GeoJSON document", [this]() {
1717
FCesiumGeoJsonDocument Document;

‎Source/CesiumRuntime/Public/CesiumGeoJsonDocument.h‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,25 +25,24 @@ struct FCesiumGeoJsonDocument {
2525
/**
2626
* @brief Creates an empty `FCesiumGeoJsonDocument`.
2727
*/
28-
FCesiumGeoJsonDocument() : _document(nullptr) {}
28+
FCesiumGeoJsonDocument();
2929

3030
/**
3131
* @brief Creates a `FCesiumGeoJsonDocument` wrapping the provided
3232
* `CesiumVectorData::GeoJsonDocument`.
3333
*/
3434
FCesiumGeoJsonDocument(
35-
std::shared_ptr<CesiumVectorData::GeoJsonDocument>&& document)
36-
: _document(std::move(document)) {}
35+
std::shared_ptr<CesiumVectorData::GeoJsonDocument>&& document);
3736

3837
private:
39-
std::shared_ptr<CesiumVectorData::GeoJsonDocument> _document;
38+
std::shared_ptr<CesiumVectorData::GeoJsonDocument> _pDocument;
4039

4140
friend class UCesiumGeoJsonDocumentBlueprintLibrary;
4241
};
4342

4443
/**
4544
* @brief A Blueprint Function Library providing functions for interacting with
46-
* a `FCesiumVectorDocument`.
45+
* a `FCesiumGeoJsonDocument`.
4746
*/
4847
UCLASS()
4948
class UCesiumGeoJsonDocumentBlueprintLibrary
@@ -52,7 +51,7 @@ class UCesiumGeoJsonDocumentBlueprintLibrary
5251

5352
public:
5453
/**
55-
* @brief Attempts to load a `FCesiumVectorDocument` from a string containing
54+
* @brief Attempts to load a `FCesiumGeoJsonDocument` from a string containing
5655
* GeoJSON data.
5756
*
5857
* If loading fails, this function will return false and `OutVectorDocument`

‎Source/CesiumRuntime/Public/CesiumGeoJsonObject.h‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -76,20 +76,19 @@ struct FCesiumGeoJsonFeature {
7676
GENERATED_BODY()
7777

7878
/** @brief Creates a new `FCesiumVectorPrimitive` with an empty primitive. */
79-
FCesiumGeoJsonFeature() : _document(nullptr), _feature(nullptr) {}
79+
FCesiumGeoJsonFeature();
8080

8181
/**
8282
* @brief Creates a new `FCesiumGeoJsonFeature` wrapping the provided
8383
* `CesiumVectorData::GeoJsonFeature`.
8484
*/
8585
FCesiumGeoJsonFeature(
8686
const std::shared_ptr<CesiumVectorData::GeoJsonDocument>& document,
87-
const CesiumVectorData::GeoJsonFeature* feature)
88-
: _document(document), _feature(feature) {}
87+
const CesiumVectorData::GeoJsonFeature* feature);
8988

9089
private:
91-
std::shared_ptr<CesiumVectorData::GeoJsonDocument> _document;
92-
const CesiumVectorData::GeoJsonFeature* _feature;
90+
std::shared_ptr<CesiumVectorData::GeoJsonDocument> _pDocument;
91+
const CesiumVectorData::GeoJsonFeature* _pFeature;
9392

9493
friend class UCesiumGeoJsonFeatureBlueprintLibrary;
9594
};

0 commit comments

Comments
 (0)