CesiumGS / CesiumGS/cesium-native

Consider exposing fewer of the internal Tile functions to the user

Open
#36 4 comments 0 reactions 1 assignee Claimed by @javagl View on GitHub
quality
Dominant language
C++
Stars
623
Forks
277
PR merge metrics
No merged PRs in 30d

Description

At the time of writing this, the [`Tile` class has >30 functions that are `public`](https://github.com/CesiumGS/cesium-native/blob/7260ddca4d8ec2bcab0aa87f3c34b5938d01b556/Cesium3DTiles/include/Cesium3DTiles/Tile.h#L125).

I'm listing them here (omitting possible `const` versions), as an overview, and from a **very** short glance (just to have an estimate!), only the first half of this list should actually be `public`:

Tileset* getTileset();
TileContext* getContext();
Tile* getParent();
gsl::span getChildren();
const BoundingVolume& getBoundingVolume() const;
const std::optional& getViewerRequestVolume() const;
const std::optional& getContentBoundingVolume() const;
double getGeometricError() const;
TileRefine getRefine() const;
const glm::dmat4x4& getTransform() const;
TileContentLoadResult* getContent();
void* getRendererResources() const;
bool isRenderable() const;

void prepareToDestroy();
void setContext(TileContext* pContext);
void setParent(Tile* pParent);
void createChildTiles(size_t count);
void createChildTiles(std::vector&& children);
void setBoundingVolume(const BoundingVolume& value);
void setViewerRequestVolume(const std::optional& value);
void setGeometricError(double value);
void setRefine(TileRefine value);
void setTransform(const glm::dmat4x4& value);
const TileID& getTileID() const;
void setTileID(const TileID& id);
void setContentBoundingVolume(const std::optional& value);
LoadState getState() const;
TileSelectionState& getLastSelectionState();
void setLastSelectionState(const TileSelectionState& newState);
void loadContent();
bool unloadContent();
void update(uint32_t previousFrameNumber, uint32_t currentFrameNumber);

The second part (particularly, basically all `set...` functions) are only or mainly used for the initialization and setup, and maintaining the tiles structure during the rendering process. Specifically, most of them are only called by the `Tileset` (and few of them by other internal classes).

One could consider to reduce the size of the "surface" of this class: Users should not call things like `tile->setParent(nullptr)`, and thus, should preferably not be *able* to do this.

(*Some* of the function may also only be relevant for certain use-cases. This should be pointed out in the documentation)

It might be possible to solve this by simply letting `Tile` and `Tileset` be `friend` classes, but more elaborate (or "clean") solutions may exist. (Similar questions may arise for other classes that are publicly exposed to the user, but to a much lesser extent).

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.