From e403d73f34ba217412e04ca794f42b3ba19a8db1 Mon Sep 17 00:00:00 2001 From: Janine Liu Date: Mon, 26 Feb 2024 17:24:53 -0500 Subject: [PATCH 01/12] Add flag for KHR_texture_transform to property texture --- CesiumGltf/include/CesiumGltf/PropertyTextureView.h | 11 +++++++++-- CesiumGltf/src/PropertyTextureView.cpp | 4 +++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/CesiumGltf/include/CesiumGltf/PropertyTextureView.h b/CesiumGltf/include/CesiumGltf/PropertyTextureView.h index f7d58cdbc..92c0488a1 100644 --- a/CesiumGltf/include/CesiumGltf/PropertyTextureView.h +++ b/CesiumGltf/include/CesiumGltf/PropertyTextureView.h @@ -55,10 +55,14 @@ class PropertyTextureView { * @param model The glTF that contains the property texture's data. * @param propertyTexture The {@link PropertyTexture} * from which the view will retrieve data. + * @param applyKhrTextureTransformExtension Whether to automatically apply the + * `KHR_texture_transform` extension to the properties in this property + * texture, if present. */ PropertyTextureView( const Model& model, - const PropertyTexture& propertyTexture) noexcept; + const PropertyTexture& propertyTexture, + bool applyKhrTextureTransformExtension = false) noexcept; /** * @brief Gets the status of this property texture view. @@ -699,7 +703,8 @@ class PropertyTextureView { propertyTextureProperty, classProperty, _pModel->samplers[samplerIndex], - image); + image, + this->_applyTextureTransform); } PropertyViewStatusType getTextureSafe( @@ -720,6 +725,8 @@ class PropertyTextureView { const PropertyTexture* _pPropertyTexture; const Class* _pClass; + bool _applyTextureTransform; + PropertyTextureViewStatus _status; }; } // namespace CesiumGltf diff --git a/CesiumGltf/src/PropertyTextureView.cpp b/CesiumGltf/src/PropertyTextureView.cpp index f5ac76a11..33ab9b1bc 100644 --- a/CesiumGltf/src/PropertyTextureView.cpp +++ b/CesiumGltf/src/PropertyTextureView.cpp @@ -3,10 +3,12 @@ namespace CesiumGltf { PropertyTextureView::PropertyTextureView( const Model& model, - const PropertyTexture& propertyTexture) noexcept + const PropertyTexture& propertyTexture, + bool applyKhrTextureTransformExtension) noexcept : _pModel(&model), _pPropertyTexture(&propertyTexture), _pClass(nullptr), + _applyTextureTransform(applyKhrTextureTransformExtension), _status() { const ExtensionModelExtStructuralMetadata* pMetadata = model.getExtension(); From 9f8678ea31d1a49ee92862b445f0ca6059f4d37e Mon Sep 17 00:00:00 2001 From: Janine Liu Date: Tue, 27 Feb 2024 14:25:41 -0500 Subject: [PATCH 02/12] Add PropertyTexturePropertyView options --- .../CesiumGltf/PropertyTexturePropertyView.h | 197 +++++++++++------- .../include/CesiumGltf/PropertyTextureView.h | 8 +- CesiumGltf/src/PropertyTextureView.cpp | 4 +- .../test/TestPropertyTexturePropertyView.cpp | 134 +++++++++++- 4 files changed, 257 insertions(+), 86 deletions(-) diff --git a/CesiumGltf/include/CesiumGltf/PropertyTexturePropertyView.h b/CesiumGltf/include/CesiumGltf/PropertyTexturePropertyView.h index c2a5d6291..d9186885b 100644 --- a/CesiumGltf/include/CesiumGltf/PropertyTexturePropertyView.h +++ b/CesiumGltf/include/CesiumGltf/PropertyTexturePropertyView.h @@ -82,6 +82,44 @@ class PropertyTexturePropertyViewStatus : public PropertyViewStatus { static const int ErrorChannelsAndTypeMismatch = 22; }; +/** + * @brief Describes options for constructing a {@link PropertyTexturePropertyView}. + */ +struct PropertyTexturePropertyViewOptions { + /** + * @brief Whether to automatically apply the `KHR_texture_transform` extension + * to the property texture property, if it exists. + * + * A property texture property may contain the `KHR_texture_transform` + * extension, which transforms the texture coordinates used to sample the + * texture. The extension may also override the TEXCOORD set index that was + * originally specified by the property texture property. + * + * If a view is constructed with applyKhrTextureTransformExtension set to + * true, the view will automatically apply the texture transform to any UV + * coordinates used to sample the texture. If the extension defines its own + * TEXCOORD set index, it will override the original value. + * + * Otherwise, if the flag is set to false, UVs will not be transformed and + * the original TEXCOORD set index will be preserved. The extension's values + * may still be retrieved using getTextureTransform, if desired. + */ + bool applyKhrTextureTransformExtension; + + /** + * @brief Whether to copy the input image. + * + * By default, a view is constructed on the input glTF image without copying + * its pixels. This can be problematic for clients that move or delete the + * original glTF model. When this flag is true, the view will manage its own + * copy of the pixel data to avoid such issues. + */ + bool makeImageCopy; + + PropertyTexturePropertyViewOptions() + : applyKhrTextureTransformExtension(false), makeImageCopy(false) {} +}; + template ElementType assembleScalarValue(const gsl::span bytes) noexcept { if constexpr (std::is_same_v) { @@ -300,43 +338,29 @@ class PropertyTexturePropertyView /** * @brief Construct a view of the data specified by a {@link PropertyTextureProperty}. * - * A property texture property may contain the `KHR_texture_transform` - * extension, which transforms the texture coordinates used to sample the - * texture. The extension may also override the TEXCOORD set index that was - * originally specified by the property texture property. - * - * If a view is constructed with applyKhrTextureTransformExtension set to - * true, the view will automatically apply the texture transform to any UV - * coordinates used to sample the texture. If the extension defines its own - * TEXCOORD set index, it will override the original value. - * - * Otherwise, if the flag is set to false, UVs will not be transformed and - * the original TEXCOORD set index will be preserved. The extension's values - * may still be retrieved using getTextureTransform, if desired. - * * @param property The {@link PropertyTextureProperty} * @param classProperty The {@link ClassProperty} this property conforms to. * @param sampler The {@link Sampler} used by the property. * @param image The {@link ImageCesium} used by the property. * @param channels The value of {@link PropertyTextureProperty::channels}. - * @param applyKhrTextureTransformExtension Whether to automatically apply the - * `KHR_texture_transform` extension to the property texture property, if it - * exists. + * @param options The options for constructing the view. */ PropertyTexturePropertyView( const PropertyTextureProperty& property, const ClassProperty& classProperty, const Sampler& sampler, const ImageCesium& image, - bool applyKhrTextureTransformExtension = false) noexcept + PropertyTexturePropertyViewOptions options = + PropertyTexturePropertyViewOptions()) noexcept : PropertyView(classProperty, property), _pSampler(&sampler), - _pImage(&image), + _pImage(nullptr), _texCoordSetIndex(property.texCoord), _channels(property.channels), _swizzle(), - _applyTextureTransform(applyKhrTextureTransformExtension), - _textureTransform(std::nullopt) { + _applyTextureTransform(options.applyKhrTextureTransformExtension), + _textureTransform(std::nullopt), + _imageCopy(std::nullopt) { if (this->_status != PropertyTexturePropertyViewStatus::Valid) { return; } @@ -368,6 +392,12 @@ class PropertyTexturePropertyView if (pTextureTransform) { this->_textureTransform = KhrTextureTransform(*pTextureTransform); } + + if (options.makeImageCopy) { + this->_imageCopy = ImageCesium(image); + } else { + this->_pImage = ℑ + } } /** @@ -438,8 +468,9 @@ class PropertyTexturePropertyView u = applySamplerWrapS(u, this->_pSampler->wrapS); v = applySamplerWrapT(v, this->_pSampler->wrapT); + const ImageCesium& image = this->_imageCopy.value_or(*this->_pImage); std::array sample = - sampleNearestPixel(*this->_pImage, this->_channels, u, v); + sampleNearestPixel(image, this->_channels, u, v); return assembleValueFromChannels( gsl::span(sample.data(), this->_channels.size())); } @@ -447,11 +478,12 @@ class PropertyTexturePropertyView /** * @brief Get the texture coordinate set index for this property. * - * If applyKhrTextureTransformExtension is true, and if the property texture - * property contains the `KHR_texture_transform` extension, this will return - * the value from the extension, as it is meant to override the original - * index. However, if the extension does not specify a TEXCOORD set index, - * then the original index of the property texture property is returned. + * If this view was constructed with options.applyKhrTextureTransformExtension + * as true, and if the property texture property contains the + * `KHR_texture_transform` extension, this will return the value from the + * extension, as it is meant to override the original index. However, if the + * extension does not specify a TEXCOORD set index, then the original index of + * the property texture property is returned. */ int64_t getTexCoordSetIndex() const noexcept { if (this->_applyTextureTransform && this->_textureTransform) { @@ -476,7 +508,12 @@ class PropertyTexturePropertyView * This will be nullptr if the property texture property view runs into * problems during construction. */ - const ImageCesium* getImage() const noexcept { return this->_pImage; } + const ImageCesium* getImage() const noexcept { + if (this->_imageCopy) { + return &(this->_imageCopy.value()); + } + return this->_pImage; + } /** * @brief Gets the channels of this property texture property. @@ -491,15 +528,15 @@ class PropertyTexturePropertyView const std::string& getSwizzle() const noexcept { return this->_swizzle; } /** - * @brief Get the KHR_texture_transform for this property texture property, if - * it exists. + * @brief Get the KHR_texture_transform for this property texture property, + * if it exists. * - * Even if this view was constructed with applyKhrTextureTransformExtension - * set to false, it will save the extension's values, and they may be - * retrieved through this function. + * Even if this view was constructed with + * options.applyKhrTextureTransformExtension set to false, it will save the + * extension's values, and they may be retrieved through this function. * - * If this view was constructed with applyKhrTextureTransformExtension set to - * true, any texture coordinates passed into `get` or `getRaw` will be + * If this view was constructed with applyKhrTextureTransformExtension set + * to true, any texture coordinates passed into `get` or `getRaw` will be * automatically transformed, so there's no need to re-apply the transform * here. */ @@ -513,8 +550,10 @@ class PropertyTexturePropertyView int64_t _texCoordSetIndex; std::vector _channels; std::string _swizzle; + bool _applyTextureTransform; std::optional _textureTransform; + std::optional _imageCopy; }; #pragma endregion @@ -564,12 +603,14 @@ class PropertyTexturePropertyView _textureTransform(std::nullopt) { assert( this->_status != PropertyTexturePropertyViewStatus::Valid && - "An empty property view should not be constructed with a valid status"); + "An empty property view should not be constructed with a valid " + "status"); } /** - * @brief Constructs an instance of an empty property that specifies a default - * value. Although this property has no data, it can return the default value + * @brief Constructs an instance of an empty property that specifies a + * default value. Although this property has no data, it can return the + * default value * when {@link PropertyTexturePropertyView::get} is called. However, * {@link PropertyTexturePropertyView::getRaw} cannot be used. * @@ -605,42 +646,27 @@ class PropertyTexturePropertyView /** * @brief Construct a view of the data specified by a {@link PropertyTextureProperty}. * - * A property texture property may contain the `KHR_texture_transform` - * extension, which transforms the texture coordinates used to sample the - * texture. The extension may also override the TEXCOORD set index that was - * originally specified by the property texture property. - * - * If a view is constructed with applyKhrTextureTransformExtension set to - * true, the view will automatically apply the texture transform to any UV - * coordinates used to sample the texture. If the extension defines its own - * TEXCOORD set index, it will override the original value. - * - * Otherwise, if the flag is set to false, UVs will not be transformed and - * the original TEXCOORD set index will be preserved. The extension's values - * may still be retrieved using getTextureTransform, if desired. - * * @param property The {@link PropertyTextureProperty} * @param classProperty The {@link ClassProperty} this property conforms to. * @param sampler The {@link Sampler} used by the property. * @param image The {@link ImageCesium} used by the property. * @param channels The value of {@link PropertyTextureProperty::channels}. - * @param applyKhrTextureTransformExtension Whether to automatically apply the - * `KHR_texture_transform` extension to the property texture property, if it - * exists. + * @param options The options for constructing the view. */ PropertyTexturePropertyView( const PropertyTextureProperty& property, const ClassProperty& classProperty, const Sampler& sampler, const ImageCesium& image, - bool applyKhrTextureTransformExtension = false) noexcept + PropertyTexturePropertyViewOptions options = + PropertyTexturePropertyViewOptions()) noexcept : PropertyView(classProperty, property), _pSampler(&sampler), _pImage(&image), _texCoordSetIndex(property.texCoord), _channels(property.channels), _swizzle(), - _applyTextureTransform(applyKhrTextureTransformExtension), + _applyTextureTransform(options.applyKhrTextureTransformExtension), _textureTransform(std::nullopt) { if (this->_status != PropertyTexturePropertyViewStatus::Valid) { return; @@ -672,6 +698,12 @@ class PropertyTexturePropertyView if (pTextureTransform) { this->_textureTransform = KhrTextureTransform(*pTextureTransform); } + + if (options.makeImageCopy) { + this->_imageCopy = ImageCesium(image); + } else { + this->_pImage = ℑ + } } /** @@ -681,10 +713,10 @@ class PropertyTexturePropertyView * returned. The sampler's wrapping mode will be used when sampling the * texture. * - * If this property has a specified "no data" value, and the retrieved element - * is equal to that value, then this will return the property's specified - * default value. If the property did not provide a default value, this - * returns std::nullopt. + * If this property has a specified "no data" value, and the retrieved + * element is equal to that value, then this will return the property's + * specified default value. If the property did not provide a default value, + * this returns std::nullopt. * * @param u The u-component of the texture coordinates. * @param v The v-component of the texture coordinates. @@ -746,8 +778,8 @@ class PropertyTexturePropertyView * coordinates. The sampler's wrapping mode will be used when sampling the * texture. * - * If this property has a specified "no data" value, the raw value will still - * be returned, even if it equals the "no data" value. + * If this property has a specified "no data" value, the raw value will + * still be returned, even if it equals the "no data" value. * * @param u The u-component of the texture coordinates. * @param v The v-component of the texture coordinates. @@ -769,8 +801,9 @@ class PropertyTexturePropertyView u = applySamplerWrapS(u, this->_pSampler->wrapS); v = applySamplerWrapT(v, this->_pSampler->wrapT); + const ImageCesium& image = this->_imageCopy.value_or(*this->_pImage); std::array sample = - sampleNearestPixel(*this->_pImage, this->_channels, u, v); + sampleNearestPixel(image, this->_channels, u, v); return assembleValueFromChannels( gsl::span(sample.data(), this->_channels.size())); @@ -778,12 +811,12 @@ class PropertyTexturePropertyView /** * @brief Get the texture coordinate set index for this property. - * - * If applyKhrTextureTransformExtension is true, and if the property texture - * property contains the `KHR_texture_transform` extension, this will return - * the value from the extension, as it is meant to override the original - * index. However, if the extension does not specify a TEXCOORD set index, - * then the original index of the property texture property is returned. + * If this view was constructed with options.applyKhrTextureTransformExtension + * as true, and if the property texture property contains the + * `KHR_texture_transform` extension, this will return the value from the + * extension, as it is meant to override the original index. However, if the + * extension does not specify a TEXCOORD set index, then the original index of + * the property texture property is returned. */ int64_t getTexCoordSetIndex() const noexcept { if (this->_applyTextureTransform && this->_textureTransform) { @@ -808,7 +841,12 @@ class PropertyTexturePropertyView * This will be nullptr if the property texture property view runs into * problems during construction. */ - const ImageCesium* getImage() const noexcept { return this->_pImage; } + const ImageCesium* getImage() const noexcept { + if (this->_imageCopy) { + return &(this->_imageCopy.value()); + } + return this->_pImage; + } /** * @brief Gets the channels of this property texture property. @@ -823,15 +861,15 @@ class PropertyTexturePropertyView const std::string& getSwizzle() const noexcept { return this->_swizzle; } /** - * @brief Get the KHR_texture_transform for this property texture property, if - * it exists. + * @brief Get the KHR_texture_transform for this property texture property, + * if it exists. * - * Even if this view was constructed with applyKhrTextureTransformExtension - * set to false, it will save the extension's values, and they may be - * retrieved through this function. + * Even if this view was constructed with + * options.applyKhrTextureTransformExtension set to false, it will save the + * extension's values, and they may be retrieved through this function. * - * If this view was constructed with applyKhrTextureTransformExtension set to - * true, any texture coordinates passed into `get` or `getRaw` will be + * If this view was constructed with applyKhrTextureTransformExtension set + * to true, any texture coordinates passed into `get` or `getRaw` will be * automatically transformed, so there's no need to re-apply the transform * here. */ @@ -845,8 +883,11 @@ class PropertyTexturePropertyView int64_t _texCoordSetIndex; std::vector _channels; std::string _swizzle; + bool _applyTextureTransform; std::optional _textureTransform; + + std::optional _imageCopy; }; #pragma endregion diff --git a/CesiumGltf/include/CesiumGltf/PropertyTextureView.h b/CesiumGltf/include/CesiumGltf/PropertyTextureView.h index 92c0488a1..f9cb4ed2d 100644 --- a/CesiumGltf/include/CesiumGltf/PropertyTextureView.h +++ b/CesiumGltf/include/CesiumGltf/PropertyTextureView.h @@ -62,7 +62,8 @@ class PropertyTextureView { PropertyTextureView( const Model& model, const PropertyTexture& propertyTexture, - bool applyKhrTextureTransformExtension = false) noexcept; + PropertyTexturePropertyViewOptions propertyOptions = + PropertyTexturePropertyViewOptions()) noexcept; /** * @brief Gets the status of this property texture view. @@ -704,7 +705,7 @@ class PropertyTextureView { classProperty, _pModel->samplers[samplerIndex], image, - this->_applyTextureTransform); + this->_propertyOptions); } PropertyViewStatusType getTextureSafe( @@ -725,8 +726,7 @@ class PropertyTextureView { const PropertyTexture* _pPropertyTexture; const Class* _pClass; - bool _applyTextureTransform; - + PropertyTexturePropertyViewOptions _propertyOptions; PropertyTextureViewStatus _status; }; } // namespace CesiumGltf diff --git a/CesiumGltf/src/PropertyTextureView.cpp b/CesiumGltf/src/PropertyTextureView.cpp index 33ab9b1bc..0a2c16934 100644 --- a/CesiumGltf/src/PropertyTextureView.cpp +++ b/CesiumGltf/src/PropertyTextureView.cpp @@ -4,11 +4,11 @@ namespace CesiumGltf { PropertyTextureView::PropertyTextureView( const Model& model, const PropertyTexture& propertyTexture, - bool applyKhrTextureTransformExtension) noexcept + PropertyTexturePropertyViewOptions options) noexcept : _pModel(&model), _pPropertyTexture(&propertyTexture), _pClass(nullptr), - _applyTextureTransform(applyKhrTextureTransformExtension), + _propertyOptions(options), _status() { const ExtensionModelExtStructuralMetadata* pMetadata = model.getExtension(); diff --git a/CesiumGltf/test/TestPropertyTexturePropertyView.cpp b/CesiumGltf/test/TestPropertyTexturePropertyView.cpp index e112b6a93..02b486e08 100644 --- a/CesiumGltf/test/TestPropertyTexturePropertyView.cpp +++ b/CesiumGltf/test/TestPropertyTexturePropertyView.cpp @@ -1805,8 +1805,11 @@ TEST_CASE("Test PropertyTextureProperty constructs with " property.channels = {0}; + PropertyTexturePropertyViewOptions options; + options.applyKhrTextureTransformExtension = true; + PropertyTexturePropertyView - view(property, classProperty, sampler, image, true); + view(property, classProperty, sampler, image, options); REQUIRE(view.status() == PropertyTexturePropertyViewStatus::Valid); auto textureTransform = view.getTextureTransform(); @@ -1875,8 +1878,11 @@ TEST_CASE("Test normalized PropertyTextureProperty constructs with " property.channels = {0}; + PropertyTexturePropertyViewOptions options; + options.applyKhrTextureTransformExtension = true; + PropertyTexturePropertyView - view(property, classProperty, sampler, image, true); + view(property, classProperty, sampler, image, options); REQUIRE(view.status() == PropertyTexturePropertyViewStatus::Valid); auto textureTransform = view.getTextureTransform(); @@ -1911,3 +1917,127 @@ TEST_CASE("Test normalized PropertyTextureProperty constructs with " static_cast(expectedValues[i]) / 255.0); } } + +TEST_CASE("Test PropertyTextureProperty constructs with makeImageCopy = true") { + std::vector data{0, 64, 127, 255}; + + PropertyTextureProperty property; + property.texCoord = 0; + + ExtensionKhrTextureTransform& textureTransformExtension = + property.addExtension(); + textureTransformExtension.offset = {0.5, -0.5}; + textureTransformExtension.rotation = CesiumUtility::Math::PiOverTwo; + textureTransformExtension.scale = {0.5, 0.5}; + textureTransformExtension.texCoord = 10; + + ClassProperty classProperty; + classProperty.type = ClassProperty::Type::SCALAR; + classProperty.componentType = ClassProperty::ComponentType::UINT8; + classProperty.normalized = true; + + Sampler sampler; + sampler.wrapS = Sampler::WrapS::REPEAT; + sampler.wrapT = Sampler::WrapT::REPEAT; + + ImageCesium image; + image.width = 2; + image.height = 2; + image.channels = 1; + image.bytesPerChannel = 1; + + std::vector& imageData = image.pixelData; + imageData.resize(data.size()); + std::memcpy(imageData.data(), data.data(), data.size()); + + property.channels = {0}; + + PropertyTexturePropertyViewOptions options; + options.makeImageCopy = true; + + PropertyTexturePropertyView + view(property, classProperty, sampler, image, options); + REQUIRE(view.status() == PropertyTexturePropertyViewStatus::Valid); + + // Clear the original image data. + std::vector emptyData; + image.pixelData.swap(emptyData); + + const ImageCesium* pImage = view.getImage(); + REQUIRE(pImage); + REQUIRE(pImage->pixelData.size() == data.size()); + + std::vector texCoords{ + glm::dvec2(0, 0), + glm::dvec2(0.5, 0), + glm::dvec2(0, 0.5), + glm::dvec2(0.5, 0.5)}; + + for (size_t i = 0; i < texCoords.size(); i++) { + glm::dvec2 uv = texCoords[i]; + REQUIRE(view.getRaw(uv[0], uv[1]) == data[i]); + REQUIRE(view.get(uv[0], uv[1]) == static_cast(data[i]) / 255.0); + } +} + +TEST_CASE("Test normalized PropertyTextureProperty constructs with " + "makeImageCopy = true") { + std::vector data{1, 2, 3, 4}; + + PropertyTextureProperty property; + property.texCoord = 0; + + ExtensionKhrTextureTransform& textureTransformExtension = + property.addExtension(); + textureTransformExtension.offset = {0.5, -0.5}; + textureTransformExtension.rotation = CesiumUtility::Math::PiOverTwo; + textureTransformExtension.scale = {0.5, 0.5}; + textureTransformExtension.texCoord = 10; + + ClassProperty classProperty; + classProperty.type = ClassProperty::Type::SCALAR; + classProperty.componentType = ClassProperty::ComponentType::UINT8; + + Sampler sampler; + sampler.wrapS = Sampler::WrapS::REPEAT; + sampler.wrapT = Sampler::WrapT::REPEAT; + + ImageCesium image; + image.width = 2; + image.height = 2; + image.channels = 1; + image.bytesPerChannel = 1; + + std::vector& imageData = image.pixelData; + imageData.resize(data.size()); + std::memcpy(imageData.data(), data.data(), data.size()); + + property.channels = {0}; + + PropertyTexturePropertyViewOptions options; + options.makeImageCopy = true; + + PropertyTexturePropertyView + view(property, classProperty, sampler, image, options); + REQUIRE(view.status() == PropertyTexturePropertyViewStatus::Valid); + + // Clear the original image data. + std::vector emptyData; + image.pixelData.swap(emptyData); + + const ImageCesium* pImage = view.getImage(); + REQUIRE(pImage); + REQUIRE(pImage->pixelData.size() == data.size()); + + std::vector texCoords{ + glm::dvec2(0, 0), + glm::dvec2(0.5, 0), + glm::dvec2(0, 0.5), + glm::dvec2(0.5, 0.5)}; + + for (size_t i = 0; i < texCoords.size(); i++) { + glm::dvec2 uv = texCoords[i]; + REQUIRE(view.getRaw(uv[0], uv[1]) == data[i]); + REQUIRE(view.get(uv[0], uv[1]) == data[i]); + } +} From 218bf1ab1e5d6e838407b30d6728505a88ee8f5e Mon Sep 17 00:00:00 2001 From: Janine Liu Date: Tue, 27 Feb 2024 18:00:42 -0500 Subject: [PATCH 03/12] Rework PropertyTexturePropertyView to use TextureView --- .../CesiumGltf/PropertyTexturePropertyView.h | 350 ++++-------------- .../include/CesiumGltf/PropertyTextureView.h | 7 +- CesiumGltf/include/CesiumGltf/TextureView.h | 222 +++++++++++ .../src/PropertyTexturePropertyView.cpp | 42 --- CesiumGltf/src/PropertyTextureView.cpp | 2 +- CesiumGltf/src/TextureView.cpp | 168 +++++++++ .../test/TestPropertyTexturePropertyView.cpp | 8 +- 7 files changed, 478 insertions(+), 321 deletions(-) create mode 100644 CesiumGltf/include/CesiumGltf/TextureView.h create mode 100644 CesiumGltf/src/TextureView.cpp diff --git a/CesiumGltf/include/CesiumGltf/PropertyTexturePropertyView.h b/CesiumGltf/include/CesiumGltf/PropertyTexturePropertyView.h index d9186885b..685408c7f 100644 --- a/CesiumGltf/include/CesiumGltf/PropertyTexturePropertyView.h +++ b/CesiumGltf/include/CesiumGltf/PropertyTexturePropertyView.h @@ -8,6 +8,7 @@ #include "CesiumGltf/PropertyView.h" #include "CesiumGltf/Sampler.h" #include "CesiumGltf/SamplerUtility.h" +#include "CesiumGltf/TextureView.h" #include #include @@ -82,44 +83,6 @@ class PropertyTexturePropertyViewStatus : public PropertyViewStatus { static const int ErrorChannelsAndTypeMismatch = 22; }; -/** - * @brief Describes options for constructing a {@link PropertyTexturePropertyView}. - */ -struct PropertyTexturePropertyViewOptions { - /** - * @brief Whether to automatically apply the `KHR_texture_transform` extension - * to the property texture property, if it exists. - * - * A property texture property may contain the `KHR_texture_transform` - * extension, which transforms the texture coordinates used to sample the - * texture. The extension may also override the TEXCOORD set index that was - * originally specified by the property texture property. - * - * If a view is constructed with applyKhrTextureTransformExtension set to - * true, the view will automatically apply the texture transform to any UV - * coordinates used to sample the texture. If the extension defines its own - * TEXCOORD set index, it will override the original value. - * - * Otherwise, if the flag is set to false, UVs will not be transformed and - * the original TEXCOORD set index will be preserved. The extension's values - * may still be retrieved using getTextureTransform, if desired. - */ - bool applyKhrTextureTransformExtension; - - /** - * @brief Whether to copy the input image. - * - * By default, a view is constructed on the input glTF image without copying - * its pixels. This can be problematic for clients that move or delete the - * original glTF model. When this flag is true, the view will manage its own - * copy of the pixel data to avoid such issues. - */ - bool makeImageCopy; - - PropertyTexturePropertyViewOptions() - : applyKhrTextureTransformExtension(false), makeImageCopy(false) {} -}; - template ElementType assembleScalarValue(const gsl::span bytes) noexcept { if constexpr (std::is_same_v) { @@ -231,12 +194,6 @@ ElementType assembleValueFromChannels(const gsl::span bytes) noexcept { } } -std::array sampleNearestPixel( - const ImageCesium& image, - const std::vector& channels, - const double u, - const double v); - #pragma region Non - normalized property /** @@ -266,13 +223,14 @@ class PropertyTexturePropertyView; */ template class PropertyTexturePropertyView - : public PropertyView { + : public PropertyView, public TextureView { public: /** * @brief Constructs an invalid instance for a non-existent property. */ PropertyTexturePropertyView() noexcept : PropertyView(), + TextureView(), _pSampler(nullptr), _pImage(nullptr), _texCoordSetIndex(0), @@ -288,13 +246,9 @@ class PropertyTexturePropertyView */ PropertyTexturePropertyView(PropertyViewStatusType status) noexcept : PropertyView(status), - _pSampler(nullptr), - _pImage(nullptr), - _texCoordSetIndex(0), + TextureView(), _channels(), - _swizzle(), - _applyTextureTransform(false), - _textureTransform(std::nullopt) { + _swizzle() { assert( this->_status != PropertyTexturePropertyViewStatus::Valid && "An empty property view should not be constructed with a valid status"); @@ -310,13 +264,9 @@ class PropertyTexturePropertyView */ PropertyTexturePropertyView(const ClassProperty& classProperty) noexcept : PropertyView(classProperty), - _pSampler(nullptr), - _pImage(nullptr), - _texCoordSetIndex(0), + TextureView(), _channels(), - _swizzle(), - _applyTextureTransform(false), - _textureTransform(std::nullopt) { + _swizzle() { if (this->_status != PropertyTexturePropertyViewStatus::Valid) { // Don't override the status / size if something is wrong with the class // property's definition. @@ -350,21 +300,43 @@ class PropertyTexturePropertyView const ClassProperty& classProperty, const Sampler& sampler, const ImageCesium& image, - PropertyTexturePropertyViewOptions options = - PropertyTexturePropertyViewOptions()) noexcept + TextureViewOptions options = TextureViewOptions()) noexcept : PropertyView(classProperty, property), - _pSampler(&sampler), - _pImage(nullptr), - _texCoordSetIndex(property.texCoord), + TextureView( + sampler, + image, + property.texCoord, + property.getExtension(), + options), _channels(property.channels), - _swizzle(), - _applyTextureTransform(options.applyKhrTextureTransformExtension), - _textureTransform(std::nullopt), - _imageCopy(std::nullopt) { + _swizzle() { if (this->_status != PropertyTexturePropertyViewStatus::Valid) { return; } + switch (this->getTextureViewStatus()) { + case TextureViewStatus::Valid: + break; + case TextureViewStatus::ErrorInvalidSampler: + this->_status = PropertyTexturePropertyViewStatus::ErrorInvalidSampler; + return; + case TextureViewStatus::ErrorInvalidImage: + this->_status = PropertyTexturePropertyViewStatus::ErrorInvalidImage; + return; + case TextureViewStatus::ErrorEmptyImage: + this->_status = PropertyTexturePropertyViewStatus::ErrorEmptyImage; + return; + case TextureViewStatus::ErrorInvalidBytesPerChannel: + this->_status = + PropertyTexturePropertyViewStatus::ErrorInvalidBytesPerChannel; + return; + case TextureViewStatus::ErrorUninitialized: + case TextureViewStatus::ErrorInvalidTexture: + default: + this->_status = PropertyTexturePropertyViewStatus::ErrorInvalidTexture; + return; + } + _swizzle.reserve(_channels.size()); for (size_t i = 0; i < _channels.size(); ++i) { @@ -385,19 +357,6 @@ class PropertyTexturePropertyView assert(false && "A valid channels vector must be passed to the view."); } } - - const ExtensionKhrTextureTransform* pTextureTransform = - property.getExtension(); - - if (pTextureTransform) { - this->_textureTransform = KhrTextureTransform(*pTextureTransform); - } - - if (options.makeImageCopy) { - this->_imageCopy = ImageCesium(image); - } else { - this->_pImage = ℑ - } } /** @@ -459,62 +418,13 @@ class PropertyTexturePropertyView this->_status == PropertyTexturePropertyViewStatus::Valid && "Check the status() first to make sure view is valid"); - if (this->_applyTextureTransform && this->_textureTransform) { - glm::dvec2 transformedUvs = this->_textureTransform->applyTransform(u, v); - u = transformedUvs.x; - v = transformedUvs.y; - } - - u = applySamplerWrapS(u, this->_pSampler->wrapS); - v = applySamplerWrapT(v, this->_pSampler->wrapT); + std::vector sample = + this->sampleNearestPixel(u, v, this->_channels); - const ImageCesium& image = this->_imageCopy.value_or(*this->_pImage); - std::array sample = - sampleNearestPixel(image, this->_channels, u, v); return assembleValueFromChannels( gsl::span(sample.data(), this->_channels.size())); } - /** - * @brief Get the texture coordinate set index for this property. - * - * If this view was constructed with options.applyKhrTextureTransformExtension - * as true, and if the property texture property contains the - * `KHR_texture_transform` extension, this will return the value from the - * extension, as it is meant to override the original index. However, if the - * extension does not specify a TEXCOORD set index, then the original index of - * the property texture property is returned. - */ - int64_t getTexCoordSetIndex() const noexcept { - if (this->_applyTextureTransform && this->_textureTransform) { - return this->_textureTransform->getTexCoordSetIndex().value_or( - this->_texCoordSetIndex); - } - return this->_texCoordSetIndex; - } - - /** - * @brief Get the sampler describing how to sample the data from the - * property's texture. - * - * This will be nullptr if the property texture property view runs into - * problems during construction. - */ - const Sampler* getSampler() const noexcept { return this->_pSampler; } - - /** - * @brief Get the image containing this property's data. - * - * This will be nullptr if the property texture property view runs into - * problems during construction. - */ - const ImageCesium* getImage() const noexcept { - if (this->_imageCopy) { - return &(this->_imageCopy.value()); - } - return this->_pImage; - } - /** * @brief Gets the channels of this property texture property. */ @@ -527,33 +437,9 @@ class PropertyTexturePropertyView */ const std::string& getSwizzle() const noexcept { return this->_swizzle; } - /** - * @brief Get the KHR_texture_transform for this property texture property, - * if it exists. - * - * Even if this view was constructed with - * options.applyKhrTextureTransformExtension set to false, it will save the - * extension's values, and they may be retrieved through this function. - * - * If this view was constructed with applyKhrTextureTransformExtension set - * to true, any texture coordinates passed into `get` or `getRaw` will be - * automatically transformed, so there's no need to re-apply the transform - * here. - */ - std::optional getTextureTransform() const noexcept { - return this->_textureTransform; - } - private: - const Sampler* _pSampler; - const ImageCesium* _pImage; - int64_t _texCoordSetIndex; std::vector _channels; std::string _swizzle; - - bool _applyTextureTransform; - std::optional _textureTransform; - std::optional _imageCopy; }; #pragma endregion @@ -569,7 +455,7 @@ class PropertyTexturePropertyView */ template class PropertyTexturePropertyView - : public PropertyView { + : public PropertyView, public TextureView { private: using NormalizedType = typename TypeToNormalizedType::type; @@ -579,13 +465,9 @@ class PropertyTexturePropertyView */ PropertyTexturePropertyView() noexcept : PropertyView(), - _pSampler(nullptr), - _pImage(nullptr), - _texCoordSetIndex(0), + TextureView(), _channels(), - _swizzle(), - _applyTextureTransform(false), - _textureTransform(std::nullopt) {} + _swizzle() {} /** * @brief Constructs an invalid instance for an erroneous property. @@ -594,13 +476,9 @@ class PropertyTexturePropertyView */ PropertyTexturePropertyView(PropertyViewStatusType status) noexcept : PropertyView(status), - _pSampler(nullptr), - _pImage(nullptr), - _texCoordSetIndex(0), + TextureView(), _channels(), - _swizzle(), - _applyTextureTransform(false), - _textureTransform(std::nullopt) { + _swizzle() { assert( this->_status != PropertyTexturePropertyViewStatus::Valid && "An empty property view should not be constructed with a valid " @@ -618,13 +496,9 @@ class PropertyTexturePropertyView */ PropertyTexturePropertyView(const ClassProperty& classProperty) noexcept : PropertyView(classProperty), - _pSampler(nullptr), - _pImage(nullptr), - _texCoordSetIndex(0), + TextureView(), _channels(), - _swizzle(), - _applyTextureTransform(false), - _textureTransform(std::nullopt) { + _swizzle() { if (this->_status != PropertyTexturePropertyViewStatus::Valid) { // Don't override the status / size if something is wrong with the class // property's definition. @@ -658,20 +532,43 @@ class PropertyTexturePropertyView const ClassProperty& classProperty, const Sampler& sampler, const ImageCesium& image, - PropertyTexturePropertyViewOptions options = - PropertyTexturePropertyViewOptions()) noexcept + TextureViewOptions options = TextureViewOptions()) noexcept : PropertyView(classProperty, property), - _pSampler(&sampler), - _pImage(&image), - _texCoordSetIndex(property.texCoord), + TextureView( + sampler, + image, + property.texCoord, + property.getExtension(), + options), _channels(property.channels), - _swizzle(), - _applyTextureTransform(options.applyKhrTextureTransformExtension), - _textureTransform(std::nullopt) { + _swizzle() { if (this->_status != PropertyTexturePropertyViewStatus::Valid) { return; } + switch (this->getTextureViewStatus()) { + case TextureViewStatus::Valid: + break; + case TextureViewStatus::ErrorInvalidSampler: + this->_status = PropertyTexturePropertyViewStatus::ErrorInvalidSampler; + return; + case TextureViewStatus::ErrorInvalidImage: + this->_status = PropertyTexturePropertyViewStatus::ErrorInvalidImage; + return; + case TextureViewStatus::ErrorEmptyImage: + this->_status = PropertyTexturePropertyViewStatus::ErrorEmptyImage; + return; + case TextureViewStatus::ErrorInvalidBytesPerChannel: + this->_status = + PropertyTexturePropertyViewStatus::ErrorInvalidBytesPerChannel; + return; + case TextureViewStatus::ErrorUninitialized: + case TextureViewStatus::ErrorInvalidTexture: + default: + this->_status = PropertyTexturePropertyViewStatus::ErrorInvalidTexture; + return; + } + _swizzle.reserve(_channels.size()); for (size_t i = 0; i < _channels.size(); ++i) { switch (_channels[i]) { @@ -691,19 +588,6 @@ class PropertyTexturePropertyView assert(false && "A valid channels vector must be passed to the view."); } } - - const ExtensionKhrTextureTransform* pTextureTransform = - property.getExtension(); - - if (pTextureTransform) { - this->_textureTransform = KhrTextureTransform(*pTextureTransform); - } - - if (options.makeImageCopy) { - this->_imageCopy = ImageCesium(image); - } else { - this->_pImage = ℑ - } } /** @@ -792,62 +676,13 @@ class PropertyTexturePropertyView this->_status == PropertyTexturePropertyViewStatus::Valid && "Check the status() first to make sure view is valid"); - if (this->_applyTextureTransform && this->_textureTransform) { - glm::dvec2 transformedUvs = this->_textureTransform->applyTransform(u, v); - u = transformedUvs.x; - v = transformedUvs.y; - } - - u = applySamplerWrapS(u, this->_pSampler->wrapS); - v = applySamplerWrapT(v, this->_pSampler->wrapT); - - const ImageCesium& image = this->_imageCopy.value_or(*this->_pImage); - std::array sample = - sampleNearestPixel(image, this->_channels, u, v); + std::vector sample = + this->sampleNearestPixel(u, v, this->_channels); return assembleValueFromChannels( gsl::span(sample.data(), this->_channels.size())); } - /** - * @brief Get the texture coordinate set index for this property. - * If this view was constructed with options.applyKhrTextureTransformExtension - * as true, and if the property texture property contains the - * `KHR_texture_transform` extension, this will return the value from the - * extension, as it is meant to override the original index. However, if the - * extension does not specify a TEXCOORD set index, then the original index of - * the property texture property is returned. - */ - int64_t getTexCoordSetIndex() const noexcept { - if (this->_applyTextureTransform && this->_textureTransform) { - return this->_textureTransform->getTexCoordSetIndex().value_or( - this->_texCoordSetIndex); - } - return this->_texCoordSetIndex; - } - - /** - * @brief Get the sampler describing how to sample the data from the - * property's texture. - * - * This will be nullptr if the property texture property view runs into - * problems during construction. - */ - const Sampler* getSampler() const noexcept { return this->_pSampler; } - - /** - * @brief Get the image containing this property's data. - * - * This will be nullptr if the property texture property view runs into - * problems during construction. - */ - const ImageCesium* getImage() const noexcept { - if (this->_imageCopy) { - return &(this->_imageCopy.value()); - } - return this->_pImage; - } - /** * @brief Gets the channels of this property texture property. */ @@ -860,34 +695,9 @@ class PropertyTexturePropertyView */ const std::string& getSwizzle() const noexcept { return this->_swizzle; } - /** - * @brief Get the KHR_texture_transform for this property texture property, - * if it exists. - * - * Even if this view was constructed with - * options.applyKhrTextureTransformExtension set to false, it will save the - * extension's values, and they may be retrieved through this function. - * - * If this view was constructed with applyKhrTextureTransformExtension set - * to true, any texture coordinates passed into `get` or `getRaw` will be - * automatically transformed, so there's no need to re-apply the transform - * here. - */ - std::optional getTextureTransform() const noexcept { - return this->_textureTransform; - } - private: - const Sampler* _pSampler; - const ImageCesium* _pImage; - int64_t _texCoordSetIndex; std::vector _channels; std::string _swizzle; - - bool _applyTextureTransform; - std::optional _textureTransform; - - std::optional _imageCopy; }; #pragma endregion diff --git a/CesiumGltf/include/CesiumGltf/PropertyTextureView.h b/CesiumGltf/include/CesiumGltf/PropertyTextureView.h index f9cb4ed2d..b7940d9de 100644 --- a/CesiumGltf/include/CesiumGltf/PropertyTextureView.h +++ b/CesiumGltf/include/CesiumGltf/PropertyTextureView.h @@ -5,7 +5,7 @@ #include "CesiumGltf/ExtensionModelExtStructuralMetadata.h" #include "CesiumGltf/PropertyTexture.h" #include "CesiumGltf/PropertyTexturePropertyView.h" -#include "CesiumGltf/Texture.h" +#include "CesiumGltf/TextureView.h" #include "Model.h" namespace CesiumGltf { @@ -62,8 +62,7 @@ class PropertyTextureView { PropertyTextureView( const Model& model, const PropertyTexture& propertyTexture, - PropertyTexturePropertyViewOptions propertyOptions = - PropertyTexturePropertyViewOptions()) noexcept; + TextureViewOptions propertyOptions = TextureViewOptions()) noexcept; /** * @brief Gets the status of this property texture view. @@ -726,7 +725,7 @@ class PropertyTextureView { const PropertyTexture* _pPropertyTexture; const Class* _pClass; - PropertyTexturePropertyViewOptions _propertyOptions; + TextureViewOptions _propertyOptions; PropertyTextureViewStatus _status; }; } // namespace CesiumGltf diff --git a/CesiumGltf/include/CesiumGltf/TextureView.h b/CesiumGltf/include/CesiumGltf/TextureView.h new file mode 100644 index 000000000..776b5f0d8 --- /dev/null +++ b/CesiumGltf/include/CesiumGltf/TextureView.h @@ -0,0 +1,222 @@ +#pragma once + +#include "CesiumGltf/ImageCesium.h" +#include "CesiumGltf/KhrTextureTransform.h" +#include "CesiumGltf/Sampler.h" +#include "CesiumGltf/TextureInfo.h" + +#include + +namespace CesiumGltf { + +class Model; + +/** + * @brief Describes options for constructing a view on a glTF texture. + */ +struct TextureViewOptions { + /** + * @brief Whether to automatically apply the `KHR_texture_transform` extension + * to the texture view, if it exists. + * + * A glTF texture may contain the `KHR_texture_transform` extension, which + * transforms the texture coordinates used to sample the texture. The + * extension may also override the TEXCOORD set index that was specified by + * the original texture info. + * + * If a view is constructed with applyKhrTextureTransformExtension set to + * true, it should automatically apply the texture transform to any UV + * coordinates used to sample the texture. If the extension defines its own + * TEXCOORD set index, it will override the original value. + * + * Otherwise, if the flag is set to false, UVs will not be transformed and + * the original TEXCOORD set index will be preserved. The extension's values + * may still be retrieved using getTextureTransform, if desired. + */ + bool applyKhrTextureTransformExtension; + + /** + * @brief Whether to copy the input image. + * + * By default, a view is constructed on the input glTF image without copying + * its pixels. This can be problematic for clients that move or delete the + * original glTF model. When this flag is true, the view will manage its own + * copy of the pixel data to avoid such issues. + */ + bool makeImageCopy; + + TextureViewOptions() + : applyKhrTextureTransformExtension(false), makeImageCopy(false) {} +}; + +/** + * @brief Indicates the status of a texture view. + * + * The {@link TextureView} constructor always completes + * successfully. However it may not always reflect the actual content of the + * corresponding texture. This enumeration provides the reason. + */ +enum class TextureViewStatus { + /** + * @brief This texture view is valid and ready to use. + */ + Valid, + + /** + * @brief This texture view has not yet been initialized. + */ + ErrorUninitialized, + + /** + * @brief This texture view does not have a valid texture index. + */ + ErrorInvalidTexture, + + /** + * @brief This texture view does not have a valid sampler index. + */ + ErrorInvalidSampler, + + /** + * @brief This texture view does not have a valid image index. + */ + ErrorInvalidImage, + + /** + * @brief This texture is viewing an empty image. + */ + ErrorEmptyImage, + + /** + * @brief The image for this texture has channels that take up more than a + * byte. Only single-byte channels are supported. + */ + ErrorInvalidBytesPerChannel, +}; + +class TextureView { +public: + /** + * @brief Constructs an empty, uninitialized texture view. + */ + TextureView() noexcept; + + /** + * @brief Constructs a view of the texture specified by the given {@link TextureInfo}. + * + * @param model The glTF model in which to look for the texture's data. + * @param textureInfo The texture info to create a view for. + * @param options The options for constructing the view. + */ + TextureView( + const Model& model, + const TextureInfo& textureInfo, + TextureViewOptions options = TextureViewOptions()) noexcept; + + /** + * @brief Constructs a view of the texture specified by the given {@link Sampler} + * and {@link ImageCesium}. + * + * @param sampler The {@link Sampler} used by the texture. + * @param image The {@link ImageCesium} used by the texture. + * @param textureCoordinateSetIndex The set index for the `TEXCOORD_n` + * attribute used to sample this texture. + * @param pKhrTextureTransformExtension A pointer to the KHR_texture_transform + * extension on the texture, if it exists. + * @param options The options for constructing the view. + */ + TextureView( + const Sampler& sampler, + const ImageCesium& image, + int64_t textureCoordinateSetIndex, + const ExtensionKhrTextureTransform* pKhrTextureTransformExtension = + nullptr, + TextureViewOptions options = TextureViewOptions()) noexcept; + + /** + * @brief Get the status of this texture view. + * + * If invalid, it will not be safe to sample from this view. + */ + TextureViewStatus getTextureViewStatus() const noexcept { + return this->_textureViewStatus; + } + + /** + * @brief Get the texture coordinate set index for this view. + * If this view was constructed with options.applyKhrTextureTransformExtension + * as true, and if the texture contains the `KHR_texture_transform` extension, + * then this will return the value from the extension since it is meant to + * override the original index. However, if the extension does not specify a + * TEXCOORD set index, then the original index of the texture is returned. + */ + int64_t getTexCoordSetIndex() const noexcept { + if (this->_applyTextureTransform && this->_textureTransform) { + return this->_textureTransform->getTexCoordSetIndex().value_or( + this->_texCoordSetIndex); + } + return this->_texCoordSetIndex; + } + + /** + * @brief Get the sampler describing how to sample the data from the + * property's texture. + * + * This will be nullptr if the property texture property view runs into + * problems during construction. + */ + const Sampler* getSampler() const noexcept { return this->_pSampler; } + + /** + * @brief Get the image containing this property's data. + * + * This will be nullptr if the texture view runs into + * problems during construction. + */ + const ImageCesium* getImage() const noexcept { + if (this->_imageCopy) { + return &(this->_imageCopy.value()); + } + return this->_pImage; + } + + /** + * @brief Get the KHR_texture_transform for this texture if it exists. + * + * Even if this view was constructed with + * options.applyKhrTextureTransformExtension set to false, it will save the + * extension's values, and they may be retrieved through this function. + * + * If this view was constructed with applyKhrTextureTransformExtension set + * to true, any texture coordinates passed into `get` or `getRaw` will be + * automatically transformed, so there's no need to re-apply the transform + * here. + */ + std::optional getTextureTransform() const noexcept { + return this->_textureTransform; + } + + /** + * @brief Samples the image at the specified texture coordinates using NEAREST + * pixel filtering, returning the bytes as uint8_t values. A channels vector + * must be supplied to specify how many image channels are needed, and in what + * order the bytes should be retrieved. + */ + std::vector sampleNearestPixel( + double u, + double v, + const std::vector& channels) const noexcept; + +private: + TextureViewStatus _textureViewStatus; + + const Sampler* _pSampler; + const ImageCesium* _pImage; + int64_t _texCoordSetIndex; + + bool _applyTextureTransform; + std::optional _textureTransform; + + std::optional _imageCopy; +}; +} // namespace CesiumGltf diff --git a/CesiumGltf/src/PropertyTexturePropertyView.cpp b/CesiumGltf/src/PropertyTexturePropertyView.cpp index 1462b65a2..c9435dcca 100644 --- a/CesiumGltf/src/PropertyTexturePropertyView.cpp +++ b/CesiumGltf/src/PropertyTexturePropertyView.cpp @@ -22,46 +22,4 @@ const PropertyViewStatusType const PropertyViewStatusType PropertyTexturePropertyViewStatus::ErrorChannelsAndTypeMismatch; -std::array sampleNearestPixel( - const ImageCesium& image, - const std::vector& channels, - const double u, - const double v) { - // For nearest filtering, std::floor is used instead of std::round. - // This is because filtering is supposed to consider the pixel centers. But - // memory access here acts as sampling the beginning of the pixel. Example: - // 0.4 * 2 = 0.8. In a 2x1 pixel image, that should be closer to the left - // pixel's center. But it will round to 1.0 which corresponds to the right - // pixel. So the right pixel has a bigger range than the left one, which is - // incorrect. - double xCoord = std::floor(u * image.width); - double yCoord = std::floor(v * image.height); - - // Clamp to ensure no out-of-bounds data access - int64_t x = glm::clamp( - static_cast(xCoord), - static_cast(0), - static_cast(image.width) - 1); - int64_t y = glm::clamp( - static_cast(yCoord), - static_cast(0), - static_cast(image.height) - 1); - - int64_t pixelIndex = - image.bytesPerChannel * image.channels * (y * image.width + x); - - // TODO: Currently stb only outputs uint8 pixel types. If that - // changes this should account for additional pixel byte sizes. - const uint8_t* pValue = - reinterpret_cast(image.pixelData.data() + pixelIndex); - - std::array channelValues{0, 0, 0, 0}; - size_t len = glm::min(channels.size(), channelValues.size()); - for (size_t i = 0; i < len; i++) { - channelValues[i] = *(pValue + channels[i]); - } - - return channelValues; -} - } // namespace CesiumGltf diff --git a/CesiumGltf/src/PropertyTextureView.cpp b/CesiumGltf/src/PropertyTextureView.cpp index 0a2c16934..b774e27b6 100644 --- a/CesiumGltf/src/PropertyTextureView.cpp +++ b/CesiumGltf/src/PropertyTextureView.cpp @@ -4,7 +4,7 @@ namespace CesiumGltf { PropertyTextureView::PropertyTextureView( const Model& model, const PropertyTexture& propertyTexture, - PropertyTexturePropertyViewOptions options) noexcept + TextureViewOptions options) noexcept : _pModel(&model), _pPropertyTexture(&propertyTexture), _pClass(nullptr), diff --git a/CesiumGltf/src/TextureView.cpp b/CesiumGltf/src/TextureView.cpp new file mode 100644 index 000000000..8a84a45ee --- /dev/null +++ b/CesiumGltf/src/TextureView.cpp @@ -0,0 +1,168 @@ +#include "CesiumGltf/TextureView.h" + +#include "CesiumGltf/Model.h" +#include "CesiumGltf/SamplerUtility.h" + +namespace CesiumGltf { + +TextureView::TextureView() noexcept + : _textureViewStatus(TextureViewStatus::ErrorUninitialized), + _pSampler(nullptr), + _pImage(nullptr), + _texCoordSetIndex(-1), + _applyTextureTransform(false), + _textureTransform(std::nullopt), + _imageCopy(std::nullopt) {} + +TextureView::TextureView( + const Model& model, + const TextureInfo& textureInfo, + TextureViewOptions options) noexcept + : _textureViewStatus(TextureViewStatus::ErrorUninitialized), + _pSampler(nullptr), + _pImage(nullptr), + _texCoordSetIndex(textureInfo.texCoord), + _applyTextureTransform(options.applyKhrTextureTransformExtension), + _textureTransform(std::nullopt), + _imageCopy(std::nullopt) { + int32_t textureIndex = textureInfo.index; + if (textureIndex < 0 || + static_cast(textureIndex) >= model.textures.size()) { + this->_textureViewStatus = TextureViewStatus::ErrorInvalidTexture; + return; + } + + const Texture& texture = model.textures[static_cast(textureIndex)]; + if (texture.source < 0 || + static_cast(texture.source) >= model.images.size()) { + this->_textureViewStatus = TextureViewStatus::ErrorInvalidImage; + return; + } + + this->_pImage = &model.images[static_cast(texture.source)].cesium; + if (this->_pImage->width < 1 || this->_pImage->height < 1) { + this->_textureViewStatus = TextureViewStatus::ErrorEmptyImage; + return; + } + + if (texture.sampler < 0 || + static_cast(texture.sampler) >= model.samplers.size()) { + this->_textureViewStatus = TextureViewStatus::ErrorInvalidSampler; + return; + } + + this->_pSampler = &model.samplers[static_cast(texture.sampler)]; + + // TODO: once compressed texture support is merged, check that the image is + // decompressed here. + + if (this->_pImage->bytesPerChannel > 1) { + this->_textureViewStatus = + TextureViewStatus::ErrorInvalidBytesPerChannel; + return; + } + + const ExtensionKhrTextureTransform* pTextureTransform = + textureInfo.getExtension(); + + if (pTextureTransform) { + this->_textureTransform = KhrTextureTransform(*pTextureTransform); + } + + if (options.makeImageCopy) { + this->_imageCopy = ImageCesium(*this->_pImage); + } + + this->_textureViewStatus = TextureViewStatus::Valid; +} + +TextureView::TextureView( + const Sampler& sampler, + const ImageCesium& image, + int64_t textureCoordinateSetIndex, + const ExtensionKhrTextureTransform* pKhrTextureTransformExtension, + TextureViewOptions options) noexcept + : _textureViewStatus(TextureViewStatus::ErrorUninitialized), + _pSampler(&sampler), + _pImage(&image), + _texCoordSetIndex(textureCoordinateSetIndex), + _applyTextureTransform(options.applyKhrTextureTransformExtension), + _textureTransform(std::nullopt), + _imageCopy(std::nullopt) { + // TODO: once compressed texture support is merged, check that the image is + // decompressed here. + + if (this->_pImage->bytesPerChannel > 1) { + this->_textureViewStatus = + TextureViewStatus::ErrorInvalidBytesPerChannel; + return; + } + + if (pKhrTextureTransformExtension) { + this->_textureTransform = + KhrTextureTransform(*pKhrTextureTransformExtension); + } + + if (options.makeImageCopy) { + this->_imageCopy = ImageCesium(*this->_pImage); + } + + this->_textureViewStatus = TextureViewStatus::Valid; +} + +std::vector TextureView::sampleNearestPixel( + double u, + double v, + const std::vector& channels) const noexcept { + assert(this->_textureViewStatus == TextureViewStatus::Valid); + std::vector result(channels.size()); + + if (channels.size() == 0) { + return result; + } + + if (this->_applyTextureTransform && this->_textureTransform) { + glm::dvec2 transformedUvs = this->_textureTransform->applyTransform(u, v); + u = transformedUvs.x; + v = transformedUvs.y; + } + + u = applySamplerWrapS(u, this->_pSampler->wrapS); + v = applySamplerWrapT(v, this->_pSampler->wrapT); + + const ImageCesium& image = this->_imageCopy.value_or(*this->_pImage); + + // For nearest filtering, std::floor is used instead of std::round. + // This is because filtering is supposed to consider the pixel centers. But + // memory access here acts as sampling the beginning of the pixel. Example: + // 0.4 * 2 = 0.8. In a 2x1 pixel image, that should be closer to the left + // pixel's center. But it will round to 1.0 which corresponds to the right + // pixel. So the right pixel has a bigger range than the left one, which is + // incorrect. + double xCoord = std::floor(u * image.width); + double yCoord = std::floor(v * image.height); + + // Clamp to ensure no out-of-bounds data access + int64_t x = glm::clamp( + static_cast(xCoord), + static_cast(0), + static_cast(image.width) - 1); + int64_t y = glm::clamp( + static_cast(yCoord), + static_cast(0), + static_cast(image.height) - 1); + + int64_t pixelIndex = + image.bytesPerChannel * image.channels * (y * image.width + x); + + // TODO: Currently stb only outputs uint8 pixel types. If that + // changes this should account for additional pixel byte sizes. + const uint8_t* pValue = + reinterpret_cast(image.pixelData.data() + pixelIndex); + for (size_t i = 0; i < result.size(); i++) { + result[i] = *(pValue + channels[i]); + } + + return result; +} +} // namespace CesiumGltf diff --git a/CesiumGltf/test/TestPropertyTexturePropertyView.cpp b/CesiumGltf/test/TestPropertyTexturePropertyView.cpp index 02b486e08..76e6300ae 100644 --- a/CesiumGltf/test/TestPropertyTexturePropertyView.cpp +++ b/CesiumGltf/test/TestPropertyTexturePropertyView.cpp @@ -1805,7 +1805,7 @@ TEST_CASE("Test PropertyTextureProperty constructs with " property.channels = {0}; - PropertyTexturePropertyViewOptions options; + TextureViewOptions options; options.applyKhrTextureTransformExtension = true; PropertyTexturePropertyView @@ -1878,7 +1878,7 @@ TEST_CASE("Test normalized PropertyTextureProperty constructs with " property.channels = {0}; - PropertyTexturePropertyViewOptions options; + TextureViewOptions options; options.applyKhrTextureTransformExtension = true; PropertyTexturePropertyView @@ -1952,7 +1952,7 @@ TEST_CASE("Test PropertyTextureProperty constructs with makeImageCopy = true") { property.channels = {0}; - PropertyTexturePropertyViewOptions options; + TextureViewOptions options; options.makeImageCopy = true; PropertyTexturePropertyView @@ -2014,7 +2014,7 @@ TEST_CASE("Test normalized PropertyTextureProperty constructs with " property.channels = {0}; - PropertyTexturePropertyViewOptions options; + TextureViewOptions options; options.makeImageCopy = true; PropertyTexturePropertyView From cc1f333b3329af382a04b3b8e58e4cf349097f22 Mon Sep 17 00:00:00 2001 From: Janine Liu Date: Tue, 27 Feb 2024 18:17:43 -0500 Subject: [PATCH 04/12] Use TextureView for feature ID textures --- .../include/CesiumGltf/FeatureIdTextureView.h | 68 +---------- .../CesiumGltf/PropertyTexturePropertyView.h | 1 - CesiumGltf/src/FeatureIdTextureView.cpp | 110 ++++------------- CesiumGltf/test/TestFeatureIdTextureView.cpp | 114 +++++++++++++++++- .../test/TestPropertyTexturePropertyView.cpp | 25 ++-- 5 files changed, 160 insertions(+), 158 deletions(-) diff --git a/CesiumGltf/include/CesiumGltf/FeatureIdTextureView.h b/CesiumGltf/include/CesiumGltf/FeatureIdTextureView.h index fb6cfd094..ce61a689b 100644 --- a/CesiumGltf/include/CesiumGltf/FeatureIdTextureView.h +++ b/CesiumGltf/include/CesiumGltf/FeatureIdTextureView.h @@ -4,8 +4,8 @@ #include "CesiumGltf/Image.h" #include "CesiumGltf/ImageCesium.h" #include "CesiumGltf/KhrTextureTransform.h" -#include "CesiumGltf/Model.h" #include "CesiumGltf/Texture.h" +#include "CesiumGltf/TextureView.h" #include #include @@ -14,6 +14,9 @@ #include namespace CesiumGltf { + +class Model; + /** * @brief The status of a feature ID texture view. * @@ -76,7 +79,7 @@ enum class FeatureIdTextureViewStatus { * It provides the ability to sample the feature IDs from the * {@link FeatureIdTexture} using texture coordinates. */ -class FeatureIdTextureView { +class FeatureIdTextureView : public TextureView { public: /** * @brief Constructs an uninitialized and invalid view. @@ -108,7 +111,7 @@ class FeatureIdTextureView { FeatureIdTextureView( const Model& model, const FeatureIdTexture& featureIdTexture, - bool applyKhrTextureTransformExtension = false) noexcept; + TextureViewOptions options = TextureViewOptions()) noexcept; /** * @brief Get the feature ID from the texture at the given texture @@ -129,73 +132,14 @@ class FeatureIdTextureView { */ FeatureIdTextureViewStatus status() const noexcept { return this->_status; } - /** - * @brief Get the actual feature ID texture. - * - * This will be nullptr if the feature ID texture view runs into problems - * during construction. - */ - const ImageCesium* getImage() const noexcept { return this->_pImage; } - - /** - * @brief Get the sampler describing how to sample the data from the - * feature ID texture. - * - * This will be nullptr if the feature ID texture view runs into - * problems during construction. - */ - const Sampler* getSampler() const noexcept { return this->_pSampler; } - /** * @brief Get the channels of this feature ID texture. The channels represent * the bytes of the actual feature ID, in little-endian order. */ std::vector getChannels() const noexcept { return this->_channels; } - /** - * @brief Get the texture coordinate set index for this feature ID - * texture. - * - * If applyKhrTextureTransformExtension is true, and if the feature ID texture - * contains the `KHR_texture_transform` extension, this will return the value - * from the extension, as it is meant to override the original index. However, - * if the extension does not specify a TEXCOORD set index, then the original - * index of the feature ID texture is returned. - */ - int64_t getTexCoordSetIndex() const noexcept { - if (this->_applyTextureTransform && this->_textureTransform) { - return this->_textureTransform->getTexCoordSetIndex().value_or( - this->_texCoordSetIndex); - } - return this->_texCoordSetIndex; - } - - /** - * @brief Get the KHR_texture_transform for this feature ID texture, if it - * exists. - * - * Even if this view was constructed with applyKhrTextureTransformExtension - * set to false, it will save the extension's values, and they may be - * retrieved through this function. - * - * If this view was constructed with applyKhrTextureTransformExtension set to - * true, any texture coordinates passed into `getFeatureID` will be - * automatically transformed, so there's no need to re-apply the transform - * here. - */ - std::optional getTextureTransform() const noexcept { - return this->_textureTransform; - } - private: FeatureIdTextureViewStatus _status; - int64_t _texCoordSetIndex; std::vector _channels; - - const ImageCesium* _pImage; - const Sampler* _pSampler; - - bool _applyTextureTransform; - std::optional _textureTransform; }; } // namespace CesiumGltf diff --git a/CesiumGltf/include/CesiumGltf/PropertyTexturePropertyView.h b/CesiumGltf/include/CesiumGltf/PropertyTexturePropertyView.h index 685408c7f..951cad5d8 100644 --- a/CesiumGltf/include/CesiumGltf/PropertyTexturePropertyView.h +++ b/CesiumGltf/include/CesiumGltf/PropertyTexturePropertyView.h @@ -7,7 +7,6 @@ #include "CesiumGltf/PropertyTypeTraits.h" #include "CesiumGltf/PropertyView.h" #include "CesiumGltf/Sampler.h" -#include "CesiumGltf/SamplerUtility.h" #include "CesiumGltf/TextureView.h" #include diff --git a/CesiumGltf/src/FeatureIdTextureView.cpp b/CesiumGltf/src/FeatureIdTextureView.cpp index 0eb66b203..58c6a2cf1 100644 --- a/CesiumGltf/src/FeatureIdTextureView.cpp +++ b/CesiumGltf/src/FeatureIdTextureView.cpp @@ -1,69 +1,48 @@ #include "CesiumGltf/FeatureIdTextureView.h" #include "CesiumGltf/ExtensionKhrTextureTransform.h" +#include "CesiumGltf/Model.h" #include "CesiumGltf/SamplerUtility.h" namespace CesiumGltf { FeatureIdTextureView::FeatureIdTextureView() noexcept - : _status(FeatureIdTextureViewStatus::ErrorUninitialized), - _texCoordSetIndex(0), - _channels(), - _pImage(nullptr), - _pSampler(nullptr), - _applyTextureTransform(false), - _textureTransform(std::nullopt) {} + : TextureView(), + _status(FeatureIdTextureViewStatus::ErrorUninitialized), + _channels() {} FeatureIdTextureView::FeatureIdTextureView( const Model& model, const FeatureIdTexture& featureIdTexture, - bool applyKhrTextureTransform) noexcept - : _status(FeatureIdTextureViewStatus::ErrorUninitialized), - _texCoordSetIndex(featureIdTexture.texCoord), - _channels(), - _pImage(nullptr), - _pSampler(nullptr), - _applyTextureTransform(applyKhrTextureTransform), - _textureTransform(std::nullopt) { - int32_t textureIndex = featureIdTexture.index; - if (textureIndex < 0 || - static_cast(textureIndex) >= model.textures.size()) { - this->_status = FeatureIdTextureViewStatus::ErrorInvalidTexture; + TextureViewOptions options) noexcept + : TextureView(model, featureIdTexture, options), + _status(FeatureIdTextureViewStatus::ErrorUninitialized), + _channels() { + switch (this->getTextureViewStatus()) { + case TextureViewStatus::Valid: + break; + case TextureViewStatus::ErrorInvalidSampler: + this->_status = FeatureIdTextureViewStatus::ErrorInvalidSampler; return; - } - - const Texture& texture = model.textures[static_cast(textureIndex)]; - if (texture.source < 0 || - static_cast(texture.source) >= model.images.size()) { + case TextureViewStatus::ErrorInvalidImage: this->_status = FeatureIdTextureViewStatus::ErrorInvalidImage; return; - } - - this->_pImage = &model.images[static_cast(texture.source)].cesium; - if (this->_pImage->width < 1 || this->_pImage->height < 1) { + case TextureViewStatus::ErrorEmptyImage: this->_status = FeatureIdTextureViewStatus::ErrorEmptyImage; return; - } - - if (texture.sampler < 0 || - static_cast(texture.sampler) >= model.samplers.size()) { - this->_status = FeatureIdTextureViewStatus::ErrorInvalidSampler; - return; - } - - this->_pSampler = &model.samplers[static_cast(texture.sampler)]; - - // TODO: once compressed texture support is merged, check that the image is - // decompressed here. - - if (this->_pImage->bytesPerChannel > 1) { + case TextureViewStatus::ErrorInvalidBytesPerChannel: this->_status = FeatureIdTextureViewStatus::ErrorInvalidImageBytesPerChannel; return; + case TextureViewStatus::ErrorUninitialized: + case TextureViewStatus::ErrorInvalidTexture: + default: + this->_status = FeatureIdTextureViewStatus::ErrorInvalidTexture; + return; } const std::vector& channels = featureIdTexture.channels; if (channels.size() == 0 || channels.size() > 4 || - channels.size() > static_cast(this->_pImage->channels)) { + channels.size() > static_cast(this->getImage()->channels)) { this->_status = FeatureIdTextureViewStatus::ErrorInvalidChannels; return; } @@ -75,15 +54,9 @@ FeatureIdTextureView::FeatureIdTextureView( return; } } - this->_channels = channels; + this->_channels = channels; this->_status = FeatureIdTextureViewStatus::Valid; - - const ExtensionKhrTextureTransform* pTextureTransform = - featureIdTexture.getExtension(); - if (pTextureTransform) { - this->_textureTransform = KhrTextureTransform(*pTextureTransform); - } } // namespace CesiumGltf int64_t FeatureIdTextureView::getFeatureID(double u, double v) const noexcept { @@ -91,38 +64,7 @@ int64_t FeatureIdTextureView::getFeatureID(double u, double v) const noexcept { return -1; } - if (this->_applyTextureTransform && this->_textureTransform) { - glm::dvec2 transformedUvs = this->_textureTransform->applyTransform(u, v); - u = transformedUvs.x; - v = transformedUvs.y; - } - - u = applySamplerWrapS(u, this->_pSampler->wrapS); - v = applySamplerWrapT(v, this->_pSampler->wrapT); - - // Always use nearest filtering, and use std::floor instead of std::round. - // This is because filtering is supposed to consider the pixel centers. But - // memory access here acts as sampling the beginning of the pixel. Example: - // 0.4 * 2 = 0.8. In a 2x1 pixel image, that should be closer to the left - // pixel's center. But it will round to 1.0 which corresponds to the right - // pixel. So the right pixel has a bigger range than the left one, which is - // incorrect. - double xCoord = std::floor(u * this->_pImage->width); - double yCoord = std::floor(v * this->_pImage->height); - - // Clamp to ensure no out-of-bounds data access - int64_t x = glm::clamp( - static_cast(xCoord), - static_cast(0), - static_cast(this->_pImage->width - 1)); - int64_t y = glm::clamp( - static_cast(yCoord), - static_cast(0), - static_cast(this->_pImage->height - 1)); - - int64_t pixelOffset = this->_pImage->bytesPerChannel * - this->_pImage->channels * - (y * this->_pImage->width + x); + std::vector sample = this->sampleNearestPixel(u, v, this->_channels); int64_t value = 0; int64_t bitOffset = 0; @@ -130,9 +72,7 @@ int64_t FeatureIdTextureView::getFeatureID(double u, double v) const noexcept { // unsigned 8 bit integers, and represent the bytes of the actual feature ID, // in little-endian order. for (size_t i = 0; i < this->_channels.size(); i++) { - int64_t channelValue = static_cast( - this->_pImage - ->pixelData[static_cast(pixelOffset + this->_channels[i])]); + int64_t channelValue = static_cast(sample[i]); value |= channelValue << bitOffset; bitOffset += 8; } diff --git a/CesiumGltf/test/TestFeatureIdTextureView.cpp b/CesiumGltf/test/TestFeatureIdTextureView.cpp index 4544c500f..2b1857e81 100644 --- a/CesiumGltf/test/TestFeatureIdTextureView.cpp +++ b/CesiumGltf/test/TestFeatureIdTextureView.cpp @@ -1,6 +1,7 @@ #include "CesiumGltf/ExtensionExtMeshFeatures.h" #include "CesiumGltf/FeatureIdTextureView.h" #include "CesiumGltf/KhrTextureTransform.h" +#include "CesiumGltf/Model.h" #include "CesiumUtility/Math.h" #include @@ -342,7 +343,9 @@ TEST_CASE("Test FeatureIdTextureView with applyKhrTextureTransformExtension = " FeatureId featureId = meshFeatures.featureIds.emplace_back(); featureId.texture = featureIdTexture; - FeatureIdTextureView view(model, featureIdTexture, true); + TextureViewOptions options; + options.applyKhrTextureTransformExtension = true; + FeatureIdTextureView view(model, featureIdTexture, options); REQUIRE(view.status() == FeatureIdTextureViewStatus::Valid); auto textureTransform = view.getTextureTransform(); @@ -357,6 +360,60 @@ TEST_CASE("Test FeatureIdTextureView with applyKhrTextureTransformExtension = " REQUIRE(textureTransform->getTexCoordSetIndex() == 10); } +TEST_CASE("Test FeatureIdTextureView with makeImageCopy = true") { + std::vector featureIDs{1, 2, 0, 7}; + + Model model; + Mesh& mesh = model.meshes.emplace_back(); + MeshPrimitive& primitive = mesh.primitives.emplace_back(); + Sampler& sampler = model.samplers.emplace_back(); + sampler.wrapS = Sampler::WrapS::CLAMP_TO_EDGE; + sampler.wrapT = Sampler::WrapT::CLAMP_TO_EDGE; + + Image& image = model.images.emplace_back(); + image.cesium.width = 2; + image.cesium.height = 2; + image.cesium.channels = 1; + image.cesium.bytesPerChannel = 1; + image.cesium.pixelData.resize(featureIDs.size()); + std::memcpy( + image.cesium.pixelData.data(), + featureIDs.data(), + featureIDs.size()); + + Texture& texture = model.textures.emplace_back(); + texture.sampler = 0; + texture.source = 0; + + ExtensionExtMeshFeatures& meshFeatures = + primitive.addExtension(); + + FeatureIdTexture featureIdTexture; + featureIdTexture.index = 0; + featureIdTexture.texCoord = 0; + featureIdTexture.channels = {0}; + + FeatureId featureId = meshFeatures.featureIds.emplace_back(); + featureId.texture = featureIdTexture; + + TextureViewOptions options; + options.makeImageCopy = true; + FeatureIdTextureView view(model, featureIdTexture, options); + REQUIRE(view.status() == FeatureIdTextureViewStatus::Valid); + + // Clear the original image data. + std::vector emptyData; + image.cesium.pixelData.swap(emptyData); + + const ImageCesium* pImage = view.getImage(); + REQUIRE(pImage); + REQUIRE(pImage->width == image.cesium.width); + REQUIRE(pImage->height == image.cesium.height); + REQUIRE(pImage->channels == image.cesium.channels); + REQUIRE(pImage->bytesPerChannel == image.cesium.bytesPerChannel); + REQUIRE(pImage->pixelData.size() == featureIDs.size()); +} + TEST_CASE("Test getFeatureID on invalid feature ID texture view") { Model model; Mesh& mesh = model.meshes.emplace_back(); @@ -527,7 +584,9 @@ TEST_CASE( FeatureId featureId = meshFeatures.featureIds.emplace_back(); featureId.texture = featureIdTexture; - FeatureIdTextureView view(model, featureIdTexture, true); + TextureViewOptions options; + options.applyKhrTextureTransformExtension = true; + FeatureIdTextureView view(model, featureIdTexture, options); REQUIRE(view.status() == FeatureIdTextureViewStatus::Valid); // (0, 0) -> (0.5, -0.5) -> wraps to (0.5, 0.5) // (1, 0) -> (0.5, -1) -> wraps to (0.5, 0) @@ -539,6 +598,57 @@ TEST_CASE( REQUIRE(view.getFeatureID(1, 1) == 1); } +TEST_CASE("Test getFeatureId on view with makeImageCopy = true") { + std::vector featureIDs{1, 2, 0, 7}; + + Model model; + Mesh& mesh = model.meshes.emplace_back(); + MeshPrimitive& primitive = mesh.primitives.emplace_back(); + Sampler& sampler = model.samplers.emplace_back(); + sampler.wrapS = Sampler::WrapS::CLAMP_TO_EDGE; + sampler.wrapT = Sampler::WrapT::CLAMP_TO_EDGE; + + Image& image = model.images.emplace_back(); + image.cesium.width = 2; + image.cesium.height = 2; + image.cesium.channels = 1; + image.cesium.bytesPerChannel = 1; + image.cesium.pixelData.resize(featureIDs.size()); + std::memcpy( + image.cesium.pixelData.data(), + featureIDs.data(), + featureIDs.size()); + + Texture& texture = model.textures.emplace_back(); + texture.sampler = 0; + texture.source = 0; + + ExtensionExtMeshFeatures& meshFeatures = + primitive.addExtension(); + + FeatureIdTexture featureIdTexture; + featureIdTexture.index = 0; + featureIdTexture.texCoord = 0; + featureIdTexture.channels = {0}; + + FeatureId featureId = meshFeatures.featureIds.emplace_back(); + featureId.texture = featureIdTexture; + + TextureViewOptions options; + options.makeImageCopy = true; + FeatureIdTextureView view(model, featureIdTexture, options); + REQUIRE(view.status() == FeatureIdTextureViewStatus::Valid); + + // Clear the original image data. + std::vector emptyData; + image.cesium.pixelData.swap(emptyData); + + REQUIRE(view.getFeatureID(0, 0) == 1); + REQUIRE(view.getFeatureID(1, 0) == 2); + REQUIRE(view.getFeatureID(0, 1) == 0); + REQUIRE(view.getFeatureID(1, 1) == 7); +} + TEST_CASE("Test getFeatureID rounds to nearest pixel") { Model model; Mesh& mesh = model.meshes.emplace_back(); diff --git a/CesiumGltf/test/TestPropertyTexturePropertyView.cpp b/CesiumGltf/test/TestPropertyTexturePropertyView.cpp index 76e6300ae..ae3a815fb 100644 --- a/CesiumGltf/test/TestPropertyTexturePropertyView.cpp +++ b/CesiumGltf/test/TestPropertyTexturePropertyView.cpp @@ -1918,8 +1918,9 @@ TEST_CASE("Test normalized PropertyTextureProperty constructs with " } } -TEST_CASE("Test PropertyTextureProperty constructs with makeImageCopy = true") { - std::vector data{0, 64, 127, 255}; +TEST_CASE("Test PropertyTextureProperty constructs with " + "makeImageCopy = true") { + std::vector data{1, 2, 3, 4}; PropertyTextureProperty property; property.texCoord = 0; @@ -1934,7 +1935,6 @@ TEST_CASE("Test PropertyTextureProperty constructs with makeImageCopy = true") { ClassProperty classProperty; classProperty.type = ClassProperty::Type::SCALAR; classProperty.componentType = ClassProperty::ComponentType::UINT8; - classProperty.normalized = true; Sampler sampler; sampler.wrapS = Sampler::WrapS::REPEAT; @@ -1955,7 +1955,7 @@ TEST_CASE("Test PropertyTextureProperty constructs with makeImageCopy = true") { TextureViewOptions options; options.makeImageCopy = true; - PropertyTexturePropertyView + PropertyTexturePropertyView view(property, classProperty, sampler, image, options); REQUIRE(view.status() == PropertyTexturePropertyViewStatus::Valid); @@ -1965,6 +1965,10 @@ TEST_CASE("Test PropertyTextureProperty constructs with makeImageCopy = true") { const ImageCesium* pImage = view.getImage(); REQUIRE(pImage); + REQUIRE(pImage->width == image.width); + REQUIRE(pImage->height == image.height); + REQUIRE(pImage->channels == image.channels); + REQUIRE(pImage->bytesPerChannel == image.bytesPerChannel); REQUIRE(pImage->pixelData.size() == data.size()); std::vector texCoords{ @@ -1976,13 +1980,13 @@ TEST_CASE("Test PropertyTextureProperty constructs with makeImageCopy = true") { for (size_t i = 0; i < texCoords.size(); i++) { glm::dvec2 uv = texCoords[i]; REQUIRE(view.getRaw(uv[0], uv[1]) == data[i]); - REQUIRE(view.get(uv[0], uv[1]) == static_cast(data[i]) / 255.0); + REQUIRE(view.get(uv[0], uv[1]) == data[i]); } } TEST_CASE("Test normalized PropertyTextureProperty constructs with " "makeImageCopy = true") { - std::vector data{1, 2, 3, 4}; + std::vector data{0, 64, 127, 255}; PropertyTextureProperty property; property.texCoord = 0; @@ -1997,6 +2001,7 @@ TEST_CASE("Test normalized PropertyTextureProperty constructs with " ClassProperty classProperty; classProperty.type = ClassProperty::Type::SCALAR; classProperty.componentType = ClassProperty::ComponentType::UINT8; + classProperty.normalized = true; Sampler sampler; sampler.wrapS = Sampler::WrapS::REPEAT; @@ -2017,7 +2022,7 @@ TEST_CASE("Test normalized PropertyTextureProperty constructs with " TextureViewOptions options; options.makeImageCopy = true; - PropertyTexturePropertyView + PropertyTexturePropertyView view(property, classProperty, sampler, image, options); REQUIRE(view.status() == PropertyTexturePropertyViewStatus::Valid); @@ -2027,6 +2032,10 @@ TEST_CASE("Test normalized PropertyTextureProperty constructs with " const ImageCesium* pImage = view.getImage(); REQUIRE(pImage); + REQUIRE(pImage->width == image.width); + REQUIRE(pImage->height == image.height); + REQUIRE(pImage->channels == image.channels); + REQUIRE(pImage->bytesPerChannel == image.bytesPerChannel); REQUIRE(pImage->pixelData.size() == data.size()); std::vector texCoords{ @@ -2038,6 +2047,6 @@ TEST_CASE("Test normalized PropertyTextureProperty constructs with " for (size_t i = 0; i < texCoords.size(); i++) { glm::dvec2 uv = texCoords[i]; REQUIRE(view.getRaw(uv[0], uv[1]) == data[i]); - REQUIRE(view.get(uv[0], uv[1]) == data[i]); + REQUIRE(view.get(uv[0], uv[1]) == static_cast(data[i]) / 255.0); } } From 0ca2d45056066c94785c4c70aeefb8d7eca57d7f Mon Sep 17 00:00:00 2001 From: Janine Liu Date: Tue, 27 Feb 2024 18:22:49 -0500 Subject: [PATCH 05/12] class -> struct --- CesiumGltf/include/CesiumGltf/FeatureIdTextureView.h | 2 +- CesiumGltf/include/CesiumGltf/TextureView.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CesiumGltf/include/CesiumGltf/FeatureIdTextureView.h b/CesiumGltf/include/CesiumGltf/FeatureIdTextureView.h index ce61a689b..f4fb54c2d 100644 --- a/CesiumGltf/include/CesiumGltf/FeatureIdTextureView.h +++ b/CesiumGltf/include/CesiumGltf/FeatureIdTextureView.h @@ -15,7 +15,7 @@ namespace CesiumGltf { -class Model; +struct Model; /** * @brief The status of a feature ID texture view. diff --git a/CesiumGltf/include/CesiumGltf/TextureView.h b/CesiumGltf/include/CesiumGltf/TextureView.h index 776b5f0d8..c5f7d2a99 100644 --- a/CesiumGltf/include/CesiumGltf/TextureView.h +++ b/CesiumGltf/include/CesiumGltf/TextureView.h @@ -9,7 +9,7 @@ namespace CesiumGltf { -class Model; +struct Model; /** * @brief Describes options for constructing a view on a glTF texture. From 907f5aab2fc8f2f99e2c4f72b00d450b0c20d17d Mon Sep 17 00:00:00 2001 From: Janine Liu Date: Wed, 28 Feb 2024 11:29:23 -0500 Subject: [PATCH 06/12] Make property options a parameter --- .../CesiumGltf/PropertyTexturePropertyView.h | 10 +- .../include/CesiumGltf/PropertyTextureView.h | 176 ++++++++++++------ CesiumGltf/src/PropertyTextureView.cpp | 4 +- 3 files changed, 121 insertions(+), 69 deletions(-) diff --git a/CesiumGltf/include/CesiumGltf/PropertyTexturePropertyView.h b/CesiumGltf/include/CesiumGltf/PropertyTexturePropertyView.h index 951cad5d8..c64b8e402 100644 --- a/CesiumGltf/include/CesiumGltf/PropertyTexturePropertyView.h +++ b/CesiumGltf/include/CesiumGltf/PropertyTexturePropertyView.h @@ -229,14 +229,8 @@ class PropertyTexturePropertyView */ PropertyTexturePropertyView() noexcept : PropertyView(), - TextureView(), - _pSampler(nullptr), - _pImage(nullptr), - _texCoordSetIndex(0), - _channels(), - _swizzle(), - _applyTextureTransform(false), - _textureTransform(std::nullopt) {} + TextureView() _channels(), + _swizzle() {} /** * @brief Constructs an invalid instance for an erroneous property. diff --git a/CesiumGltf/include/CesiumGltf/PropertyTextureView.h b/CesiumGltf/include/CesiumGltf/PropertyTextureView.h index b7940d9de..e6613efc6 100644 --- a/CesiumGltf/include/CesiumGltf/PropertyTextureView.h +++ b/CesiumGltf/include/CesiumGltf/PropertyTextureView.h @@ -55,14 +55,10 @@ class PropertyTextureView { * @param model The glTF that contains the property texture's data. * @param propertyTexture The {@link PropertyTexture} * from which the view will retrieve data. - * @param applyKhrTextureTransformExtension Whether to automatically apply the - * `KHR_texture_transform` extension to the properties in this property - * texture, if present. */ PropertyTextureView( const Model& model, - const PropertyTexture& propertyTexture, - TextureViewOptions propertyOptions = TextureViewOptions()) noexcept; + const PropertyTexture& propertyTexture) noexcept; /** * @brief Gets the status of this property texture view. @@ -117,13 +113,15 @@ class PropertyTextureView { * @tparam T The C++ type corresponding to the type of the data retrieved. * @tparam Normalized Whether the property is normalized. Only applicable to * types with integer components. - * @param propertyId The id of the property to retrieve data from + * @param propertyId The ID of the property to retrieve data from + * @param propertyOptions The options to apply to the property. * @return A {@link PropertyTexturePropertyView} of the property. If no valid * property is found, the property view will be invalid. */ template - PropertyTexturePropertyView - getPropertyView(const std::string& propertyId) const { + PropertyTexturePropertyView getPropertyView( + const std::string& propertyId, + TextureViewOptions propertyOptions = TextureViewOptions()) const { if (this->_status != PropertyTextureViewStatus::Valid) { return PropertyTexturePropertyView( PropertyTexturePropertyViewStatus::ErrorInvalidPropertyTexture); @@ -135,7 +133,10 @@ class PropertyTextureView { PropertyTexturePropertyViewStatus::ErrorNonexistentProperty); } - return getPropertyViewImpl(propertyId, *pClassProperty); + return getPropertyViewImpl( + propertyId, + *pClassProperty, + propertyOptions); } /** @@ -156,10 +157,13 @@ class PropertyTextureView { * @param propertyId The id of the property to retrieve data from * @tparam callback A callback function that accepts a property id and a * {@link PropertyTexturePropertyView} + * @param propertyOptions The options to apply to the property. */ template - void - getPropertyView(const std::string& propertyId, Callback&& callback) const { + void getPropertyView( + const std::string& propertyId, + Callback&& callback, + TextureViewOptions propertyOptions = TextureViewOptions()) const { if (this->_status != PropertyTextureViewStatus::Valid) { callback( propertyId, @@ -201,14 +205,16 @@ class PropertyTextureView { *pClassProperty, type, componentType, - std::forward(callback)); + std::forward(callback), + propertyOptions); } else { getArrayPropertyViewImpl( propertyId, *pClassProperty, type, componentType, - std::forward(callback)); + std::forward(callback), + propertyOptions); } return; } @@ -219,13 +225,15 @@ class PropertyTextureView { propertyId, *pClassProperty, componentType, - std::forward(callback)); + std::forward(callback), + propertyOptions); } else { getScalarPropertyViewImpl( propertyId, *pClassProperty, componentType, - std::forward(callback)); + std::forward(callback), + propertyOptions); } return; } @@ -237,14 +245,16 @@ class PropertyTextureView { *pClassProperty, type, componentType, - std::forward(callback)); + std::forward(callback), + propertyOptions); } else { getVecNPropertyViewImpl( propertyId, *pClassProperty, type, componentType, - std::forward(callback)); + std::forward(callback), + propertyOptions); } return; } @@ -271,13 +281,21 @@ class PropertyTextureView { * error status will be passed to the callback. Otherwise, a valid property * view will be passed to the callback. * - * @param propertyId The id of the property to retrieve data from * @tparam callback A callback function that accepts property id and * {@link PropertyTexturePropertyView} + * @param propertyOptions The options to apply to each property in the + * property texture. */ - template void forEachProperty(Callback&& callback) const { + + template + void forEachProperty( + Callback&& callback, + TextureViewOptions propertyOptions = TextureViewOptions()) const { for (const auto& property : this->_pClass->properties) { - getPropertyView(property.first, std::forward(callback)); + getPropertyView( + property.first, + std::forward(callback), + propertyOptions); } } @@ -285,7 +303,8 @@ class PropertyTextureView { template PropertyTexturePropertyView getPropertyViewImpl( const std::string& propertyId, - const ClassProperty& classProperty) const { + const ClassProperty& classProperty, + const TextureViewOptions& propertyOptions) const { auto propertyTexturePropertyIter = _pPropertyTexture->properties.find(propertyId); if (propertyTexturePropertyIter == _pPropertyTexture->properties.end()) { @@ -306,19 +325,21 @@ class PropertyTextureView { if constexpr (IsMetadataScalar::value) { return createScalarPropertyView( classProperty, - propertyTextureProperty); + propertyTextureProperty, + propertyOptions); } if constexpr (IsMetadataVecN::value) { return createVecNPropertyView( classProperty, - propertyTextureProperty); + propertyTextureProperty, + propertyOptions); } if constexpr (IsMetadataArray::value) { return createArrayPropertyView< typename MetadataArrayType::type, - Normalized>(classProperty, propertyTextureProperty); + Normalized>(classProperty, propertyTextureProperty, propertyOptions); } } @@ -328,7 +349,8 @@ class PropertyTextureView { const ClassProperty& classProperty, PropertyType type, PropertyComponentType componentType, - Callback&& callback) const { + Callback&& callback, + const TextureViewOptions& propertyOptions) const { // Only scalar arrays are supported. if (type != PropertyType::Scalar) { callback( @@ -353,28 +375,32 @@ class PropertyTextureView { propertyId, getPropertyViewImpl, Normalized>( propertyId, - classProperty)); + classProperty, + propertyOptions)); break; case PropertyComponentType::Uint8: callback( propertyId, getPropertyViewImpl, Normalized>( propertyId, - classProperty)); + classProperty, + propertyOptions)); break; case PropertyComponentType::Int16: callback( propertyId, getPropertyViewImpl, Normalized>( propertyId, - classProperty)); + classProperty, + propertyOptions)); break; case PropertyComponentType::Uint16: callback( propertyId, getPropertyViewImpl, Normalized>( propertyId, - classProperty)); + classProperty, + propertyOptions)); break; default: callback( @@ -390,42 +416,64 @@ class PropertyTextureView { const std::string& propertyId, const ClassProperty& classProperty, PropertyComponentType componentType, - Callback&& callback) const { + Callback&& callback, + const TextureViewOptions& propertyOptions) const { switch (componentType) { case PropertyComponentType::Int8: callback( propertyId, - getPropertyViewImpl(propertyId, classProperty)); + getPropertyViewImpl( + propertyId, + classProperty, + propertyOptions)); return; case PropertyComponentType::Uint8: callback( propertyId, - getPropertyViewImpl(propertyId, classProperty)); + getPropertyViewImpl( + propertyId, + classProperty, + propertyOptions)); return; case PropertyComponentType::Int16: callback( propertyId, - getPropertyViewImpl(propertyId, classProperty)); + getPropertyViewImpl( + propertyId, + classProperty, + propertyOptions)); return; case PropertyComponentType::Uint16: callback( propertyId, - getPropertyViewImpl(propertyId, classProperty)); + getPropertyViewImpl( + propertyId, + classProperty, + propertyOptions)); break; case PropertyComponentType::Int32: callback( propertyId, - getPropertyViewImpl(propertyId, classProperty)); + getPropertyViewImpl( + propertyId, + classProperty, + propertyOptions)); break; case PropertyComponentType::Uint32: callback( propertyId, - getPropertyViewImpl(propertyId, classProperty)); + getPropertyViewImpl( + propertyId, + classProperty, + propertyOptions)); break; case PropertyComponentType::Float32: callback( propertyId, - getPropertyViewImpl(propertyId, classProperty)); + getPropertyViewImpl( + propertyId, + classProperty, + propertyOptions)); break; default: callback( @@ -441,21 +489,24 @@ class PropertyTextureView { const std::string& propertyId, const ClassProperty& classProperty, PropertyComponentType componentType, - Callback&& callback) const { + Callback&& callback, + const TextureViewOptions& propertyOptions) const { switch (componentType) { case PropertyComponentType::Int8: callback( propertyId, getPropertyViewImpl, Normalized>( propertyId, - classProperty)); + classProperty, + propertyOptions)); break; case PropertyComponentType::Uint8: callback( propertyId, getPropertyViewImpl, Normalized>( propertyId, - classProperty)); + classProperty, + propertyOptions)); break; case PropertyComponentType::Int16: if constexpr (N == 2) { @@ -463,7 +514,8 @@ class PropertyTextureView { propertyId, getPropertyViewImpl, Normalized>( propertyId, - classProperty)); + classProperty, + propertyOptions)); break; } [[fallthrough]]; @@ -473,7 +525,8 @@ class PropertyTextureView { propertyId, getPropertyViewImpl, Normalized>( propertyId, - classProperty)); + classProperty, + propertyOptions)); break; } [[fallthrough]]; @@ -492,7 +545,8 @@ class PropertyTextureView { const ClassProperty& classProperty, PropertyType type, PropertyComponentType componentType, - Callback&& callback) const { + Callback&& callback, + const TextureViewOptions& propertyOptions) const { const glm::length_t N = getDimensionsFromPropertyType(type); switch (N) { case 2: @@ -500,21 +554,24 @@ class PropertyTextureView { propertyId, classProperty, componentType, - std::forward(callback)); + std::forward(callback), + propertyOptions); break; case 3: getVecNPropertyViewImpl( propertyId, classProperty, componentType, - std::forward(callback)); + std::forward(callback), + propertyOptions); break; case 4: getVecNPropertyViewImpl( propertyId, classProperty, componentType, - std::forward(callback)); + std::forward(callback), + propertyOptions); break; default: callback( @@ -528,8 +585,8 @@ class PropertyTextureView { template PropertyTexturePropertyView createScalarPropertyView( const ClassProperty& classProperty, - [[maybe_unused]] const PropertyTextureProperty& propertyTextureProperty) - const { + [[maybe_unused]] const PropertyTextureProperty& propertyTextureProperty, + TextureViewOptions propertyOptions) const { if (classProperty.array) { return PropertyTexturePropertyView( PropertyTexturePropertyViewStatus::ErrorArrayTypeMismatch); @@ -559,7 +616,8 @@ class PropertyTextureView { return createPropertyViewImpl( classProperty, propertyTextureProperty, - sizeof(T)); + sizeof(T), + propertyOptions); } else { return PropertyTexturePropertyView( PropertyTexturePropertyViewStatus::ErrorUnsupportedProperty); @@ -569,8 +627,8 @@ class PropertyTextureView { template PropertyTexturePropertyView createVecNPropertyView( const ClassProperty& classProperty, - [[maybe_unused]] const PropertyTextureProperty& propertyTextureProperty) - const { + [[maybe_unused]] const PropertyTextureProperty& propertyTextureProperty, + [[maybe_unused]] const TextureViewOptions& propertyOptions) const { if (classProperty.array) { return PropertyTexturePropertyView( PropertyTexturePropertyViewStatus::ErrorArrayTypeMismatch); @@ -600,7 +658,8 @@ class PropertyTextureView { return createPropertyViewImpl( classProperty, propertyTextureProperty, - sizeof(T)); + sizeof(T), + propertyOptions); } else { return PropertyTexturePropertyView( PropertyTexturePropertyViewStatus::ErrorUnsupportedProperty); @@ -611,8 +670,8 @@ class PropertyTextureView { PropertyTexturePropertyView, Normalized> createArrayPropertyView( const ClassProperty& classProperty, - [[maybe_unused]] const PropertyTextureProperty& propertyTextureProperty) - const { + [[maybe_unused]] const PropertyTextureProperty& propertyTextureProperty, + [[maybe_unused]] const TextureViewOptions& propertyOptions) const { if (!classProperty.array) { return PropertyTexturePropertyView, Normalized>( PropertyTexturePropertyViewStatus::ErrorArrayTypeMismatch); @@ -655,7 +714,8 @@ class PropertyTextureView { return createPropertyViewImpl, Normalized>( classProperty, propertyTextureProperty, - count * sizeof(T)); + count * sizeof(T), + propertyOptions); } else { return PropertyTexturePropertyView, Normalized>( PropertyTexturePropertyViewStatus::ErrorUnsupportedProperty); @@ -666,7 +726,8 @@ class PropertyTextureView { PropertyTexturePropertyView createPropertyViewImpl( const ClassProperty& classProperty, const PropertyTextureProperty& propertyTextureProperty, - size_t elementSize) const { + size_t elementSize, + const TextureViewOptions& propertyOptions) const { int32_t samplerIndex; int32_t imageIndex; @@ -704,7 +765,7 @@ class PropertyTextureView { classProperty, _pModel->samplers[samplerIndex], image, - this->_propertyOptions); + propertyOptions); } PropertyViewStatusType getTextureSafe( @@ -725,7 +786,6 @@ class PropertyTextureView { const PropertyTexture* _pPropertyTexture; const Class* _pClass; - TextureViewOptions _propertyOptions; PropertyTextureViewStatus _status; }; } // namespace CesiumGltf diff --git a/CesiumGltf/src/PropertyTextureView.cpp b/CesiumGltf/src/PropertyTextureView.cpp index b774e27b6..f5ac76a11 100644 --- a/CesiumGltf/src/PropertyTextureView.cpp +++ b/CesiumGltf/src/PropertyTextureView.cpp @@ -3,12 +3,10 @@ namespace CesiumGltf { PropertyTextureView::PropertyTextureView( const Model& model, - const PropertyTexture& propertyTexture, - TextureViewOptions options) noexcept + const PropertyTexture& propertyTexture) noexcept : _pModel(&model), _pPropertyTexture(&propertyTexture), _pClass(nullptr), - _propertyOptions(options), _status() { const ExtensionModelExtStructuralMetadata* pMetadata = model.getExtension(); From ff3da04838cc25c2e655eeddb42fd1b6e8907b71 Mon Sep 17 00:00:00 2001 From: Janine Liu Date: Wed, 28 Feb 2024 11:50:48 -0500 Subject: [PATCH 07/12] Fix CI errors --- CesiumGltf/include/CesiumGltf/PropertyTexturePropertyView.h | 3 ++- CesiumGltf/src/TextureView.cpp | 6 ++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/CesiumGltf/include/CesiumGltf/PropertyTexturePropertyView.h b/CesiumGltf/include/CesiumGltf/PropertyTexturePropertyView.h index c64b8e402..588d57755 100644 --- a/CesiumGltf/include/CesiumGltf/PropertyTexturePropertyView.h +++ b/CesiumGltf/include/CesiumGltf/PropertyTexturePropertyView.h @@ -229,7 +229,8 @@ class PropertyTexturePropertyView */ PropertyTexturePropertyView() noexcept : PropertyView(), - TextureView() _channels(), + TextureView(), + _channels(), _swizzle() {} /** diff --git a/CesiumGltf/src/TextureView.cpp b/CesiumGltf/src/TextureView.cpp index 8a84a45ee..7727243db 100644 --- a/CesiumGltf/src/TextureView.cpp +++ b/CesiumGltf/src/TextureView.cpp @@ -57,8 +57,7 @@ TextureView::TextureView( // decompressed here. if (this->_pImage->bytesPerChannel > 1) { - this->_textureViewStatus = - TextureViewStatus::ErrorInvalidBytesPerChannel; + this->_textureViewStatus = TextureViewStatus::ErrorInvalidBytesPerChannel; return; } @@ -93,8 +92,7 @@ TextureView::TextureView( // decompressed here. if (this->_pImage->bytesPerChannel > 1) { - this->_textureViewStatus = - TextureViewStatus::ErrorInvalidBytesPerChannel; + this->_textureViewStatus = TextureViewStatus::ErrorInvalidBytesPerChannel; return; } From a1bf6e7d2d9066cfba16e33118bc7a099d8454a5 Mon Sep 17 00:00:00 2001 From: Janine Liu Date: Wed, 28 Feb 2024 17:21:42 -0500 Subject: [PATCH 08/12] Add unit tests --- CesiumGltf/include/CesiumGltf/TextureView.h | 4 +- CesiumGltf/test/TestPropertyTextureView.cpp | 1178 ++++++++++++++++--- 2 files changed, 987 insertions(+), 195 deletions(-) diff --git a/CesiumGltf/include/CesiumGltf/TextureView.h b/CesiumGltf/include/CesiumGltf/TextureView.h index c5f7d2a99..d8644fc9f 100644 --- a/CesiumGltf/include/CesiumGltf/TextureView.h +++ b/CesiumGltf/include/CesiumGltf/TextureView.h @@ -168,7 +168,9 @@ class TextureView { const Sampler* getSampler() const noexcept { return this->_pSampler; } /** - * @brief Get the image containing this property's data. + * @brief Get the image containing this property's data. If this view was + * constructed with options.makeImageCopy set to true, this will return a + * pointer to the copied image. * * This will be nullptr if the texture view runs into * problems during construction. diff --git a/CesiumGltf/test/TestPropertyTextureView.cpp b/CesiumGltf/test/TestPropertyTextureView.cpp index 8b616f03b..469707f82 100644 --- a/CesiumGltf/test/TestPropertyTextureView.cpp +++ b/CesiumGltf/test/TestPropertyTextureView.cpp @@ -1,4 +1,5 @@ #include "CesiumGltf/PropertyTextureView.h" +#include "CesiumUtility/Math.h" #include #include @@ -35,6 +36,29 @@ void addTextureToModel( texture.sampler = static_cast(model.samplers.size() - 1); texture.source = static_cast(model.images.size() - 1); } + +template +void verifyTextureTransformConstruction( + const PropertyTexturePropertyView& propertyView, + const ExtensionKhrTextureTransform& extension) { + auto textureTransform = propertyView.getTextureTransform(); + REQUIRE(textureTransform != std::nullopt); + REQUIRE( + textureTransform->offset() == + glm::dvec2{extension.offset[0], extension.offset[1]}); + REQUIRE(textureTransform->rotation() == extension.rotation); + REQUIRE( + textureTransform->scale() == + glm::dvec2{extension.scale[0], extension.scale[1]}); + + // Texcoord is overridden by value in KHR_texture_transform. + if (extension.texCoord) { + REQUIRE( + propertyView.getTexCoordSetIndex() == + textureTransform->getTexCoordSetIndex()); + REQUIRE(textureTransform->getTexCoordSetIndex() == *extension.texCoord); + } +} } // namespace TEST_CASE("Test PropertyTextureView on model without EXT_structural_metadata " @@ -119,8 +143,8 @@ TEST_CASE("Test scalar PropertyTextureProperty") { addTextureToModel( model, - Sampler::WrapS::CLAMP_TO_EDGE, - Sampler::WrapS::CLAMP_TO_EDGE, + Sampler::WrapS::REPEAT, + Sampler::WrapT::REPEAT, 2, 2, 1, @@ -176,6 +200,69 @@ TEST_CASE("Test scalar PropertyTextureProperty") { } } + SECTION("Access with KHR_texture_transform") { + TextureViewOptions options; + options.applyKhrTextureTransformExtension = true; + + ExtensionKhrTextureTransform& extension = + propertyTextureProperty.addExtension(); + extension.offset = {0.5, -0.5}; + extension.rotation = CesiumUtility::Math::PiOverTwo; + extension.scale = {0.5, 0.5}; + extension.texCoord = 10; + + PropertyTexturePropertyView uint8Property = + view.getPropertyView("TestClassProperty", options); + REQUIRE(uint8Property.status() == PropertyTexturePropertyViewStatus::Valid); + + verifyTextureTransformConstruction(uint8Property, extension); + + // This transforms to the following UV values: + // (0, 0) -> (0.5, -0.5) -> wraps to (0.5, 0.5) + // (1, 0) -> (0.5, -1) -> wraps to (0.5, 0) + // (0, 1) -> (1, -0.5) -> wraps to (0, 0.5) + // (1, 1) -> (1, -1) -> wraps to (0.0, 0.0) + std::vector texCoords{ + glm::dvec2(0, 0), + glm::dvec2(1, 0), + glm::dvec2(0, 1), + glm::dvec2(1, 1)}; + + std::vector expected{data[3], data[1], data[2], data[0]}; + for (size_t i = 0; i < texCoords.size(); ++i) { + glm::dvec2 uv = texCoords[i]; + REQUIRE(uint8Property.getRaw(uv[0], uv[1]) == expected[i]); + REQUIRE(uint8Property.get(uv[0], uv[1]) == expected[i]); + } + + propertyTextureProperty.extensions.clear(); + } + + SECTION("Access with image copy") { + TextureViewOptions options; + options.makeImageCopy = true; + + PropertyTexturePropertyView uint8Property = + view.getPropertyView("TestClassProperty", options); + REQUIRE(uint8Property.status() == PropertyTexturePropertyViewStatus::Valid); + + // Clear the original image data. + std::vector emptyData; + model.images[model.images.size() - 1].cesium.pixelData.swap(emptyData); + + std::vector texCoords{ + glm::dvec2(0, 0), + glm::dvec2(0.5, 0), + glm::dvec2(0, 0.5), + glm::dvec2(0.5, 0.5)}; + + for (size_t i = 0; i < texCoords.size(); ++i) { + glm::dvec2 uv = texCoords[i]; + REQUIRE(uint8Property.getRaw(uv[0], uv[1]) == data[i]); + REQUIRE(uint8Property.get(uv[0], uv[1]) == data[i]); + } + } + SECTION("Access wrong type") { PropertyTexturePropertyView u8vec2Invalid = view.getPropertyView("TestClassProperty"); @@ -300,8 +387,8 @@ TEST_CASE("Test scalar PropertyTextureProperty (normalized)") { addTextureToModel( model, - Sampler::WrapS::CLAMP_TO_EDGE, - Sampler::WrapS::CLAMP_TO_EDGE, + Sampler::WrapS::REPEAT, + Sampler::WrapT::REPEAT, 2, 2, 1, @@ -358,6 +445,69 @@ TEST_CASE("Test scalar PropertyTextureProperty (normalized)") { } } + SECTION("Access with KHR_texture_transform") { + TextureViewOptions options; + options.applyKhrTextureTransformExtension = true; + + ExtensionKhrTextureTransform& extension = + propertyTextureProperty.addExtension(); + extension.offset = {0.5, -0.5}; + extension.rotation = CesiumUtility::Math::PiOverTwo; + extension.scale = {0.5, 0.5}; + extension.texCoord = 10; + + PropertyTexturePropertyView uint8Property = + view.getPropertyView("TestClassProperty", options); + REQUIRE(uint8Property.status() == PropertyTexturePropertyViewStatus::Valid); + + verifyTextureTransformConstruction(uint8Property, extension); + + // This transforms to the following UV values: + // (0, 0) -> (0.5, -0.5) -> wraps to (0.5, 0.5) + // (1, 0) -> (0.5, -1) -> wraps to (0.5, 0) + // (0, 1) -> (1, -0.5) -> wraps to (0, 0.5) + // (1, 1) -> (1, -1) -> wraps to (0.0, 0.0) + std::vector texCoords{ + glm::dvec2(0, 0), + glm::dvec2(1, 0), + glm::dvec2(0, 1), + glm::dvec2(1, 1)}; + + std::vector expected{data[3], data[1], data[2], data[0]}; + for (size_t i = 0; i < texCoords.size(); ++i) { + glm::dvec2 uv = texCoords[i]; + REQUIRE(uint8Property.getRaw(uv[0], uv[1]) == expected[i]); + REQUIRE(uint8Property.get(uv[0], uv[1]) == normalize(expected[i])); + } + + propertyTextureProperty.extensions.clear(); + } + + SECTION("Access with image copy") { + TextureViewOptions options; + options.makeImageCopy = true; + + PropertyTexturePropertyView uint8Property = + view.getPropertyView("TestClassProperty", options); + REQUIRE(uint8Property.status() == PropertyTexturePropertyViewStatus::Valid); + + // Clear the original image data. + std::vector emptyData; + model.images[model.images.size() - 1].cesium.pixelData.swap(emptyData); + + std::vector texCoords{ + glm::dvec2(0, 0), + glm::dvec2(0.5, 0), + glm::dvec2(0, 0.5), + glm::dvec2(0.5, 0.5)}; + + for (size_t i = 0; i < texCoords.size(); ++i) { + glm::dvec2 uv = texCoords[i]; + REQUIRE(uint8Property.getRaw(uv[0], uv[1]) == data[i]); + REQUIRE(uint8Property.get(uv[0], uv[1]) == normalize(data[i])); + } + } + SECTION("Access wrong type") { PropertyTexturePropertyView u8vec2Invalid = view.getPropertyView("TestClassProperty"); @@ -419,11 +569,16 @@ TEST_CASE("Test scalar PropertyTextureProperty (normalized)") { TEST_CASE("Test vecN PropertyTextureProperty") { Model model; std::vector data = {12, 34, 10, 3, 40, 0, 30, 11}; + std::vector expected{ + glm::u8vec2(data[0], data[1]), + glm::u8vec2(data[2], data[3]), + glm::u8vec2(data[4], data[5]), + glm::u8vec2(data[6], data[7])}; addTextureToModel( model, - Sampler::WrapS::CLAMP_TO_EDGE, - Sampler::WrapS::CLAMP_TO_EDGE, + Sampler::WrapS::REPEAT, + Sampler::WrapT::REPEAT, 2, 2, 2, @@ -473,11 +628,75 @@ TEST_CASE("Test vecN PropertyTextureProperty") { glm::dvec2(0, 0.5), glm::dvec2(0.5, 0.5)}; - std::vector expected{ - glm::u8vec2(12, 34), - glm::u8vec2(10, 3), - glm::u8vec2(40, 0), - glm::u8vec2(30, 11)}; + for (size_t i = 0; i < texCoords.size(); ++i) { + glm::dvec2 uv = texCoords[i]; + REQUIRE(u8vec2Property.getRaw(uv[0], uv[1]) == expected[i]); + REQUIRE(u8vec2Property.get(uv[0], uv[1]) == expected[i]); + } + } + + SECTION("Access with KHR_texture_transform") { + TextureViewOptions options; + options.applyKhrTextureTransformExtension = true; + + ExtensionKhrTextureTransform& extension = + propertyTextureProperty.addExtension(); + extension.offset = {0.5, -0.5}; + extension.rotation = CesiumUtility::Math::PiOverTwo; + extension.scale = {0.5, 0.5}; + extension.texCoord = 10; + + PropertyTexturePropertyView u8vec2Property = + view.getPropertyView("TestClassProperty", options); + REQUIRE( + u8vec2Property.status() == PropertyTexturePropertyViewStatus::Valid); + + verifyTextureTransformConstruction(u8vec2Property, extension); + + // This transforms to the following UV values: + // (0, 0) -> (0.5, -0.5) -> wraps to (0.5, 0.5) + // (1, 0) -> (0.5, -1) -> wraps to (0.5, 0) + // (0, 1) -> (1, -0.5) -> wraps to (0, 0.5) + // (1, 1) -> (1, -1) -> wraps to (0.0, 0.0) + std::vector texCoords{ + glm::dvec2(0, 0), + glm::dvec2(1, 0), + glm::dvec2(0, 1), + glm::dvec2(1, 1)}; + + std::vector expectedTransformed{ + expected[3], + expected[1], + expected[2], + expected[0]}; + for (size_t i = 0; i < texCoords.size(); ++i) { + glm::dvec2 uv = texCoords[i]; + REQUIRE(u8vec2Property.getRaw(uv[0], uv[1]) == expectedTransformed[i]); + REQUIRE(u8vec2Property.get(uv[0], uv[1]) == expectedTransformed[i]); + } + + propertyTextureProperty.extensions.clear(); + } + + SECTION("Access with image copy") { + TextureViewOptions options; + options.makeImageCopy = true; + + PropertyTexturePropertyView u8vec2Property = + view.getPropertyView("TestClassProperty", options); + REQUIRE( + u8vec2Property.status() == PropertyTexturePropertyViewStatus::Valid); + + // Clear the original image data. + std::vector emptyData; + model.images[model.images.size() - 1].cesium.pixelData.swap(emptyData); + + std::vector texCoords{ + glm::dvec2(0, 0), + glm::dvec2(0.5, 0), + glm::dvec2(0, 0.5), + glm::dvec2(0.5, 0.5)}; + for (size_t i = 0; i < texCoords.size(); ++i) { glm::dvec2 uv = texCoords[i]; REQUIRE(u8vec2Property.getRaw(uv[0], uv[1]) == expected[i]); @@ -562,11 +781,16 @@ TEST_CASE("Test vecN PropertyTextureProperty") { TEST_CASE("Test vecN PropertyTextureProperty (normalized)") { Model model; std::vector data = {12, 34, 10, 3, 40, 0, 30, 11}; + std::vector expected{ + glm::u8vec2(data[0], data[1]), + glm::u8vec2(data[2], data[3]), + glm::u8vec2(data[4], data[5]), + glm::u8vec2(data[6], data[7])}; addTextureToModel( model, - Sampler::WrapS::CLAMP_TO_EDGE, - Sampler::WrapS::CLAMP_TO_EDGE, + Sampler::WrapS::REPEAT, + Sampler::WrapT::REPEAT, 2, 2, 2, @@ -611,17 +835,82 @@ TEST_CASE("Test vecN PropertyTextureProperty (normalized)") { REQUIRE( u8vec2Property.status() == PropertyTexturePropertyViewStatus::Valid); + std::vector texCoords{ + glm::dvec2(0, 0), + glm::dvec2(0.5, 0), + glm::dvec2(0, 0.5), + glm::dvec2(0.5, 0.5)}; + for (size_t i = 0; i < texCoords.size(); ++i) { + glm::dvec2 uv = texCoords[i]; + REQUIRE(u8vec2Property.getRaw(uv[0], uv[1]) == expected[i]); + REQUIRE(u8vec2Property.get(uv[0], uv[1]) == normalize(expected[i])); + } + } + + SECTION("Access with KHR_texture_transform") { + TextureViewOptions options; + options.applyKhrTextureTransformExtension = true; + + ExtensionKhrTextureTransform& extension = + propertyTextureProperty.addExtension(); + extension.offset = {0.5, -0.5}; + extension.rotation = CesiumUtility::Math::PiOverTwo; + extension.scale = {0.5, 0.5}; + extension.texCoord = 10; + + PropertyTexturePropertyView u8vec2Property = + view.getPropertyView("TestClassProperty", options); + REQUIRE( + u8vec2Property.status() == PropertyTexturePropertyViewStatus::Valid); + + verifyTextureTransformConstruction(u8vec2Property, extension); + + // This transforms to the following UV values: + // (0, 0) -> (0.5, -0.5) -> wraps to (0.5, 0.5) + // (1, 0) -> (0.5, -1) -> wraps to (0.5, 0) + // (0, 1) -> (1, -0.5) -> wraps to (0, 0.5) + // (1, 1) -> (1, -1) -> wraps to (0.0, 0.0) + std::vector texCoords{ + glm::dvec2(0, 0), + glm::dvec2(1, 0), + glm::dvec2(0, 1), + glm::dvec2(1, 1)}; + + std::vector expectedTransformed{ + expected[3], + expected[1], + expected[2], + expected[0]}; + for (size_t i = 0; i < texCoords.size(); ++i) { + glm::dvec2 uv = texCoords[i]; + REQUIRE(u8vec2Property.getRaw(uv[0], uv[1]) == expectedTransformed[i]); + REQUIRE( + u8vec2Property.get(uv[0], uv[1]) == + normalize(expectedTransformed[i])); + } + + propertyTextureProperty.extensions.clear(); + } + + SECTION("Access with image copy") { + TextureViewOptions options; + options.makeImageCopy = true; + + PropertyTexturePropertyView u8vec2Property = + view.getPropertyView("TestClassProperty", options); + REQUIRE( + u8vec2Property.status() == PropertyTexturePropertyViewStatus::Valid); + + // Clear the original image data. + std::vector emptyData; + model.images[model.images.size() - 1].cesium.pixelData.swap(emptyData); + std::vector texCoords{ glm::dvec2(0, 0), glm::dvec2(0.5, 0), glm::dvec2(0, 0.5), glm::dvec2(0.5, 0.5)}; - std::vector expected{ - glm::u8vec2(12, 34), - glm::u8vec2(10, 3), - glm::u8vec2(40, 0), - glm::u8vec2(30, 11)}; for (size_t i = 0; i < texCoords.size(); ++i) { glm::dvec2 uv = texCoords[i]; REQUIRE(u8vec2Property.getRaw(uv[0], uv[1]) == expected[i]); @@ -704,11 +993,17 @@ TEST_CASE("Test array PropertyTextureProperty") { 6, 3, 4, }; // clang-format on + std::vector> expected = { + {data[0], data[1], data[2]}, + {data[3], data[4], data[5]}, + {data[6], data[7], data[8]}, + {data[9], data[10], data[11]}, + }; addTextureToModel( model, - Sampler::WrapS::CLAMP_TO_EDGE, - Sampler::WrapS::CLAMP_TO_EDGE, + Sampler::WrapS::REPEAT, + Sampler::WrapT::REPEAT, 2, 2, 3, @@ -761,18 +1056,118 @@ TEST_CASE("Test array PropertyTextureProperty") { glm::dvec2(0, 0.5), glm::dvec2(0.5, 0.5)}; - int64_t size = static_cast(texCoords.size()); - for (int64_t i = 0; i < size; ++i) { - glm::dvec2 uv = texCoords[static_cast(i)]; + for (size_t i = 0; i < texCoords.size(); ++i) { + glm::dvec2 uv = texCoords[i]; + const std::array& expectedArray = expected[i]; + + const PropertyArrayView& value = + uint8ArrayProperty.getRaw(uv[0], uv[1]); + REQUIRE(static_cast(value.size()) == expectedArray.size()); + + for (int64_t j = 0; j < value.size(); j++) { + REQUIRE(value[j] == expectedArray[static_cast(j)]); + } + + auto maybeValue = uint8ArrayProperty.get(uv[0], uv[1]); + REQUIRE(maybeValue); + for (int64_t j = 0; j < maybeValue->size(); j++) { + REQUIRE((*maybeValue)[j] == value[j]); + } + } + } + + SECTION("Access with KHR_texture_transform") { + TextureViewOptions options; + options.applyKhrTextureTransformExtension = true; - auto dataStart = data.begin() + i * 3; - std::vector expected(dataStart, dataStart + 3); + ExtensionKhrTextureTransform& extension = + propertyTextureProperty.addExtension(); + extension.offset = {0.5, -0.5}; + extension.rotation = CesiumUtility::Math::PiOverTwo; + extension.scale = {0.5, 0.5}; + extension.texCoord = 10; + + PropertyTexturePropertyView> uint8ArrayProperty = + view.getPropertyView>( + "TestClassProperty", + options); + REQUIRE( + uint8ArrayProperty.status() == + PropertyTexturePropertyViewStatus::Valid); + + verifyTextureTransformConstruction(uint8ArrayProperty, extension); + + // This transforms to the following UV values: + // (0, 0) -> (0.5, -0.5) -> wraps to (0.5, 0.5) + // (1, 0) -> (0.5, -1) -> wraps to (0.5, 0) + // (0, 1) -> (1, -0.5) -> wraps to (0, 0.5) + // (1, 1) -> (1, -1) -> wraps to (0.0, 0.0) + std::vector texCoords{ + glm::dvec2(0, 0), + glm::dvec2(1, 0), + glm::dvec2(0, 1), + glm::dvec2(1, 1)}; + + std::vector> expectedTransformed{ + expected[3], + expected[1], + expected[2], + expected[0]}; + + for (size_t i = 0; i < texCoords.size(); ++i) { + glm::dvec2 uv = texCoords[i]; + const std::array& expectedArray = expectedTransformed[i]; const PropertyArrayView& value = uint8ArrayProperty.getRaw(uv[0], uv[1]); - REQUIRE(static_cast(value.size()) == expected.size()); + REQUIRE(static_cast(value.size()) == expectedArray.size()); + for (int64_t j = 0; j < value.size(); j++) { - REQUIRE(value[j] == expected[static_cast(j)]); + REQUIRE(value[j] == expectedArray[static_cast(j)]); + } + + auto maybeValue = uint8ArrayProperty.get(uv[0], uv[1]); + REQUIRE(maybeValue); + for (int64_t j = 0; j < maybeValue->size(); j++) { + REQUIRE((*maybeValue)[j] == value[j]); + } + } + + propertyTextureProperty.extensions.clear(); + } + + SECTION("Access with image copy") { + TextureViewOptions options; + options.makeImageCopy = true; + + PropertyTexturePropertyView> uint8ArrayProperty = + view.getPropertyView>( + "TestClassProperty", + options); + REQUIRE( + uint8ArrayProperty.status() == + PropertyTexturePropertyViewStatus::Valid); + + // Clear the original image data. + std::vector emptyData; + model.images[model.images.size() - 1].cesium.pixelData.swap(emptyData); + + std::vector texCoords{ + glm::dvec2(0, 0), + glm::dvec2(0.5, 0), + glm::dvec2(0, 0.5), + glm::dvec2(0.5, 0.5)}; + + for (size_t i = 0; i < texCoords.size(); ++i) { + glm::dvec2 uv = texCoords[i]; + const std::array& expectedArray = expected[i]; + + const PropertyArrayView& value = + uint8ArrayProperty.getRaw(uv[0], uv[1]); + REQUIRE(static_cast(value.size()) == expectedArray.size()); + + for (int64_t j = 0; j < value.size(); j++) { + REQUIRE(value[j] == expectedArray[static_cast(j)]); } auto maybeValue = uint8ArrayProperty.get(uv[0], uv[1]); @@ -862,10 +1257,17 @@ TEST_CASE("Test array PropertyTextureProperty (normalized)") { }; // clang-format on + std::vector> expected = { + {data[0], data[1], data[2]}, + {data[3], data[4], data[5]}, + {data[6], data[7], data[8]}, + {data[9], data[10], data[11]}, + }; + addTextureToModel( model, - Sampler::WrapS::CLAMP_TO_EDGE, - Sampler::WrapS::CLAMP_TO_EDGE, + Sampler::WrapS::REPEAT, + Sampler::WrapT::REPEAT, 2, 2, 3, @@ -924,15 +1326,117 @@ TEST_CASE("Test array PropertyTextureProperty (normalized)") { int64_t size = static_cast(texCoords.size()); for (int64_t i = 0; i < size; ++i) { glm::dvec2 uv = texCoords[static_cast(i)]; + const std::array& expectedArray = expected[i]; + + const PropertyArrayView& value = + uint8ArrayProperty.getRaw(uv[0], uv[1]); + REQUIRE(static_cast(value.size()) == expectedArray.size()); + for (int64_t j = 0; j < value.size(); j++) { + REQUIRE(value[j] == expectedArray[static_cast(j)]); + } + + auto maybeValue = uint8ArrayProperty.get(uv[0], uv[1]); + REQUIRE(maybeValue); + for (int64_t j = 0; j < maybeValue->size(); j++) { + REQUIRE((*maybeValue)[j] == normalize(value[j])); + } + } + } + + SECTION("Access with KHR_texture_transform") { + TextureViewOptions options; + options.applyKhrTextureTransformExtension = true; + + ExtensionKhrTextureTransform& extension = + propertyTextureProperty.addExtension(); + extension.offset = {0.5, -0.5}; + extension.rotation = CesiumUtility::Math::PiOverTwo; + extension.scale = {0.5, 0.5}; + extension.texCoord = 10; + + PropertyTexturePropertyView, true> + uint8ArrayProperty = + view.getPropertyView, true>( + "TestClassProperty", + options); + REQUIRE( + uint8ArrayProperty.status() == + PropertyTexturePropertyViewStatus::Valid); + + verifyTextureTransformConstruction(uint8ArrayProperty, extension); + + // This transforms to the following UV values: + // (0, 0) -> (0.5, -0.5) -> wraps to (0.5, 0.5) + // (1, 0) -> (0.5, -1) -> wraps to (0.5, 0) + // (0, 1) -> (1, -0.5) -> wraps to (0, 0.5) + // (1, 1) -> (1, -1) -> wraps to (0.0, 0.0) + std::vector texCoords{ + glm::dvec2(0, 0), + glm::dvec2(1, 0), + glm::dvec2(0, 1), + glm::dvec2(1, 1)}; + + std::vector> expectedTransformed{ + expected[3], + expected[1], + expected[2], + expected[0]}; + + for (size_t i = 0; i < texCoords.size(); ++i) { + glm::dvec2 uv = texCoords[i]; + const std::array& expectedArray = expectedTransformed[i]; + + const PropertyArrayView& value = + uint8ArrayProperty.getRaw(uv[0], uv[1]); + REQUIRE(static_cast(value.size()) == expectedArray.size()); + + for (int64_t j = 0; j < value.size(); j++) { + REQUIRE(value[j] == expectedArray[static_cast(j)]); + } + + auto maybeValue = uint8ArrayProperty.get(uv[0], uv[1]); + REQUIRE(maybeValue); + for (int64_t j = 0; j < maybeValue->size(); j++) { + REQUIRE((*maybeValue)[j] == normalize(value[j])); + } + } + + propertyTextureProperty.extensions.clear(); + } - auto dataStart = data.begin() + i * 3; - std::vector expected(dataStart, dataStart + 3); + SECTION("Access with image copy") { + TextureViewOptions options; + options.makeImageCopy = true; + + PropertyTexturePropertyView, true> + uint8ArrayProperty = + view.getPropertyView, true>( + "TestClassProperty", + options); + REQUIRE( + uint8ArrayProperty.status() == + PropertyTexturePropertyViewStatus::Valid); + + // Clear the original image data. + std::vector emptyData; + model.images[model.images.size() - 1].cesium.pixelData.swap(emptyData); + + std::vector texCoords{ + glm::dvec2(0, 0), + glm::dvec2(0.5, 0), + glm::dvec2(0, 0.5), + glm::dvec2(0.5, 0.5)}; + + for (size_t i = 0; i < texCoords.size(); ++i) { + glm::dvec2 uv = texCoords[i]; + const std::array& expectedArray = expected[i]; const PropertyArrayView& value = uint8ArrayProperty.getRaw(uv[0], uv[1]); - REQUIRE(static_cast(value.size()) == expected.size()); + REQUIRE(static_cast(value.size()) == expectedArray.size()); + for (int64_t j = 0; j < value.size(); j++) { - REQUIRE(value[j] == expected[static_cast(j)]); + REQUIRE(value[j] == expectedArray[static_cast(j)]); } auto maybeValue = uint8ArrayProperty.get(uv[0], uv[1]); @@ -1686,32 +2190,73 @@ TEST_CASE("Test callback for scalar PropertyTextureProperty") { glm::dvec2(0.5, 0.5)}; std::vector expected{-1, 268, 542, -256}; - uint32_t invokedCallbackCount = 0; - view.getPropertyView( - "TestClassProperty", - [&expected, &texCoords, &invokedCallbackCount]( - const std::string& /*propertyId*/, - auto propertyValue) mutable { - invokedCallbackCount++; - if constexpr (std::is_same_v< - PropertyTexturePropertyView, - decltype(propertyValue)>) { - REQUIRE( - propertyValue.status() == - PropertyTexturePropertyViewStatus::Valid); - for (size_t i = 0; i < expected.size(); ++i) { - glm::dvec2& uv = texCoords[i]; - REQUIRE(propertyValue.getRaw(uv[0], uv[1]) == expected[i]); - REQUIRE(propertyValue.get(uv[0], uv[1]) == expected[i]); + SECTION("Works") { + uint32_t invokedCallbackCount = 0; + view.getPropertyView( + "TestClassProperty", + [&expected, &texCoords, &invokedCallbackCount]( + const std::string& /*propertyId*/, + auto propertyValue) mutable { + invokedCallbackCount++; + if constexpr (std::is_same_v< + PropertyTexturePropertyView, + decltype(propertyValue)>) { + REQUIRE( + propertyValue.status() == + PropertyTexturePropertyViewStatus::Valid); + + for (size_t i = 0; i < expected.size(); ++i) { + glm::dvec2& uv = texCoords[i]; + REQUIRE(propertyValue.getRaw(uv[0], uv[1]) == expected[i]); + REQUIRE(propertyValue.get(uv[0], uv[1]) == expected[i]); + } + } else { + FAIL("getPropertyView returned PropertyTexturePropertyView of " + "incorrect type for TestClassProperty."); } - } else { - FAIL("getPropertyView returned PropertyTexturePropertyView of " - "incorrect type for TestClassProperty."); - } - }); + }); - REQUIRE(invokedCallbackCount == 1); + REQUIRE(invokedCallbackCount == 1); + } + + SECTION("Works with options") { + TextureViewOptions options; + options.makeImageCopy = true; + + uint32_t invokedCallbackCount = 0; + view.getPropertyView( + "TestClassProperty", + [&expected, &texCoords, &invokedCallbackCount, &model]( + const std::string& /*propertyId*/, + auto propertyValue) mutable { + invokedCallbackCount++; + if constexpr (std::is_same_v< + PropertyTexturePropertyView, + decltype(propertyValue)>) { + REQUIRE( + propertyValue.status() == + PropertyTexturePropertyViewStatus::Valid); + + // Clear the original image data. + std::vector emptyData; + model.images[model.images.size() - 1].cesium.pixelData.swap( + emptyData); + + for (size_t i = 0; i < expected.size(); ++i) { + glm::dvec2& uv = texCoords[i]; + REQUIRE(propertyValue.getRaw(uv[0], uv[1]) == expected[i]); + REQUIRE(propertyValue.get(uv[0], uv[1]) == expected[i]); + } + } else { + FAIL("getPropertyView returned PropertyTexturePropertyView of " + "incorrect type for TestClassProperty."); + } + }, + options); + + REQUIRE(invokedCallbackCount == 1); + } } TEST_CASE("Test callback for scalar PropertyTextureProperty (normalized)") { @@ -1766,32 +2311,75 @@ TEST_CASE("Test callback for scalar PropertyTextureProperty (normalized)") { glm::dvec2(0.5, 0.5)}; std::vector expected{-1, 268, 542, -256}; - uint32_t invokedCallbackCount = 0; - view.getPropertyView( - "TestClassProperty", - [&expected, &texCoords, &invokedCallbackCount]( - const std::string& /*propertyId*/, - auto propertyValue) mutable { - invokedCallbackCount++; - if constexpr (std::is_same_v< - PropertyTexturePropertyView, - decltype(propertyValue)>) { - REQUIRE( - propertyValue.status() == - PropertyTexturePropertyViewStatus::Valid); - for (size_t i = 0; i < expected.size(); ++i) { - glm::dvec2& uv = texCoords[i]; - REQUIRE(propertyValue.getRaw(uv[0], uv[1]) == expected[i]); - REQUIRE(propertyValue.get(uv[0], uv[1]) == normalize(expected[i])); + SECTION("Works") { + uint32_t invokedCallbackCount = 0; + view.getPropertyView( + "TestClassProperty", + [&expected, &texCoords, &invokedCallbackCount, &model]( + const std::string& /*propertyId*/, + auto propertyValue) mutable { + invokedCallbackCount++; + if constexpr (std::is_same_v< + PropertyTexturePropertyView, + decltype(propertyValue)>) { + REQUIRE( + propertyValue.status() == + PropertyTexturePropertyViewStatus::Valid); + + for (size_t i = 0; i < expected.size(); ++i) { + glm::dvec2& uv = texCoords[i]; + REQUIRE(propertyValue.getRaw(uv[0], uv[1]) == expected[i]); + REQUIRE( + propertyValue.get(uv[0], uv[1]) == normalize(expected[i])); + } + } else { + FAIL("getPropertyView returned PropertyTexturePropertyView of " + "incorrect type for TestClassProperty."); } - } else { - FAIL("getPropertyView returned PropertyTexturePropertyView of " - "incorrect type for TestClassProperty."); - } - }); + }); - REQUIRE(invokedCallbackCount == 1); + REQUIRE(invokedCallbackCount == 1); + } + + SECTION("Works with options") { + TextureViewOptions options; + options.makeImageCopy = true; + + uint32_t invokedCallbackCount = 0; + view.getPropertyView( + "TestClassProperty", + [&expected, &texCoords, &invokedCallbackCount, &model]( + const std::string& /*propertyId*/, + auto propertyValue) mutable { + invokedCallbackCount++; + if constexpr (std::is_same_v< + PropertyTexturePropertyView, + decltype(propertyValue)>) { + REQUIRE( + propertyValue.status() == + PropertyTexturePropertyViewStatus::Valid); + + // Clear the original image data. + std::vector emptyData; + model.images[model.images.size() - 1].cesium.pixelData.swap( + emptyData); + + for (size_t i = 0; i < expected.size(); ++i) { + glm::dvec2& uv = texCoords[i]; + REQUIRE(propertyValue.getRaw(uv[0], uv[1]) == expected[i]); + REQUIRE( + propertyValue.get(uv[0], uv[1]) == normalize(expected[i])); + } + } else { + FAIL("getPropertyView returned PropertyTexturePropertyView of " + "incorrect type for TestClassProperty."); + } + }, + options); + + REQUIRE(invokedCallbackCount == 1); + } } TEST_CASE("Test callback for vecN PropertyTextureProperty") { @@ -1856,32 +2444,72 @@ TEST_CASE("Test callback for vecN PropertyTextureProperty") { glm::dvec2(0, 0.5), glm::dvec2(0.5, 0.5)}; - uint32_t invokedCallbackCount = 0; - view.getPropertyView( - "TestClassProperty", - [&expected, &texCoords, &invokedCallbackCount]( - const std::string& /*propertyId*/, - auto propertyValue) mutable { - invokedCallbackCount++; - if constexpr (std::is_same_v< - PropertyTexturePropertyView, - decltype(propertyValue)>) { - REQUIRE( - propertyValue.status() == - PropertyTexturePropertyViewStatus::Valid); + SECTION("Works") { + uint32_t invokedCallbackCount = 0; + view.getPropertyView( + "TestClassProperty", + [&expected, &texCoords, &invokedCallbackCount]( + const std::string& /*propertyId*/, + auto propertyValue) mutable { + invokedCallbackCount++; + if constexpr (std::is_same_v< + PropertyTexturePropertyView, + decltype(propertyValue)>) { + REQUIRE( + propertyValue.status() == + PropertyTexturePropertyViewStatus::Valid); - for (size_t i = 0; i < expected.size(); ++i) { - glm::dvec2& uv = texCoords[i]; - REQUIRE(propertyValue.getRaw(uv[0], uv[1]) == expected[i]); - REQUIRE(propertyValue.get(uv[0], uv[1]) == expected[i]); + for (size_t i = 0; i < expected.size(); ++i) { + glm::dvec2& uv = texCoords[i]; + REQUIRE(propertyValue.getRaw(uv[0], uv[1]) == expected[i]); + REQUIRE(propertyValue.get(uv[0], uv[1]) == expected[i]); + } + } else { + FAIL("getPropertyView returned PropertyTexturePropertyView of " + "incorrect type for TestClassProperty."); } - } else { - FAIL("getPropertyView returned PropertyTexturePropertyView of " - "incorrect type for TestClassProperty."); - } - }); + }); - REQUIRE(invokedCallbackCount == 1); + REQUIRE(invokedCallbackCount == 1); + } + + SECTION("Works with options") { + TextureViewOptions options; + options.makeImageCopy = true; + + uint32_t invokedCallbackCount = 0; + view.getPropertyView( + "TestClassProperty", + [&expected, &texCoords, &invokedCallbackCount, &model]( + const std::string& /*propertyId*/, + auto propertyValue) mutable { + invokedCallbackCount++; + if constexpr (std::is_same_v< + PropertyTexturePropertyView, + decltype(propertyValue)>) { + REQUIRE( + propertyValue.status() == + PropertyTexturePropertyViewStatus::Valid); + + // Clear the original image data. + std::vector emptyData; + model.images[model.images.size() - 1].cesium.pixelData.swap( + emptyData); + + for (size_t i = 0; i < expected.size(); ++i) { + glm::dvec2& uv = texCoords[i]; + REQUIRE(propertyValue.getRaw(uv[0], uv[1]) == expected[i]); + REQUIRE(propertyValue.get(uv[0], uv[1]) == expected[i]); + } + } else { + FAIL("getPropertyView returned PropertyTexturePropertyView of " + "incorrect type for TestClassProperty."); + } + }, + options); + + REQUIRE(invokedCallbackCount == 1); + } } TEST_CASE("Test callback for vecN PropertyTextureProperty (normalized)") { @@ -1947,32 +2575,74 @@ TEST_CASE("Test callback for vecN PropertyTextureProperty (normalized)") { glm::dvec2(0, 0.5), glm::dvec2(0.5, 0.5)}; - uint32_t invokedCallbackCount = 0; - view.getPropertyView( - "TestClassProperty", - [&expected, &texCoords, &invokedCallbackCount]( - const std::string& /*propertyId*/, - auto propertyValue) mutable { - invokedCallbackCount++; - if constexpr (std::is_same_v< - PropertyTexturePropertyView, - decltype(propertyValue)>) { - REQUIRE( - propertyValue.status() == - PropertyTexturePropertyViewStatus::Valid); + SECTION("Works") { + uint32_t invokedCallbackCount = 0; + view.getPropertyView( + "TestClassProperty", + [&expected, &texCoords, &invokedCallbackCount]( + const std::string& /*propertyId*/, + auto propertyValue) mutable { + invokedCallbackCount++; + if constexpr (std::is_same_v< + PropertyTexturePropertyView, + decltype(propertyValue)>) { + REQUIRE( + propertyValue.status() == + PropertyTexturePropertyViewStatus::Valid); - for (size_t i = 0; i < expected.size(); ++i) { - glm::dvec2& uv = texCoords[i]; - REQUIRE(propertyValue.getRaw(uv[0], uv[1]) == expected[i]); - REQUIRE(propertyValue.get(uv[0], uv[1]) == normalize(expected[i])); + for (size_t i = 0; i < expected.size(); ++i) { + glm::dvec2& uv = texCoords[i]; + REQUIRE(propertyValue.getRaw(uv[0], uv[1]) == expected[i]); + REQUIRE( + propertyValue.get(uv[0], uv[1]) == normalize(expected[i])); + } + } else { + FAIL("getPropertyView returned PropertyTexturePropertyView of " + "incorrect type for TestClassProperty."); } - } else { - FAIL("getPropertyView returned PropertyTexturePropertyView of " - "incorrect type for TestClassProperty."); - } - }); + }); - REQUIRE(invokedCallbackCount == 1); + REQUIRE(invokedCallbackCount == 1); + } + + SECTION("Works with options") { + TextureViewOptions options; + options.makeImageCopy = true; + + uint32_t invokedCallbackCount = 0; + view.getPropertyView( + "TestClassProperty", + [&expected, &texCoords, &invokedCallbackCount, &model]( + const std::string& /*propertyId*/, + auto propertyValue) mutable { + invokedCallbackCount++; + if constexpr (std::is_same_v< + PropertyTexturePropertyView, + decltype(propertyValue)>) { + REQUIRE( + propertyValue.status() == + PropertyTexturePropertyViewStatus::Valid); + + // Clear the original image data. + std::vector emptyData; + model.images[model.images.size() - 1].cesium.pixelData.swap( + emptyData); + + for (size_t i = 0; i < expected.size(); ++i) { + glm::dvec2& uv = texCoords[i]; + REQUIRE(propertyValue.getRaw(uv[0], uv[1]) == expected[i]); + REQUIRE( + propertyValue.get(uv[0], uv[1]) == normalize(expected[i])); + } + } else { + FAIL("getPropertyView returned PropertyTexturePropertyView of " + "incorrect type for TestClassProperty."); + } + }, + options); + + REQUIRE(invokedCallbackCount == 1); + } } TEST_CASE("Test callback for array PropertyTextureProperty") { @@ -2039,49 +2709,108 @@ TEST_CASE("Test callback for array PropertyTextureProperty") { glm::dvec2(0, 0.5), glm::dvec2(0.5, 0.5)}; - uint32_t invokedCallbackCount = 0; - view.getPropertyView( - "TestClassProperty", - [&expected, &texCoords, &invokedCallbackCount]( - const std::string& /*propertyId*/, - auto propertyValue) mutable { - invokedCallbackCount++; - if constexpr (std::is_same_v< - PropertyTexturePropertyView< - PropertyArrayView>, - decltype(propertyValue)>) { - REQUIRE( - propertyValue.status() == - PropertyTexturePropertyViewStatus::Valid); + SECTION("Works") { + uint32_t invokedCallbackCount = 0; + view.getPropertyView( + "TestClassProperty", + [&expected, &texCoords, &invokedCallbackCount]( + const std::string& /*propertyId*/, + auto propertyValue) mutable { + invokedCallbackCount++; + if constexpr (std::is_same_v< + PropertyTexturePropertyView< + PropertyArrayView>, + decltype(propertyValue)>) { + REQUIRE( + propertyValue.status() == + PropertyTexturePropertyViewStatus::Valid); - for (size_t i = 0; i < expected.size(); ++i) { - std::vector& expectedArray = expected[i]; - glm::dvec2& uv = texCoords[i]; - PropertyArrayView array = - propertyValue.getRaw(uv[0], uv[1]); + for (size_t i = 0; i < expected.size(); ++i) { + std::vector& expectedArray = expected[i]; + glm::dvec2& uv = texCoords[i]; + PropertyArrayView array = + propertyValue.getRaw(uv[0], uv[1]); + + REQUIRE( + static_cast(array.size()) == expectedArray.size()); + for (int64_t j = 0; j < array.size(); j++) { + REQUIRE(array[j] == expectedArray[static_cast(j)]); + } - REQUIRE(static_cast(array.size()) == expectedArray.size()); - for (int64_t j = 0; j < array.size(); j++) { - REQUIRE(array[j] == expectedArray[static_cast(j)]); + auto maybeArray = propertyValue.get(uv[0], uv[1]); + REQUIRE(maybeArray); + REQUIRE( + static_cast(maybeArray->size()) == + expectedArray.size()); + for (int64_t j = 0; j < array.size(); j++) { + REQUIRE( + (*maybeArray)[j] == expectedArray[static_cast(j)]); + } } + } else { + FAIL("getPropertyView returned PropertyTexturePropertyView of " + "incorrect type for TestClassProperty."); + } + }); - auto maybeArray = propertyValue.get(uv[0], uv[1]); - REQUIRE(maybeArray); + REQUIRE(invokedCallbackCount == 1); + } + + SECTION("Works with options") { + TextureViewOptions options; + options.makeImageCopy = true; + + uint32_t invokedCallbackCount = 0; + view.getPropertyView( + "TestClassProperty", + [&expected, &texCoords, &invokedCallbackCount, &model]( + const std::string& /*propertyId*/, + auto propertyValue) mutable { + invokedCallbackCount++; + if constexpr (std::is_same_v< + PropertyTexturePropertyView< + PropertyArrayView>, + decltype(propertyValue)>) { REQUIRE( - static_cast(maybeArray->size()) == - expectedArray.size()); - for (int64_t j = 0; j < array.size(); j++) { + propertyValue.status() == + PropertyTexturePropertyViewStatus::Valid); + + // Clear the original image data. + std::vector emptyData; + model.images[model.images.size() - 1].cesium.pixelData.swap( + emptyData); + + for (size_t i = 0; i < expected.size(); ++i) { + std::vector& expectedArray = expected[i]; + glm::dvec2& uv = texCoords[i]; + PropertyArrayView array = + propertyValue.getRaw(uv[0], uv[1]); + REQUIRE( - (*maybeArray)[j] == expectedArray[static_cast(j)]); + static_cast(array.size()) == expectedArray.size()); + for (int64_t j = 0; j < array.size(); j++) { + REQUIRE(array[j] == expectedArray[static_cast(j)]); + } + + auto maybeArray = propertyValue.get(uv[0], uv[1]); + REQUIRE(maybeArray); + REQUIRE( + static_cast(maybeArray->size()) == + expectedArray.size()); + for (int64_t j = 0; j < array.size(); j++) { + REQUIRE( + (*maybeArray)[j] == expectedArray[static_cast(j)]); + } } + } else { + FAIL("getPropertyView returned PropertyTexturePropertyView of " + "incorrect type for TestClassProperty."); } - } else { - FAIL("getPropertyView returned PropertyTexturePropertyView of " - "incorrect type for TestClassProperty."); - } - }); + }, + options); - REQUIRE(invokedCallbackCount == 1); + REQUIRE(invokedCallbackCount == 1); + } } TEST_CASE("Test callback for array PropertyTextureProperty (normalized)") { @@ -2149,49 +2878,110 @@ TEST_CASE("Test callback for array PropertyTextureProperty (normalized)") { glm::dvec2(0, 0.5), glm::dvec2(0.5, 0.5)}; - uint32_t invokedCallbackCount = 0; - view.getPropertyView( - "TestClassProperty", - [&expected, &texCoords, &invokedCallbackCount]( - const std::string& /*propertyId*/, - auto propertyValue) mutable { - invokedCallbackCount++; - if constexpr ( - std::is_same_v< - PropertyTexturePropertyView, true>, - decltype(propertyValue)>) { - REQUIRE( - propertyValue.status() == - PropertyTexturePropertyViewStatus::Valid); + SECTION("Works") { + uint32_t invokedCallbackCount = 0; + view.getPropertyView( + "TestClassProperty", + [&expected, &texCoords, &invokedCallbackCount]( + const std::string& /*propertyId*/, + auto propertyValue) mutable { + invokedCallbackCount++; + if constexpr (std::is_same_v< + PropertyTexturePropertyView< + PropertyArrayView, + true>, + decltype(propertyValue)>) { + REQUIRE( + propertyValue.status() == + PropertyTexturePropertyViewStatus::Valid); - for (size_t i = 0; i < expected.size(); ++i) { - std::vector& expectedArray = expected[i]; - glm::dvec2& uv = texCoords[i]; - PropertyArrayView array = - propertyValue.getRaw(uv[0], uv[1]); + for (size_t i = 0; i < expected.size(); ++i) { + std::vector& expectedArray = expected[i]; + glm::dvec2& uv = texCoords[i]; + PropertyArrayView array = + propertyValue.getRaw(uv[0], uv[1]); + + REQUIRE( + static_cast(array.size()) == expectedArray.size()); + for (int64_t j = 0; j < array.size(); j++) { + REQUIRE(array[j] == expectedArray[static_cast(j)]); + } - REQUIRE(static_cast(array.size()) == expectedArray.size()); - for (int64_t j = 0; j < array.size(); j++) { - REQUIRE(array[j] == expectedArray[static_cast(j)]); + auto maybeArray = propertyValue.get(uv[0], uv[1]); + REQUIRE(maybeArray); + REQUIRE( + static_cast(maybeArray->size()) == + expectedArray.size()); + for (int64_t j = 0; j < array.size(); j++) { + auto rawValue = expectedArray[static_cast(j)]; + REQUIRE((*maybeArray)[j] == normalize(rawValue)); + } } + } else { + FAIL("getPropertyView returned PropertyTexturePropertyView of " + "incorrect type for TestClassProperty."); + } + }); + + REQUIRE(invokedCallbackCount == 1); + } - auto maybeArray = propertyValue.get(uv[0], uv[1]); - REQUIRE(maybeArray); + SECTION("Works with options") { + TextureViewOptions options; + options.makeImageCopy = true; + + uint32_t invokedCallbackCount = 0; + view.getPropertyView( + "TestClassProperty", + [&expected, &texCoords, &invokedCallbackCount, &model]( + const std::string& /*propertyId*/, + auto propertyValue) mutable { + invokedCallbackCount++; + if constexpr (std::is_same_v< + PropertyTexturePropertyView< + PropertyArrayView, + true>, + decltype(propertyValue)>) { REQUIRE( - static_cast(maybeArray->size()) == - expectedArray.size()); - for (int64_t j = 0; j < array.size(); j++) { - auto rawValue = expectedArray[static_cast(j)]; - REQUIRE((*maybeArray)[j] == normalize(rawValue)); + propertyValue.status() == + PropertyTexturePropertyViewStatus::Valid); + + // Clear the original image data. + std::vector emptyData; + model.images[model.images.size() - 1].cesium.pixelData.swap( + emptyData); + + for (size_t i = 0; i < expected.size(); ++i) { + std::vector& expectedArray = expected[i]; + glm::dvec2& uv = texCoords[i]; + PropertyArrayView array = + propertyValue.getRaw(uv[0], uv[1]); + + REQUIRE( + static_cast(array.size()) == expectedArray.size()); + for (int64_t j = 0; j < array.size(); j++) { + REQUIRE(array[j] == expectedArray[static_cast(j)]); + } + + auto maybeArray = propertyValue.get(uv[0], uv[1]); + REQUIRE(maybeArray); + REQUIRE( + static_cast(maybeArray->size()) == + expectedArray.size()); + for (int64_t j = 0; j < array.size(); j++) { + auto rawValue = expectedArray[static_cast(j)]; + REQUIRE((*maybeArray)[j] == normalize(rawValue)); + } } + } else { + FAIL("getPropertyView returned PropertyTexturePropertyView of " + "incorrect type for TestClassProperty."); } - } else { - FAIL("getPropertyView returned PropertyTexturePropertyView of " - "incorrect type for TestClassProperty."); - } - }); + }, + options); - REQUIRE(invokedCallbackCount == 1); + REQUIRE(invokedCallbackCount == 1); + } } TEST_CASE("Test callback on unsupported PropertyTextureProperty") { From 0c733fb79c79b19dcc405cb5f3342139432f0e2f Mon Sep 17 00:00:00 2001 From: Janine Liu Date: Wed, 28 Feb 2024 17:25:02 -0500 Subject: [PATCH 09/12] Update changelog --- CHANGES.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 621e0443e..76eef4ca1 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -9,9 +9,11 @@ ##### Additions :tada: +- Added `TextureViewOptions`, which includes the following flags: + - `applyKhrTextureTransformExtension` . When true, the view will automatically transform texture coordinates before sampling the texture. + - `makeImageCopy`. When true, the view will make its own CPU copy of the image data. +- Added `TextureView`, which views an arbitrary glTF texture and can be affected by `TextureViewOptions`. `FeatureIdTextureView` and `PropertyTexturePropertyView` now inherit this class. - Added `KhrTextureTransform`, a utility class that parses the `KHR_texture_transform` glTF extension and reports whether it is valid. UVs may be transformed on the CPU using `applyTransform`. -- Added `applyKhrTextureTransformExtension` to the constructors of `FeatureIdTextureView` and `PropertyTexturePropertyView`. When true, the views will automatically transform texture coordinates before retrieving features and metadata. This is false by default to avoid breaking existing implementations. -- Added `getTextureTransform` methods to `FeatureIdTextureView` and `PropertyTexturePropertyView`. - Added `contains` method to `BoundingSphere`. - Added `GlobeRectangle::MAXIMUM` static field. - Added `ReferenceCountedThreadSafe` type alias. From 90531bb6fd2e6469598b0f1a0a5e62fb3487e605 Mon Sep 17 00:00:00 2001 From: Janine Liu Date: Wed, 28 Feb 2024 17:37:39 -0500 Subject: [PATCH 10/12] Minor tweaks --- CHANGES.md | 1 + CesiumGltf/test/TestPropertyTextureView.cpp | 5 ++--- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 76eef4ca1..1a0c61b0c 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -13,6 +13,7 @@ - `applyKhrTextureTransformExtension` . When true, the view will automatically transform texture coordinates before sampling the texture. - `makeImageCopy`. When true, the view will make its own CPU copy of the image data. - Added `TextureView`, which views an arbitrary glTF texture and can be affected by `TextureViewOptions`. `FeatureIdTextureView` and `PropertyTexturePropertyView` now inherit this class. +- Added `options` parameter to `PropertyTextureView::getPropertyView` and `PropertyTextureView::forEachProperty`, which allow views to be constructed with property-specific options. - Added `KhrTextureTransform`, a utility class that parses the `KHR_texture_transform` glTF extension and reports whether it is valid. UVs may be transformed on the CPU using `applyTransform`. - Added `contains` method to `BoundingSphere`. - Added `GlobeRectangle::MAXIMUM` static field. diff --git a/CesiumGltf/test/TestPropertyTextureView.cpp b/CesiumGltf/test/TestPropertyTextureView.cpp index 469707f82..fe0773b71 100644 --- a/CesiumGltf/test/TestPropertyTextureView.cpp +++ b/CesiumGltf/test/TestPropertyTextureView.cpp @@ -1323,9 +1323,8 @@ TEST_CASE("Test array PropertyTextureProperty (normalized)") { glm::dvec2(0, 0.5), glm::dvec2(0.5, 0.5)}; - int64_t size = static_cast(texCoords.size()); - for (int64_t i = 0; i < size; ++i) { - glm::dvec2 uv = texCoords[static_cast(i)]; + for (size_t i = 0; i < texCoords.size(); ++i) { + glm::dvec2 uv = texCoords[i]; const std::array& expectedArray = expected[i]; const PropertyArrayView& value = From 636c40f97380fa0990052adcfb8d7c4b38f9560c Mon Sep 17 00:00:00 2001 From: Janine Liu Date: Wed, 28 Feb 2024 17:47:48 -0500 Subject: [PATCH 11/12] Fix unused lambda variable --- CesiumGltf/test/TestPropertyTextureView.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CesiumGltf/test/TestPropertyTextureView.cpp b/CesiumGltf/test/TestPropertyTextureView.cpp index fe0773b71..a163fb0b2 100644 --- a/CesiumGltf/test/TestPropertyTextureView.cpp +++ b/CesiumGltf/test/TestPropertyTextureView.cpp @@ -2315,7 +2315,7 @@ TEST_CASE("Test callback for scalar PropertyTextureProperty (normalized)") { uint32_t invokedCallbackCount = 0; view.getPropertyView( "TestClassProperty", - [&expected, &texCoords, &invokedCallbackCount, &model]( + [&expected, &texCoords, &invokedCallbackCount]( const std::string& /*propertyId*/, auto propertyValue) mutable { invokedCallbackCount++; From f17e4846b1fd794e67b7f324fbda4e129ac71a85 Mon Sep 17 00:00:00 2001 From: Janine Liu Date: Thu, 29 Feb 2024 10:12:09 -0500 Subject: [PATCH 12/12] Minor tweaks from PR review --- CesiumGltf/include/CesiumGltf/FeatureIdTextureView.h | 2 +- .../include/CesiumGltf/PropertyTexturePropertyView.h | 9 ++++----- CesiumGltf/include/CesiumGltf/PropertyTextureView.h | 10 +++++----- CesiumGltf/include/CesiumGltf/TextureView.h | 11 ++++------- CesiumGltf/src/FeatureIdTextureView.cpp | 2 +- CesiumGltf/src/TextureView.cpp | 8 ++++---- 6 files changed, 19 insertions(+), 23 deletions(-) diff --git a/CesiumGltf/include/CesiumGltf/FeatureIdTextureView.h b/CesiumGltf/include/CesiumGltf/FeatureIdTextureView.h index f4fb54c2d..807838618 100644 --- a/CesiumGltf/include/CesiumGltf/FeatureIdTextureView.h +++ b/CesiumGltf/include/CesiumGltf/FeatureIdTextureView.h @@ -111,7 +111,7 @@ class FeatureIdTextureView : public TextureView { FeatureIdTextureView( const Model& model, const FeatureIdTexture& featureIdTexture, - TextureViewOptions options = TextureViewOptions()) noexcept; + const TextureViewOptions& options = TextureViewOptions()) noexcept; /** * @brief Get the feature ID from the texture at the given texture diff --git a/CesiumGltf/include/CesiumGltf/PropertyTexturePropertyView.h b/CesiumGltf/include/CesiumGltf/PropertyTexturePropertyView.h index 588d57755..4841c4826 100644 --- a/CesiumGltf/include/CesiumGltf/PropertyTexturePropertyView.h +++ b/CesiumGltf/include/CesiumGltf/PropertyTexturePropertyView.h @@ -294,7 +294,7 @@ class PropertyTexturePropertyView const ClassProperty& classProperty, const Sampler& sampler, const ImageCesium& image, - TextureViewOptions options = TextureViewOptions()) noexcept + const TextureViewOptions& options = TextureViewOptions()) noexcept : PropertyView(classProperty, property), TextureView( sampler, @@ -482,9 +482,8 @@ class PropertyTexturePropertyView /** * @brief Constructs an instance of an empty property that specifies a * default value. Although this property has no data, it can return the - * default value - * when {@link PropertyTexturePropertyView::get} is called. However, - * {@link PropertyTexturePropertyView::getRaw} cannot be used. + * default value when {@link PropertyTexturePropertyView::get} is called. + * However, {@link PropertyTexturePropertyView::getRaw} cannot be used. * * @param classProperty The {@link ClassProperty} this property conforms to. */ @@ -526,7 +525,7 @@ class PropertyTexturePropertyView const ClassProperty& classProperty, const Sampler& sampler, const ImageCesium& image, - TextureViewOptions options = TextureViewOptions()) noexcept + const TextureViewOptions& options = TextureViewOptions()) noexcept : PropertyView(classProperty, property), TextureView( sampler, diff --git a/CesiumGltf/include/CesiumGltf/PropertyTextureView.h b/CesiumGltf/include/CesiumGltf/PropertyTextureView.h index e6613efc6..2f2ac0d72 100644 --- a/CesiumGltf/include/CesiumGltf/PropertyTextureView.h +++ b/CesiumGltf/include/CesiumGltf/PropertyTextureView.h @@ -3,10 +3,10 @@ #include "CesiumGltf/Class.h" #include "CesiumGltf/ClassProperty.h" #include "CesiumGltf/ExtensionModelExtStructuralMetadata.h" +#include "CesiumGltf/Model.h" #include "CesiumGltf/PropertyTexture.h" #include "CesiumGltf/PropertyTexturePropertyView.h" #include "CesiumGltf/TextureView.h" -#include "Model.h" namespace CesiumGltf { /** @@ -121,7 +121,7 @@ class PropertyTextureView { template PropertyTexturePropertyView getPropertyView( const std::string& propertyId, - TextureViewOptions propertyOptions = TextureViewOptions()) const { + const TextureViewOptions& propertyOptions = TextureViewOptions()) const { if (this->_status != PropertyTextureViewStatus::Valid) { return PropertyTexturePropertyView( PropertyTexturePropertyViewStatus::ErrorInvalidPropertyTexture); @@ -163,7 +163,7 @@ class PropertyTextureView { void getPropertyView( const std::string& propertyId, Callback&& callback, - TextureViewOptions propertyOptions = TextureViewOptions()) const { + const TextureViewOptions& propertyOptions = TextureViewOptions()) const { if (this->_status != PropertyTextureViewStatus::Valid) { callback( propertyId, @@ -290,7 +290,7 @@ class PropertyTextureView { template void forEachProperty( Callback&& callback, - TextureViewOptions propertyOptions = TextureViewOptions()) const { + const TextureViewOptions& propertyOptions = TextureViewOptions()) const { for (const auto& property : this->_pClass->properties) { getPropertyView( property.first, @@ -586,7 +586,7 @@ class PropertyTextureView { PropertyTexturePropertyView createScalarPropertyView( const ClassProperty& classProperty, [[maybe_unused]] const PropertyTextureProperty& propertyTextureProperty, - TextureViewOptions propertyOptions) const { + const TextureViewOptions& propertyOptions) const { if (classProperty.array) { return PropertyTexturePropertyView( PropertyTexturePropertyViewStatus::ErrorArrayTypeMismatch); diff --git a/CesiumGltf/include/CesiumGltf/TextureView.h b/CesiumGltf/include/CesiumGltf/TextureView.h index d8644fc9f..9eb15af19 100644 --- a/CesiumGltf/include/CesiumGltf/TextureView.h +++ b/CesiumGltf/include/CesiumGltf/TextureView.h @@ -33,7 +33,7 @@ struct TextureViewOptions { * the original TEXCOORD set index will be preserved. The extension's values * may still be retrieved using getTextureTransform, if desired. */ - bool applyKhrTextureTransformExtension; + bool applyKhrTextureTransformExtension = false; /** * @brief Whether to copy the input image. @@ -43,10 +43,7 @@ struct TextureViewOptions { * original glTF model. When this flag is true, the view will manage its own * copy of the pixel data to avoid such issues. */ - bool makeImageCopy; - - TextureViewOptions() - : applyKhrTextureTransformExtension(false), makeImageCopy(false) {} + bool makeImageCopy = false; }; /** @@ -111,7 +108,7 @@ class TextureView { TextureView( const Model& model, const TextureInfo& textureInfo, - TextureViewOptions options = TextureViewOptions()) noexcept; + const TextureViewOptions& options = TextureViewOptions()) noexcept; /** * @brief Constructs a view of the texture specified by the given {@link Sampler} @@ -131,7 +128,7 @@ class TextureView { int64_t textureCoordinateSetIndex, const ExtensionKhrTextureTransform* pKhrTextureTransformExtension = nullptr, - TextureViewOptions options = TextureViewOptions()) noexcept; + const TextureViewOptions& options = TextureViewOptions()) noexcept; /** * @brief Get the status of this texture view. diff --git a/CesiumGltf/src/FeatureIdTextureView.cpp b/CesiumGltf/src/FeatureIdTextureView.cpp index 58c6a2cf1..ab7719e47 100644 --- a/CesiumGltf/src/FeatureIdTextureView.cpp +++ b/CesiumGltf/src/FeatureIdTextureView.cpp @@ -13,7 +13,7 @@ FeatureIdTextureView::FeatureIdTextureView() noexcept FeatureIdTextureView::FeatureIdTextureView( const Model& model, const FeatureIdTexture& featureIdTexture, - TextureViewOptions options) noexcept + const TextureViewOptions& options) noexcept : TextureView(model, featureIdTexture, options), _status(FeatureIdTextureViewStatus::ErrorUninitialized), _channels() { diff --git a/CesiumGltf/src/TextureView.cpp b/CesiumGltf/src/TextureView.cpp index 7727243db..826a69c5c 100644 --- a/CesiumGltf/src/TextureView.cpp +++ b/CesiumGltf/src/TextureView.cpp @@ -17,7 +17,7 @@ TextureView::TextureView() noexcept TextureView::TextureView( const Model& model, const TextureInfo& textureInfo, - TextureViewOptions options) noexcept + const TextureViewOptions& options) noexcept : _textureViewStatus(TextureViewStatus::ErrorUninitialized), _pSampler(nullptr), _pImage(nullptr), @@ -69,7 +69,7 @@ TextureView::TextureView( } if (options.makeImageCopy) { - this->_imageCopy = ImageCesium(*this->_pImage); + this->_imageCopy = *this->_pImage; } this->_textureViewStatus = TextureViewStatus::Valid; @@ -80,7 +80,7 @@ TextureView::TextureView( const ImageCesium& image, int64_t textureCoordinateSetIndex, const ExtensionKhrTextureTransform* pKhrTextureTransformExtension, - TextureViewOptions options) noexcept + const TextureViewOptions& options) noexcept : _textureViewStatus(TextureViewStatus::ErrorUninitialized), _pSampler(&sampler), _pImage(&image), @@ -102,7 +102,7 @@ TextureView::TextureView( } if (options.makeImageCopy) { - this->_imageCopy = ImageCesium(*this->_pImage); + this->_imageCopy = *this->_pImage; } this->_textureViewStatus = TextureViewStatus::Valid;