Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

3d tiles writer #306

Closed
wants to merge 15 commits into from
Closed
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,9 @@ add_subdirectory(CesiumJsonWriter)
add_subdirectory(CesiumGltfWriter)
add_subdirectory(CesiumGltfReader)
add_subdirectory(CesiumAsync)
add_subdirectory(Cesium3DTiles)
add_subdirectory(Cesium3DTilesSelection)
add_subdirectory(Cesium3DTilesWriter)
add_subdirectory(CesiumIonClient)

# Private Targets
Expand Down
23 changes: 23 additions & 0 deletions Cesium3DTiles/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
add_library(Cesium3DTiles INTERFACE)

cesium_glob_files(CESIUM_3DTILES_HEADERS src/*.h src/*.hpp)
cesium_glob_files(CESIUM_3DTILES_PUBLIC_HEADERS include/Cesium3DTiles/*.h)

set_target_properties(Cesium3DTiles
PROPERTIES
PUBLIC_HEADER "${CESIUM_3DTILES_PUBLIC_HEADERS}"
)

target_include_directories(
Cesium3DTiles
SYSTEM INTERFACE
${CESIUM_NATIVE_RAPIDJSON_INCLUDE_DIR}
${CMAKE_CURRENT_LIST_DIR}/include/
INTERFACE
${CMAKE_CURRENT_LIST_DIR}/src
)

install(TARGETS Cesium3DTiles
LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
PUBLIC_HEADER DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/Cesium3DTiles
)
199 changes: 199 additions & 0 deletions Cesium3DTiles/include/Cesium3DTiles/Cesium3DTiles.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,199 @@
// This file was generated by generate-gltf-classes.
// DO NOT EDIT THIS FILE!
#pragma once

#include "Library.h"

#include <CesiumUtility/ExtensibleObject.h>
#include <optional>
#include <string>
#include <unordered_map>
#include <vector>

#define CODEGEN_API CESIUM3DTILES_API

namespace Cesium3DTiles {

struct CODEGEN_API Asset : public CesiumUtility::ExtensibleObject {
static inline constexpr const char* TypeName = "Asset";

std::string version;

std::optional<std::string> tilesetVersion;
};

struct CODEGEN_API BoundingVolume : public CesiumUtility::ExtensibleObject {
static inline constexpr const char* TypeName = "BoundingVolume";

std::optional<std::vector<double>> box;

std::optional<std::vector<double>> region;

std::optional<std::vector<double>> sphere;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick from me, it would be nice if this one can be a variant type. Maybe save a little bit of memory. Also it would be nice if we have a Box type that defines the center and half-axis, rather than vector<double>. Similar to Region and Sphere. If it's not possible for the generator, I wouldn't worry about it

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to use a hybrid approach here? For example, we can manually define Box, Sphere, and Region C++ type and all the utility functions. Then, we tell the generator to use those defined type, rather than create it's own. I haven't tried it, so I'm not sure it's worth the effort

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be possible, but I do think the generated code should be as schema-faithful as possible with minimum "customization", with any custom logic done through other means e.g. inheritance, non-member non-friend utility functions. So maybe in this case we provide e.g. a non-member non-friend BoundingVolume createBox(const glm::dvec3& center, const glm::dvec3& halfSize); in include/Utils.h and src/Utils.cpp.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related discussion in #362

};

struct CODEGEN_API TileContent : public CesiumUtility::ExtensibleObject {
static inline constexpr const char* TypeName = "TileContent";

std::optional<BoundingVolume> boundingVolume;

std::string uri;
};

struct CODEGEN_API Tile : public CesiumUtility::ExtensibleObject {
static inline constexpr const char* TypeName = "Tile";

enum class Refine { ADD, REPLACE };

BoundingVolume boundingVolume;

std::optional<BoundingVolume> viewerRequestVolume;

double geometricError;

std::optional<Refine> refine;

std::vector<double> transform =
{1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1};

std::optional<TileContent> content;

std::optional<std::vector<Tile>> children;
};

struct CODEGEN_API TilesetProperties : public CesiumUtility::ExtensibleObject {
static inline constexpr const char* TypeName = "TilesetProperties";

double maximum;

double minimum;
};

struct CODEGEN_API Tileset : public CesiumUtility::ExtensibleObject {
static inline constexpr const char* TypeName = "Tileset";

struct CODEGEN_API Properties : public CesiumUtility::ExtensibleObject {
static inline constexpr const char* TypeName = "Properties";

std::unordered_map<std::string, TilesetProperties> additionalProperties;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using unordered_map maybe slower than map when there are very few item

};

Asset asset;

std::optional<Properties> properties;

double geometricError;

Tile root;

std::optional<std::vector<std::string>> extensionsUsed;

std::optional<std::vector<std::string>> extensionsRequired;
};

struct CODEGEN_API FeatureTable : public CesiumUtility::ExtensibleObject {
static inline constexpr const char* TypeName = "FeatureTable";

struct CODEGEN_API BinaryBodyReference
: public CesiumUtility::ExtensibleObject {
static inline constexpr const char* TypeName = "BinaryBodyReference";

enum class ComponentType {
BYTE,
UNSIGNED_BYTE,
SHORT,
UNSIGNED_SHORT,
INT,
UNSIGNED_INT,
FLOAT,
DOUBLE
};

double byteOffset;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CesiumGltf has int64 for byteOffset. Can the generator do the same?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue is that the 3D Tiles schema has number for this, which does properly correspond to a double. Options are

  • change the schema to integer like glTF, which is technically a non-standard type (but the generator does handle)
  • add logic for handling this special case, which feels "incorrect"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should change the spec. integer is standard in the version of JSON Schema 3D Tiles and glTF use: https://datatracker.ietf.org/doc/html/draft-zyp-json-schema-04

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


std::optional<ComponentType> componentType;
};

struct CODEGEN_API Property {
static inline constexpr const char* TypeName = "Property";

std::optional<BinaryBodyReference> v0;
std::optional<std::vector<double>> v1;
std::optional<double> v2;
};
struct CODEGEN_API GlobalPropertyScalar {
static inline constexpr const char* TypeName = "GlobalPropertyScalar";

struct CODEGEN_API Variant0 : public CesiumUtility::ExtensibleObject {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kinda know why we use Variant0 naming here since schema doesn't define the name for it. But it still not a good word for API alone

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed but unsure how best to address this - looks like it would be fairly not-easy to override this in generator config

static inline constexpr const char* TypeName = "Variant0";

double byteOffset;
};

std::optional<Variant0> v0;
std::optional<std::vector<double>> v1;
std::optional<double> v2;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may have the same problem with the bounding volume above. The schema specifies GlobalPropertyScalar can only accept one of the properties above. Should using std::variant be a better choice in this case? cc @kring for more opinions

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did consider this, my concern was that the reader becomes much more complicated since it has to "read every alternative at the same time", so

  • some template/STL gymnastics is required to get each type instantiated as a read destination
  • there's no obvious choice when multiple alternatives are valid other than "first in the specified order", which also requires some STL mess to accomplish

Not opposed to doing it that way, just will take a little more work and be a little STL-heavy.

The "multiple optionalsapproach also covers the semantics of bothoneOfandanyOf`, granted that's not relevant for glTF/3DTiles

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without necessarily understanding the full implications, std::variant sounds right to me. It more closely matches the schema semantics and will be more memory efficient as well.

};
struct CODEGEN_API GlobalPropertyCartesian3 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick from me, but we should have space between structs

static inline constexpr const char* TypeName = "GlobalPropertyCartesian3";

struct CODEGEN_API Variant0 : public CesiumUtility::ExtensibleObject {
static inline constexpr const char* TypeName = "Variant0";

double byteOffset;
};

std::optional<Variant0> v0;
std::optional<std::vector<double>> v1;
};
struct CODEGEN_API GlobalPropertyCartesian4 {
static inline constexpr const char* TypeName = "GlobalPropertyCartesian4";

struct CODEGEN_API Variant0 : public CesiumUtility::ExtensibleObject {
static inline constexpr const char* TypeName = "Variant0";

double byteOffset;
};

std::optional<Variant0> v0;
std::optional<std::vector<double>> v1;
};

std::unordered_map<std::string, Property> additionalProperties;
};

struct CODEGEN_API PntsFeatureTable : public FeatureTable {
static inline constexpr const char* TypeName = "PntsFeatureTable";

std::optional<BinaryBodyReference> POSITION;

std::optional<BinaryBodyReference> POSITION_QUANTIZED;

std::optional<BinaryBodyReference> RGBA;

std::optional<BinaryBodyReference> RGB;

std::optional<BinaryBodyReference> RGB565;

std::optional<BinaryBodyReference> NORMAL;

std::optional<BinaryBodyReference> NORMAL_OCT16P;

std::optional<BinaryBodyReference> BATCH_ID;

std::optional<GlobalPropertyScalar> POINTS_LENGTH;

std::optional<GlobalPropertyCartesian3> RTC_CENTER;

std::optional<GlobalPropertyCartesian3> QUANTIZED_VOLUME_OFFSET;

std::optional<GlobalPropertyCartesian3> QUANTIZED_VOLUME_SCALE;

std::optional<GlobalPropertyCartesian4> CONSTANT_RGBA;

std::optional<GlobalPropertyScalar> BATCH_LENGTH;
};

} // namespace Cesium3DTiles

#undef CODEGEN_API
16 changes: 16 additions & 0 deletions Cesium3DTiles/include/Cesium3DTiles/Library.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#pragma once

/**
* @brief Classes for working with [glTF](https://www.khronos.org/gltf/) models.
*/
namespace Cesium3DTiles {}

#if defined(_WIN32) && defined(CESIUM_SHARED)
#ifdef CESIUM3DTILES_BUILDING
#define CESIUM3DTILES_API __declspec(dllexport)
#else
#define CESIUM3DTILES_API __declspec(dllimport)
#endif
#else
#define CESIUM3DTILES_API
#endif
53 changes: 53 additions & 0 deletions Cesium3DTilesWriter/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
add_library(Cesium3DTilesWriter "")

configure_cesium_library(Cesium3DTilesWriter)

cesium_glob_files(CESIUM_3DTILES_WRITER_SOURCES src/*.cpp generated/*.cpp)
cesium_glob_files(CESIUM_3DTILES_WRITER_HEADERS src/*.h src/*.hpp generated/*.h)
cesium_glob_files(CESIUM_3DTILES_WRITER_PUBLIC_HEADERS include/Cesium3DTiles/*.h)
cesium_glob_files(CESIUM_3DTILES_WRITER_TEST_SOURCES test/*.cpp)
cesium_glob_files(CESIUM_3DTILES_WRITER_TEST_HEADERS test/*.h)

set_target_properties(Cesium3DTilesWriter
PROPERTIES
TEST_SOURCES "${CESIUM_3DTILES_WRITER_TEST_SOURCES}"
TEST_HEADERS "${CESIUM_3DTILES_WRITER_TEST_HEADERS}"
PUBLIC_HEADER "${CESIUM_3DTILES_WRITER_PUBLIC_HEADERS}"
)

target_sources(
Cesium3DTilesWriter
PRIVATE
${CESIUM_3DTILES_WRITER_SOURCES}
${CESIUM_3DTILES_WRITER_HEADERS}
PUBLIC
${CESIUM_3DTILES_WRITER_PUBLIC_HEADERS}
)

target_compile_definitions(
Cesium3DTilesWriter
PRIVATE
${CESIUM_NATIVE_RAPIDJSON_DEFINES}
)

target_include_directories(
Cesium3DTilesWriter
SYSTEM PUBLIC
${CESIUM_NATIVE_RAPIDJSON_INCLUDE_DIR}
${CMAKE_BINARY_DIR}
${CMAKE_CURRENT_LIST_DIR}/include
PRIVATE
${CMAKE_CURRENT_LIST_DIR}/src
${CMAKE_CURRENT_LIST_DIR}/generated
)

target_link_libraries(Cesium3DTilesWriter
PUBLIC
Cesium3DTiles
CesiumJsonWriter
)

install(TARGETS Cesium3DTilesWriter
LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
PUBLIC_HEADER DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/Cesium3DTiles
)
Loading