Change reader to return unique_ptr<File>

Author: mikerreed

include/rive/file.hpp

@@ -55,13 +55,12 @@ namespace rive {
         ///
         /// Imports a Rive file from a binary buffer.
         /// @param reader a pointer to a binary reader attached to the file.
-        /// @param importedFile a handle to a file that will contain the
-        /// imported data.
+        /// @param result is an optional status result.
         /// @param assetResolver is an optional helper to resolve assets which
         /// cannot be found in-band.
-        /// @returns whether the import was successful or an error occurred.
-        static ImportResult import(BinaryReader& reader,
-                                   File** importedFile,
+        /// @returns a pointer to the file, or null on failure.
+        static std::unique_ptr<File> import(BinaryReader& reader,
+                                   ImportResult* result  = nullptr,
                                    FileAssetResolver* assetResolver = nullptr);
 
         /// @returns the file's backboard. All files have exactly one backboard.

skia/viewer/src/main.cpp

@@ -47,18 +47,19 @@ void initStateMachine(int index) {
     animationIndex = -1;
     assert(fileBytes != nullptr);
     auto reader = rive::BinaryReader(fileBytes, fileBytesLength);
-    rive::File* file = nullptr;
-    auto result = rive::File::import(reader, &file);
-    if (result != rive::ImportResult::success) {
+    auto file = rive::File::import(reader);
+    if (!file) {
         delete[] fileBytes;
         fprintf(stderr, "failed to import file\n");
         return;
     }
-    artboard = file->artboard()->instance();
-    artboard->advance(0.0f);
-
     delete animationInstance;
     delete stateMachineInstance;
+    currentFile = std::move(file);
+
+    artboard = currentFile->artboard()->instance();
+    artboard->advance(0.0f);
+
     animationInstance = nullptr;
     stateMachineInstance = nullptr;
 
@@ -68,27 +69,26 @@ void initStateMachine(int index) {
     if (stateMachine != nullptr) {
         stateMachineInstance = new rive::StateMachineInstance(stateMachine);
     }
-
-    currentFile.reset(file);
 }
 
 void initAnimation(int index) {
     animationIndex = index;
     stateMachineIndex = -1;
     assert(fileBytes != nullptr);
     auto reader = rive::BinaryReader(fileBytes, fileBytesLength);
-    rive::File* file = nullptr;
-    auto result = rive::File::import(reader, &file);
-    if (result != rive::ImportResult::success) {
+    auto file = rive::File::import(reader);
+    if (!file) {
         delete[] fileBytes;
         fprintf(stderr, "failed to import file\n");
         return;
     }
-    artboard = file->artboard()->instance();
-    artboard->advance(0.0f);
-
     delete animationInstance;
     delete stateMachineInstance;
+    currentFile = std::move(file);
+
+    artboard = currentFile->artboard()->instance();
+    artboard->advance(0.0f);
+
     animationInstance = nullptr;
     stateMachineInstance = nullptr;
 
@@ -97,8 +97,6 @@ void initAnimation(int index) {
     if (animation != nullptr) {
         animationInstance = new rive::LinearAnimationInstance(animation);
     }
-
-    currentFile.reset(file);
 }
 
 void glfwErrorCallback(int error, const char* description) { puts(description); }

src/file.cpp

@@ -111,12 +111,15 @@ File::File(FileAssetResolver* assetResolver) : m_AssetResolver(assetResolver) {}
 File::~File() {}
 
 // Import a Rive file from a file handle
-ImportResult
-File::import(BinaryReader& reader, File** importedFile, FileAssetResolver* assetResolver) {
+std::unique_ptr<File>
+File::import(BinaryReader& reader, ImportResult* result, FileAssetResolver* assetResolver) {
     RuntimeHeader header;
     if (!RuntimeHeader::read(reader, header)) {
         fprintf(stderr, "Bad header\n");
-        return ImportResult::malformed;
+        if (result) {
+            *result = ImportResult::malformed;
+        }
+        return nullptr;
     }
     if (header.majorVersion() != majorVersion) {
         fprintf(stderr,
@@ -125,16 +128,20 @@ File::import(BinaryReader& reader, File** importedFile, FileAssetResolver* asset
                 header.minorVersion(),
                 majorVersion,
                 minorVersion);
-        return ImportResult::unsupportedVersion;
+        if (result) {
+            *result = ImportResult::unsupportedVersion;
+        }
+        return nullptr;
+    }
+    auto file = std::unique_ptr<File>(new File(assetResolver));
+    auto readResult = file->read(reader, header);
+    if (readResult != ImportResult::success) {
+        file.reset(nullptr);
     }
-    auto file = new File(assetResolver);
-    auto result = file->read(reader, header);
-    if (result != ImportResult::success) {
-        delete file;
-        return result;
+    if (result) {
+        *result = ImportResult::success;
     }
-    *importedFile = file;
-    return result;
+    return file;
 }
 
 ImportResult File::read(BinaryReader& reader, const RuntimeHeader& header) {

test/image_asset_test.cpp

@@ -55,10 +55,8 @@ TEST_CASE("out of band image assets loads correctly", "[assets]") {
     uint8_t* bytes = new uint8_t[length];
     REQUIRE(fread(bytes, 1, length, fp) == length);
     auto reader = rive::BinaryReader(bytes, length);
-    rive::File* file = nullptr;
-    auto result = rive::File::import(reader, &file, &resolver);
+    auto file = rive::File::import(reader, nullptr, &resolver);
 
-    REQUIRE(result == rive::ImportResult::success);
     REQUIRE(file != nullptr);
     REQUIRE(file->artboard() != nullptr);
 
@@ -88,6 +86,5 @@ TEST_CASE("out of band image assets loads correctly", "[assets]") {
     rive::NoOpRenderer renderer;
     file->artboard()->draw(&renderer);
 
-    delete file;
     delete[] bytes;
 }

test/instancing_test.cpp

@@ -18,10 +18,8 @@ TEST_CASE("cloning an ellipse works", "[instancing]") {
     uint8_t* bytes = new uint8_t[length];
     REQUIRE(fread(bytes, 1, length, fp) == length);
     auto reader = rive::BinaryReader(bytes, length);
-    rive::File* file = nullptr;
-    auto result = rive::File::import(reader, &file);
+    auto file = rive::File::import(reader);
 
-    REQUIRE(result == rive::ImportResult::success);
     REQUIRE(file != nullptr);
     REQUIRE(file->artboard() != nullptr);
 
@@ -33,8 +31,6 @@ TEST_CASE("cloning an ellipse works", "[instancing]") {
     REQUIRE(node->y() == clonedNode->y());
 
     delete clonedNode;
-
-    delete file;
     delete[] bytes;
 }
 
@@ -48,10 +44,8 @@ TEST_CASE("instancing artboard clones clipped properties", "[instancing]") {
     uint8_t* bytes = new uint8_t[length];
     REQUIRE(fread(bytes, 1, length, fp) == length);
     auto reader = rive::BinaryReader(bytes, length);
-    rive::File* file = nullptr;
-    auto result = rive::File::import(reader, &file);
+    auto file = rive::File::import(reader);
 
-    REQUIRE(result == rive::ImportResult::success);
     REQUIRE(file != nullptr);
     REQUIRE(file->artboard() != nullptr);
     REQUIRE(!file->artboard()->isInstance());
@@ -74,7 +68,6 @@ TEST_CASE("instancing artboard clones clipped properties", "[instancing]") {
     rive::NoOpRenderer renderer;
     artboard->draw(&renderer);
 
-    delete file;
     delete[] bytes;
 }
 
@@ -88,10 +81,8 @@ TEST_CASE("instancing artboard doesn't clone animations", "[instancing]") {
     uint8_t* bytes = new uint8_t[length];
     REQUIRE(fread(bytes, 1, length, fp) == length);
     auto reader = rive::BinaryReader(bytes, length);
-    rive::File* file = nullptr;
-    auto result = rive::File::import(reader, &file);
+    auto file = rive::File::import(reader);
 
-    REQUIRE(result == rive::ImportResult::success);
     REQUIRE(file != nullptr);
     REQUIRE(file->artboard() != nullptr);
 
@@ -105,7 +96,7 @@ TEST_CASE("instancing artboard doesn't clone animations", "[instancing]") {
     REQUIRE(rive::LinearAnimation::deleteCount == 0);
 
     int numberOfAnimations = file->artboard()->animationCount();
-    delete file;
+    file.reset(nullptr);
     // Now the animations should've been deleted.
     REQUIRE(rive::LinearAnimation::deleteCount == numberOfAnimations);
 

test/rive_file_reader.hpp

@@ -6,7 +6,7 @@
 #include "rive_testing.hpp"
 
 class RiveFileReader {
-    rive::File* m_File = nullptr;
+    std::unique_ptr<rive::File> m_File;
     uint8_t* m_Bytes = nullptr;
     rive::BinaryReader* m_Reader;
 
@@ -21,19 +21,19 @@ class RiveFileReader {
         m_Bytes = new uint8_t[length];
         REQUIRE(fread(m_Bytes, 1, length, fp) == length);
         m_Reader = new rive::BinaryReader(m_Bytes, length);
-        auto result = rive::File::import(*m_Reader, &m_File);
+        rive::ImportResult result;
+        m_File = rive::File::import(*m_Reader, &result);
 
         REQUIRE(result == rive::ImportResult::success);
         REQUIRE(m_File != nullptr);
         REQUIRE(m_File->artboard() != nullptr);
     }
     ~RiveFileReader() {
-        delete m_File;
         delete m_Reader;
         delete[] m_Bytes;
     }
 
-    rive::File* file() const { return m_File; }
+    rive::File* file() const { return m_File.get(); }
 };
 
 #endif