fix(ImageIO): image input robustness and faster image IO

include/geode/io/image/detail/gdal_file.hpp

@@ -23,6 +23,8 @@
 
 #pragma once
 
+#include <absl/types/span.h>
+
 #include <geode/basic/pimpl.hpp>
 
 #include <geode/io/image/common.hpp>
@@ -35,28 +37,51 @@
     ALIAS_2D( CoordinateSystem );
 } // namespace geode
 
 namespace geode
 {
     namespace detail
     {
+        /*!
+         * While alive, GDAL warnings are logged and GDAL errors are not
+         * printed: they should be reported with last_error()
+         */
+        class opengeode_io_image_api GDALErrorScope
+        {
+        public:
+            GDALErrorScope();
+            ~GDALErrorScope();
+            GDALErrorScope( const GDALErrorScope& ) = delete;
+            GDALErrorScope& operator=( const GDALErrorScope& ) = delete;
+            GDALErrorScope( GDALErrorScope&& ) = delete;
+            GDALErrorScope& operator=( GDALErrorScope&& ) = delete;
+
+            std::string last_error() const;
+        };
+
         class opengeode_io_image_api GDALFile
         {
         public:
-            GDALFile( std::string_view filename );
+            /*!
+             * @param[in] allowed_drivers Names of the GDAL drivers which can
+             * open the file, all the drivers if empty
+             */
+            explicit GDALFile( std::string_view filename,
+                absl::Span< const char* const > allowed_drivers = {} );
             ~GDALFile();
 
             CoordinateSystem2D read_coordinate_system();
 
             bool is_coordinate_system_loadable();
 
+            // Existing files used with the main file (e.g. world files)
             template < typename AdditionalFiles >
             AdditionalFiles additional_files()
             {
                 AdditionalFiles files;
                 for( auto& file : associated_files() )
                 {
                     files.optional_files.emplace_back(
-                        std::move( file ), true );
+                        std::move( file ), false );
                 }
                 return files;
             }

include/geode/io/image/detail/vtk_output.hpp

@@ -29,6 +29,7 @@
 #include <charconv>
 #include <cmath>
 #include <cstring>
+#include <filesystem>
 #include <fstream>
 #include <map>
 #include <memory>
@@ -48,7 +49,7 @@
 #include <geode/geometry/point.hpp>
 #include <geode/geometry/vector.hpp>
 
 namespace geode
 {
     namespace detail
     {
@@ -56,9 +57,9 @@
         // independently of the locale
         inline std::string vtk_double_string( double value )
         {
             std::array< char, 32 > buffer;
             const auto result = std::to_chars(
                 buffer.data(), buffer.data() + buffer.size(), value );
             return { buffer.data(), result.ptr };
         }
 
@@ -74,13 +75,13 @@
             }
             else if constexpr( std::is_signed_v< T > )
             {
                 constexpr std::array< const char*, 9 > names{ "", "Int8",
                     "Int16", "", "Int32", "", "", "", "Int64" };
                 return names[sizeof( T )];
             }
             else
             {
                 constexpr std::array< const char*, 9 > names{ "", "UInt8",
                     "UInt16", "", "UInt32", "", "", "", "UInt64" };
                 return names[sizeof( T )];
             }
@@ -96,7 +97,7 @@
         {
             using Stored = std::
                 conditional_t< std::is_same_v< Value, bool >, uint8_t, Value >;
             static constexpr local_index_t nb_components = 1;
             static Stored component(
                 const Value& value, local_index_t /*unused*/ )
             {
@@ -193,17 +194,37 @@
                 {
                     write_with_appended_data( root );
                 }
-                file_.flush();
-                OpenGeodeIOImageException::check_exception( file_.good(),
+                file_.close();
+                OpenGeodeIOImageException::check_exception( !file_.fail(),
                     nullptr, OpenGeodeException::TYPE::data,
                     "[VTKOutput] Error while writing file: ", filename_ );
+                written_ = true;
+                if( temporary_filename_.empty() )
+                {
+                    return;
+                }
+                std::error_code error;
+                std::filesystem::rename(
+                    temporary_filename_, target_filename_, error );
+                if( error )
+                {
+                    std::filesystem::remove( temporary_filename_, error );
+                    throw OpenGeodeIOImageException{ nullptr,
+                        OpenGeodeException::TYPE::data,
+                        "[VTKOutput] Error while writing file: ", filename_,
+                        " (", error.message(), ")" };
+                }
             }
 
         protected:
             VTKOutputImpl(
                 std::string_view filename, const Mesh& mesh, const char* type )
                 : filename_{ filename },
-                  file_{ to_string( filename ), std::ios::binary },
+                  target_filename_{ target_file( filename ) },
+                  temporary_filename_{ temporary_file( target_filename_ ) },
+                  file_{ temporary_filename_.empty() ? target_filename_
+                                                     : temporary_filename_,
+                      std::ios::binary },
                   mesh_( mesh ),
                   type_{ type }
             {
@@ -212,7 +233,49 @@
                     "[VTKOutput] Error while writing file: ", filename );
             }
 
-            virtual ~VTKOutputImpl() {}
+            // The file is written in a temporary file which replaces the
+            // output file once complete: a failure never leaves a truncated
+            // file nor erases an existing one
+            virtual ~VTKOutputImpl()
+            {
+                if( written_ || temporary_filename_.empty() )
+                {
+                    return;
+                }
+                file_.close();
+                std::error_code error;
+                std::filesystem::remove( temporary_filename_, error );
+            }
+
+            // Symbolic links are followed to replace their target
+            static std::string target_file( std::string_view filename )
+            {
+                const std::filesystem::path path{ to_string( filename ) };
+                std::error_code error;
+                if( std::filesystem::is_symlink( path, error ) )
+                {
+                    const auto target =
+                        std::filesystem::canonical( path, error );
+                    if( !error )
+                    {
+                        return target.string();
+                    }
+                }
+                return path.string();
+            }
+
+            // Special files (e.g. devices) cannot be replaced: they are written
+            // directly, without temporary file
+            static std::string temporary_file( const std::string& target )
+            {
+                std::error_code error;
+                const auto status = std::filesystem::status( target, error );
+                if( std::filesystem::is_other( status ) )
+                {
+                    return {};
+                }
+                return absl::StrCat( target, ".tmp" );
+            }
 
             const Mesh& mesh() const
             {
@@ -298,6 +361,22 @@
                 std::string_view name,
                 std::vector< T >&& values,
                 local_index_t nb_components = 1 )
+            {
+                auto owner =
+                    std::make_shared< std::vector< T > >( std::move( values ) );
+                auto data_array = write_external_data_array< T >(
+                    parent, name, *owner, nb_components );
+                appended_blocks_.back().owner = std::move( owner );
+                return data_array;
+            }
+
+            // Same as above, the values are neither copied nor owned: they
+            // must stay alive until the file is written
+            template < typename T >
+            pugi::xml_node write_external_data_array( pugi::xml_node& parent,
+                std::string_view name,
+                absl::Span< const T > values,
+                local_index_t nb_components = 1 )
             {
                 auto data_array = parent.append_child( "DataArray" );
                 data_array.append_attribute( "type" ).set_value(
@@ -307,16 +386,13 @@
                 data_array.append_attribute( "format" ).set_value( "appended" );
                 data_array.append_attribute( "NumberOfComponents" )
                     .set_value( nb_components );
-                write_range( data_array, absl::Span< const T >{ values } );
+                write_range( data_array, values );
                 data_array.append_attribute( "offset" )
                     .set_value( appended_size_ );
-                auto owner =
-                    std::make_shared< std::vector< T > >( std::move( values ) );
                 AppendedBlock block;
-                block.nb_bytes = owner->size() * sizeof( T );
+                block.nb_bytes = values.size() * sizeof( T );
                 // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
-                block.data = reinterpret_cast< const char* >( owner->data() );
-                block.owner = std::move( owner );
+                block.data = reinterpret_cast< const char* >( values.data() );
                 appended_size_ += sizeof( HeaderType ) + block.nb_bytes;
                 appended_blocks_.push_back( std::move( block ) );
                 return data_array;
@@ -583,7 +659,10 @@
 
         private:
             std::string_view filename_;
+            std::string target_filename_;
+            std::string temporary_filename_;
             std::ofstream file_;
+            bool written_{ false };
             const Mesh& mesh_;
             pugi::xml_document document_;
             const char* type_;

include/geode/io/image/internal/raster_image_input.hpp

@@ -44,9 +44,8 @@ namespace geode
 
             RasterImage2D read_file();
 
-            RasterImage2D read_reversed_y_axis_file();
-
-            Percentage is_loadable();
+            // Unreadable files are not loadable
+            static Percentage is_loadable( std::string_view filename );
         };
     } // namespace internal
 } // namespace geode

src/geode/io/image/bmp_input.cpp

@@ -45,8 +45,7 @@ namespace geode
 
         Percentage BMPInput::is_loadable() const
         {
-            ImageInputImpl reader{ filename() };
-            return reader.is_loadable();
+            return ImageInputImpl::is_loadable( filename() );
         }
     } // namespace internal
 } // namespace geode

src/geode/io/image/gdal_file.cpp

@@ -23,26 +23,59 @@
 
 #include <geode/io/image/detail/gdal_file.hpp>
 
+#include <filesystem>
+
 #include <gdal_priv.h>
 
+#include <geode/basic/logger.hpp>
 #include <geode/basic/pimpl_impl.hpp>
 
 #include <geode/geometry/coordinate_system.hpp>
 
+namespace
+{
+    void CPL_STDCALL log_gdal_warnings(
+        CPLErr type, CPLErrorNum /*unused*/, const char* message )
+    {
+        if( type == CE_Warning )
+        {
+            geode::Logger::warning( "[GDAL] ", message );
+        }
+    }
+} // namespace
+
 namespace geode
 {
     namespace detail
     {
+        GDALErrorScope::GDALErrorScope()
+        {
+            CPLPushErrorHandler( log_gdal_warnings );
+            CPLErrorReset();
+        }
+
+        GDALErrorScope::~GDALErrorScope()
+        {
+            CPLPopErrorHandler();
+        }
+
+        std::string GDALErrorScope::last_error() const
+        {
+            return CPLGetLastErrorMsg();
+        }
+
         class GDALFile::Impl
         {
         public:
-            Impl( std::string_view filename )
-                : dataset_{ GDALDataset::Open(
-                      geode::to_string( filename ).c_str(), GDAL_OF_READONLY ) }
+            Impl( std::string_view filename,
+                absl::Span< const char* const > allowed_drivers )
             {
+                const GDALErrorScope errors;
+                dataset_.reset( open( filename, allowed_drivers ) );
                 OpenGeodeIOImageException::check_exception( dataset_ != nullptr,
                     nullptr, OpenGeodeException::TYPE::data,
-                    "[GDALFile] Failed to open file ", filename );
+                    "[GDALFile] Failed to open file ", filename, ": ",
+                    errors.last_error() );
             }
 
             GDALDataset& dataset()
@@ -85,20 +118,52 @@ namespace geode
                 {
                     return {};
                 }
+                // The list starts with the main file, which is not associated
+                const std::filesystem::path main_file{
+                    dataset_->GetDescription()
+                };
                 std::vector< std::string > files;
                 for( int i = 0; papszFileList[i] != nullptr; i++ )
                 {
+                    std::error_code error;
+                    if( papszFileList[i] == main_file
+                        || std::filesystem::equivalent(
+                            papszFileList[i], main_file, error ) )
+                    {
+                        continue;
+                    }
                     files.emplace_back( papszFileList[i] );
                 }
                 CSLDestroy( papszFileList );
                 return files;
             }
 
+        private:
+            static GDALDataset* open( std::string_view filename,
+                absl::Span< const char* const > allowed_drivers )
+            {
+                if( allowed_drivers.empty() )
+                {
+                    return GDALDataset::Open(
+                        geode::to_string( filename ).c_str(),
+                        GDAL_OF_READONLY | GDAL_OF_VERBOSE_ERROR );
+                }
+                std::vector< const char* > drivers{ allowed_drivers.begin(),
+                    allowed_drivers.end() };
+                drivers.push_back( nullptr );
+                return GDALDataset::Open( geode::to_string( filename ).c_str(),
+                    GDAL_OF_READONLY | GDAL_OF_VERBOSE_ERROR, drivers.data() );
+            }
+
         private:
             GDALDatasetUniquePtr dataset_;
         };
 
-        GDALFile::GDALFile( std::string_view filename ) : impl_{ filename } {}
+        GDALFile::GDALFile( std::string_view filename,
+            absl::Span< const char* const > allowed_drivers )
+            : impl_{ filename, allowed_drivers }
+        {
+        }
 
         GDALFile::~GDALFile() = default;
 

src/geode/io/image/jpg_input.cpp

@@ -34,7 +34,7 @@ namespace geode
         RasterImage2D JPGInput::read()
         {
             ImageInputImpl reader{ filename() };
-            return reader.read_reversed_y_axis_file();
+            return reader.read_file();
         }
 
         auto JPGInput::additional_files() const -> AdditionalFiles
@@ -45,8 +45,7 @@ namespace geode
 
         Percentage JPGInput::is_loadable() const
         {
-            ImageInputImpl reader{ filename() };
-            return reader.is_loadable();
+            return ImageInputImpl::is_loadable( filename() );
         }
     } // namespace internal
 } // namespace geode

src/geode/io/image/png_input.cpp

@@ -34,7 +34,7 @@ namespace geode
         RasterImage2D PNGInput::read()
         {
             ImageInputImpl reader{ filename() };
-            return reader.read_reversed_y_axis_file();
+            return reader.read_file();
         }
 
         auto PNGInput::additional_files() const -> AdditionalFiles
@@ -45,8 +45,7 @@ namespace geode
 
         Percentage PNGInput::is_loadable() const
         {
-            ImageInputImpl reader{ filename() };
-            return reader.is_loadable();
+            return ImageInputImpl::is_loadable( filename() );
         }
     } // namespace internal
 } // namespace geode