ImageDecoder cleanup.

CesiumGS · Oct 30, 2024 · 9dfe894 · 9dfe894
1 parent e3c3daf
commit 9dfe894
Show file tree

Hide file tree

Showing 10 changed files with 118 additions and 111 deletions.
diff --git a/CesiumGltf/CMakeLists.txt b/CesiumGltf/CMakeLists.txt
@@ -22,8 +22,8 @@ cesium_glob_files(CESIUM_GLTF_TEST_HEADERS
 
 set_target_properties(CesiumGltf
     PROPERTIES
-    TEST_SOURCES "${CESIUM_GLTF_TEST_SOURCES}"
-    TEST_HEADERS "${CESIUM_GLTF_TEST_HEADERS}"
+        TEST_SOURCES "${CESIUM_GLTF_TEST_SOURCES}"
+        TEST_HEADERS "${CESIUM_GLTF_TEST_HEADERS}"
 )
 
 set_target_properties(CesiumGltf
@@ -34,27 +34,27 @@ set_target_properties(CesiumGltf
 target_sources(
     CesiumGltf
     PRIVATE
-    ${CESIUM_GLTF_SOURCES}
-    ${CESIUM_GLTF_HEADERS}
+        ${CESIUM_GLTF_SOURCES}
+        ${CESIUM_GLTF_HEADERS}
     PUBLIC
-    ${CESIUM_GLTF_PUBLIC_HEADERS}
+        ${CESIUM_GLTF_PUBLIC_HEADERS}
 )
 
 target_include_directories(
     CesiumGltf
     SYSTEM PUBLIC
-    ${CMAKE_CURRENT_LIST_DIR}/include/
-    ${CMAKE_CURRENT_LIST_DIR}/generated/include
+        ${CMAKE_CURRENT_LIST_DIR}/include/
+        ${CMAKE_CURRENT_LIST_DIR}/generated/include
     PRIVATE
-    ${CESIUM_NATIVE_RAPIDJSON_INCLUDE_DIR}
-    ${CMAKE_CURRENT_LIST_DIR}/src
-    ${CMAKE_CURRENT_LIST_DIR}/generated/src
+        ${CESIUM_NATIVE_RAPIDJSON_INCLUDE_DIR}
+        ${CMAKE_CURRENT_LIST_DIR}/src
+        ${CMAKE_CURRENT_LIST_DIR}/generated/src
 )
 
 target_link_libraries(CesiumGltf
     PUBLIC
-    CesiumUtility
-    Microsoft.GSL::GSL
+        CesiumUtility
+        Microsoft.GSL::GSL
 )
 
 target_compile_definitions(CesiumGltf PRIVATE GLM_ENABLE_EXPERIMENTAL)
diff --git a/CesiumGltfReader/include/CesiumGltfReader/GltfReader.h b/CesiumGltfReader/include/CesiumGltfReader/GltfReader.h
@@ -198,28 +198,22 @@ class CESIUMGLTFREADER_API GltfReader {
       GltfReaderResult&& result);
 
   /**
-   * Reads an Image from a buffer.
+   * @brief Reads an Image from a buffer.
    * @deprecated Use {@link ImageDecoder::readImage} instead.
    */
-  static ImageReaderResult readImage(
+  [[deprecated(
+      "Use ImageDecoder::readImage instead.")]] static ImageReaderResult
+  readImage(
       const gsl::span<const std::byte>& data,
-      const CesiumGltf::Ktx2TranscodeTargets& ktx2TranscodeTargets) {
-    return ImageDecoder::readImage(data, ktx2TranscodeTargets);
-  }
+      const CesiumGltf::Ktx2TranscodeTargets& ktx2TranscodeTargets);
 
   /**
    * @brief Generate mipmaps for this image.
-   *
-   * Does nothing if mipmaps already exist or the compressedPixelFormat is not
-   * GpuCompressedPixelFormat::NONE.
-   *
-   * @param image The image to generate mipmaps for.   *
-   * @return A string describing the error, if unable to generate mipmaps.
+   * @deprecated Use {@link ImageDecoder::generateMipMaps} instead.
    */
-  static std::optional<std::string>
-  generateMipMaps(CesiumGltf::ImageAsset& image) {
-    return ImageDecoder::generateMipMaps(image);
-  }
+  [[deprecated("Use ImageDecoder::generateMipMaps instead.")]] static std::
+      optional<std::string>
+      generateMipMaps(CesiumGltf::ImageAsset& image);
 
 private:
   CesiumJsonReader::JsonReaderOptions _context;

diff --git a/CesiumGltfReader/include/CesiumGltfReader/ImageDecoder.h b/CesiumGltfReader/include/CesiumGltfReader/ImageDecoder.h
@@ -1,3 +1,5 @@
+#pragma once
+
 #include "CesiumGltf/ImageAsset.h"
 #include "CesiumGltfReader/Library.h"
 

diff --git a/CesiumGltfReader/src/GltfReader.cpp b/CesiumGltfReader/src/GltfReader.cpp
@@ -234,10 +234,7 @@ GltfReaderResult readBinaryGltf(
   return result;
 }
 
-void postprocess(
-    const GltfReader& reader,
-    GltfReaderResult& readGltf,
-    const GltfReaderOptions& options) {
+void postprocess(GltfReaderResult& readGltf, const GltfReaderOptions& options) {
   Model& model = readGltf.model.value();
 
   auto extFeatureMetadataIter = std::find(
@@ -253,7 +250,7 @@ void postprocess(
   }
 
   if (options.decodeDataUrls) {
-    decodeDataUrls(reader, readGltf, options);
+    decodeDataUrls(readGltf, options);
   }
 
   if (options.decodeEmbeddedImages) {
@@ -381,7 +378,7 @@ GltfReaderResult GltfReader::readGltf(
                                                : readJsonGltf(context, data);
 
   if (result.model) {
-    postprocess(*this, result, options);
+    postprocess(result, options);
   }
 
   return result;
@@ -437,7 +434,7 @@ CesiumAsync::Future<GltfReaderResult> GltfReader::loadGltf(
                 std::move(result));
           })
       .thenInWorkerThread([options, this](GltfReaderResult&& result) {
-        postprocess(*this, result, options);
+        postprocess(result, options);
         return std::move(result);
       });
 }
@@ -446,7 +443,7 @@ void CesiumGltfReader::GltfReader::postprocessGltf(
     GltfReaderResult& readGltf,
     const GltfReaderOptions& options) {
   if (readGltf.model) {
-    postprocess(*this, readGltf, options);
+    postprocess(readGltf, options);
   }
 }
 
@@ -623,3 +620,14 @@ void CesiumGltfReader::GltfReader::postprocessGltf(
             return std::move(*pResult.release());
           });
 }
+
+/*static*/ ImageReaderResult GltfReader::readImage(
+    const gsl::span<const std::byte>& data,
+    const Ktx2TranscodeTargets& ktx2TranscodeTargets) {
+  return ImageDecoder::readImage(data, ktx2TranscodeTargets);
+}
+
+/*static*/ std::optional<std::string>
+GltfReader::generateMipMaps(CesiumGltf::ImageAsset& image) {
+  return ImageDecoder::generateMipMaps(image);
+}
diff --git a/CesiumGltfReader/src/decodeDataUrls.cpp b/CesiumGltfReader/src/decodeDataUrls.cpp
@@ -1,8 +1,8 @@
 #include "decodeDataUrls.h"
 
-#include "CesiumGltfReader/GltfReader.h"
-
 #include <CesiumGltf/Model.h>
+#include <CesiumGltfReader/GltfReader.h>
+#include <CesiumGltfReader/ImageDecoder.h>
 #include <CesiumUtility/Tracing.h>
 
 #include <modp_b64.h>
@@ -87,7 +87,6 @@ std::optional<DecodeResult> tryDecode(const std::string& uri) {
 } // namespace
 
 void decodeDataUrls(
-    const GltfReader& reader,
     GltfReaderResult& readGltf,
     const GltfReaderOptions& options) {
   CESIUM_TRACE("CesiumGltfReader::decodeDataUrls");
@@ -134,8 +133,9 @@ void decodeDataUrls(
       continue;
     }
 
-    ImageReaderResult imageResult =
-        reader.readImage(decoded.value().data, options.ktx2TranscodeTargets);
+    ImageReaderResult imageResult = ImageDecoder::readImage(
+        decoded.value().data,
+        options.ktx2TranscodeTargets);
 
     if (!imageResult.pImage) {
       continue;

diff --git a/CesiumGltfReader/src/decodeDataUrls.h b/CesiumGltfReader/src/decodeDataUrls.h
@@ -7,7 +7,6 @@ struct GltfReaderOptions;
 class GltfReader;
 
 void decodeDataUrls(
-    const GltfReader& reader,
     GltfReaderResult& readGltf,
     const GltfReaderOptions& clearDecodedDataUrls);
 } // namespace CesiumGltfReader
diff --git a/CesiumGltfReader/test/TestGltfReader.cpp b/CesiumGltfReader/test/TestGltfReader.cpp
@@ -563,67 +563,6 @@ TEST_CASE("Can apply RTC CENTER if model uses Cesium RTC extension") {
   CHECK(cesiumRTC->center == rtcCenter);
 }
 
-TEST_CASE("Can correctly interpret mipmaps in KTX2 files") {
-  {
-    // This KTX2 file has a single mip level and no further mip levels should be
-    // generated. `mipPositions` should reflect this single mip level.
-    std::filesystem::path ktx2File = CesiumGltfReader_TEST_DATA_DIR;
-    ktx2File /= "ktx2/kota-onelevel.ktx2";
-    std::vector<std::byte> data = readFile(ktx2File.string());
-    ImageReaderResult imageResult =
-        GltfReader::readImage(data, Ktx2TranscodeTargets{});
-    REQUIRE(imageResult.pImage);
-
-    const ImageAsset& image = *imageResult.pImage;
-    REQUIRE(image.mipPositions.size() == 1);
-    CHECK(image.mipPositions[0].byteOffset == 0);
-    CHECK(image.mipPositions[0].byteSize > 0);
-    CHECK(
-        image.mipPositions[0].byteSize ==
-        size_t(image.width * image.height * image.channels));
-    CHECK(image.mipPositions[0].byteSize == image.pixelData.size());
-  }
-
-  {
-    // This KTX2 file has only a base image but further mip levels can be
-    // generated. This image effectively has no mip levels.
-    std::filesystem::path ktx2File = CesiumGltfReader_TEST_DATA_DIR;
-    ktx2File /= "ktx2/kota-automipmap.ktx2";
-    std::vector<std::byte> data = readFile(ktx2File.string());
-    ImageReaderResult imageResult =
-        GltfReader::readImage(data, Ktx2TranscodeTargets{});
-    REQUIRE(imageResult.pImage);
-
-    const ImageAsset& image = *imageResult.pImage;
-    REQUIRE(image.mipPositions.size() == 0);
-    CHECK(image.pixelData.size() > 0);
-  }
-
-  {
-    // This KTX2 file has a complete mip chain.
-    std::filesystem::path ktx2File = CesiumGltfReader_TEST_DATA_DIR;
-    ktx2File /= "ktx2/kota-mipmaps.ktx2";
-    std::vector<std::byte> data = readFile(ktx2File.string());
-    ImageReaderResult imageResult =
-        GltfReader::readImage(data, Ktx2TranscodeTargets{});
-    REQUIRE(imageResult.pImage);
-
-    const ImageAsset& image = *imageResult.pImage;
-    REQUIRE(image.mipPositions.size() == 9);
-    CHECK(image.mipPositions[0].byteSize > 0);
-    CHECK(
-        image.mipPositions[0].byteSize ==
-        size_t(image.width * image.height * image.channels));
-    CHECK(image.mipPositions[0].byteSize < image.pixelData.size());
-
-    size_t smallerThan = image.mipPositions[0].byteSize;
-    for (size_t i = 1; i < image.mipPositions.size(); ++i) {
-      CHECK(image.mipPositions[i].byteSize < smallerThan);
-      smallerThan = image.mipPositions[i].byteSize;
-    }
-  }
-}
-
 TEST_CASE("Can read unknown properties from a glTF") {
   const std::string s = R"(
     {

diff --git a/CesiumGltfReader/test/TestImageDecoder.cpp b/CesiumGltfReader/test/TestImageDecoder.cpp
@@ -0,0 +1,72 @@
+#include <CesiumGltfReader/ImageDecoder.h>
+#include <CesiumNativeTests/readFile.h>
+
+#include <catch2/catch.hpp>
+
+#include <filesystem>
+
+using namespace CesiumGltf;
+using namespace CesiumGltfReader;
+
+TEST_CASE("CesiumGltfReader::ImageDecoder") {
+  SECTION("Can correctly interpret mipmaps in KTX2 files") {
+    {
+      // This KTX2 file has a single mip level and no further mip levels should
+      // be generated. `mipPositions` should reflect this single mip level.
+      std::filesystem::path ktx2File = CesiumGltfReader_TEST_DATA_DIR;
+      ktx2File /= "ktx2/kota-onelevel.ktx2";
+      std::vector<std::byte> data = readFile(ktx2File.string());
+      ImageReaderResult imageResult =
+          ImageDecoder::readImage(data, Ktx2TranscodeTargets{});
+      REQUIRE(imageResult.pImage);
+
+      const ImageAsset& image = *imageResult.pImage;
+      REQUIRE(image.mipPositions.size() == 1);
+      CHECK(image.mipPositions[0].byteOffset == 0);
+      CHECK(image.mipPositions[0].byteSize > 0);
+      CHECK(
+          image.mipPositions[0].byteSize ==
+          size_t(image.width * image.height * image.channels));
+      CHECK(image.mipPositions[0].byteSize == image.pixelData.size());
+    }
+
+    {
+      // This KTX2 file has only a base image but further mip levels can be
+      // generated. This image effectively has no mip levels.
+      std::filesystem::path ktx2File = CesiumGltfReader_TEST_DATA_DIR;
+      ktx2File /= "ktx2/kota-automipmap.ktx2";
+      std::vector<std::byte> data = readFile(ktx2File.string());
+      ImageReaderResult imageResult =
+          ImageDecoder::readImage(data, Ktx2TranscodeTargets{});
+      REQUIRE(imageResult.pImage);
+
+      const ImageAsset& image = *imageResult.pImage;
+      REQUIRE(image.mipPositions.size() == 0);
+      CHECK(image.pixelData.size() > 0);
+    }
+
+    {
+      // This KTX2 file has a complete mip chain.
+      std::filesystem::path ktx2File = CesiumGltfReader_TEST_DATA_DIR;
+      ktx2File /= "ktx2/kota-mipmaps.ktx2";
+      std::vector<std::byte> data = readFile(ktx2File.string());
+      ImageReaderResult imageResult =
+          ImageDecoder::readImage(data, Ktx2TranscodeTargets{});
+      REQUIRE(imageResult.pImage);
+
+      const ImageAsset& image = *imageResult.pImage;
+      REQUIRE(image.mipPositions.size() == 9);
+      CHECK(image.mipPositions[0].byteSize > 0);
+      CHECK(
+          image.mipPositions[0].byteSize ==
+          size_t(image.width * image.height * image.channels));
+      CHECK(image.mipPositions[0].byteSize < image.pixelData.size());
+
+      size_t smallerThan = image.mipPositions[0].byteSize;
+      for (size_t i = 1; i < image.mipPositions.size(); ++i) {
+        CHECK(image.mipPositions[i].byteSize < smallerThan);
+        smallerThan = image.mipPositions[i].byteSize;
+      }
+    }
+  }
+}
diff --git a/CesiumRasterOverlays/include/CesiumRasterOverlays/RasterOverlayTileProvider.h b/CesiumRasterOverlays/include/CesiumRasterOverlays/RasterOverlayTileProvider.h
@@ -414,7 +414,5 @@ class CESIUMRASTEROVERLAYS_API RasterOverlayTileProvider
   CESIUM_TRACE_DECLARE_TRACK_SET(
       _loadingSlots,
       "Raster Overlay Tile Loading Slot");
-
-  static CesiumGltfReader::GltfReader _gltfReader;
 };
 } // namespace CesiumRasterOverlays
diff --git a/CesiumRasterOverlays/src/RasterOverlayTileProvider.cpp b/CesiumRasterOverlays/src/RasterOverlayTileProvider.cpp
@@ -1,5 +1,5 @@
 #include <CesiumAsync/IAssetResponse.h>
-#include <CesiumGltfReader/GltfReader.h>
+#include <CesiumGltfReader/ImageDecoder.h>
 #include <CesiumRasterOverlays/IPrepareRasterOverlayRendererResources.h>
 #include <CesiumRasterOverlays/RasterOverlay.h>
 #include <CesiumRasterOverlays/RasterOverlayTile.h>
@@ -18,9 +18,6 @@ using namespace CesiumUtility;
 
 namespace CesiumRasterOverlays {
 
-/*static*/ CesiumGltfReader::GltfReader
-    RasterOverlayTileProvider::_gltfReader{};
-
 RasterOverlayTileProvider::RasterOverlayTileProvider(
     const CesiumUtility::IntrusivePointer<const RasterOverlay>& pOwner,
     const CesiumAsync::AsyncSystem& asyncSystem,
@@ -189,9 +186,7 @@ RasterOverlayTileProvider::loadTileImageFromUrl(
             const gsl::span<const std::byte> data = pResponse->data();
 
             CesiumGltfReader::ImageReaderResult loadedImage =
-                RasterOverlayTileProvider::_gltfReader.readImage(
-                    data,
-                    Ktx2TranscodeTargets);
+                ImageDecoder::readImage(data, Ktx2TranscodeTargets);
 
             if (!loadedImage.errors.empty()) {
               loadedImage.errors.push_back("Image url: " + pRequest->url());