From 40ce71b759b1ea7aafff230a7622a8f085c6ef52 Mon Sep 17 00:00:00 2001 From: Kevin Kreiser Date: Wed, 13 Mar 2024 15:38:27 -0400 Subject: [PATCH] use a singleton pattern to register the gdal driver once in one place (#4628) --- CHANGELOG.md | 1 + CMakeLists.txt | 17 +++++++++++------ src/tyr/CMakeLists.txt | 6 ++---- src/tyr/isochrone_serializer.cc | 19 ++++++++++++++++++- src/valhalla_run_isochrone.cc | 9 --------- src/valhalla_service.cc | 8 -------- test/isochrone.cc | 3 --- 7 files changed, 32 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 49dc12b027..adbe9dcc43 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,7 @@ * FIXED: Use helper function for only parsing out names from DirectedEdge when populating intersecting edges [#4604](https://github.com/valhalla/valhalla/pull/4604) * FIXED: Osmnode size reduction: Fixed excessive disk space for planet build [#4605](https://github.com/valhalla/valhalla/pull/4605) * FIXED: CostMatrix for trivial routes with oneways [#4626](https://github.com/valhalla/valhalla/pull/4626) + * FIXED: some entry points to creating geotiff isochrones output did not register the geotiff driver before attempting to use it [#4628](https://github.com/valhalla/valhalla/pull/4628) * **Enhancement** * UPDATED: French translations, thanks to @xlqian [#4159](https://github.com/valhalla/valhalla/pull/4159) * CHANGED: -j flag for multithreaded executables to override mjolnir.concurrency [#4168](https://github.com/valhalla/valhalla/pull/4168) diff --git a/CMakeLists.txt b/CMakeLists.txt index b1f238201a..209e06d3d0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -36,7 +36,7 @@ option(ENABLE_SINGLE_FILES_WERROR "Convert compiler warnings to errors for singl option(PREFER_EXTERNAL_DEPS "Whether to use internally vendored headers or find the equivalent external package" OFF) # useful to workaround issues likes this https://stackoverflow.com/questions/24078873/cmake-generated-xcode-project-wont-compile option(ENABLE_STATIC_LIBRARY_MODULES "If ON builds Valhalla modules as STATIC library targets" OFF) -option(ENABLE_GDAL "Whether to include GDAL; currently only used for raster serialization of isotile grid" OFF) +option(ENABLE_GDAL "Whether to include GDAL; currently only used for raster serialization of isotile grid" ON) set(LOGGING_LEVEL "" CACHE STRING "Logging level, default is INFO") set_property(CACHE LOGGING_LEVEL PROPERTY STRINGS "NONE;ALL;ERROR;WARN;INFO;DEBUG;TRACE") @@ -191,12 +191,17 @@ else() message(FATAL_ERROR "Required target protobuf::libprotobuf-lite or protobuf::libprotobuf is not defined") endif() -# gdal -set(gdal_target "") +# unless you said you didnt want gdal we try to turn it on, if we cant we tell you +set(GDAL_TARGET "") if (ENABLE_GDAL) - find_package(GDAL REQUIRED) - add_compile_definitions(ENABLE_GDAL) - set(gdal_target GDAL::GDAL) + find_package(GDAL QUIET) + if (GDAL_FOUND) + set(GDAL_TARGET GDAL::GDAL) + add_compile_definitions(ENABLE_GDAL) + message(STATUS "Gdal support is enabled") + else() + message(WARNING "Unable to enable gdal support") + endif() endif() set(CMAKE_FIND_PACKAGE_PREFER_CONFIG OFF) diff --git a/src/tyr/CMakeLists.txt b/src/tyr/CMakeLists.txt index 9c0b22dee2..571d2b0de1 100644 --- a/src/tyr/CMakeLists.txt +++ b/src/tyr/CMakeLists.txt @@ -20,8 +20,6 @@ set(system_includes ${date_include_dir} $<$:${dirent_include_dir}>) - - valhalla_module(NAME tyr SOURCES ${sources} @@ -46,6 +44,6 @@ valhalla_module(NAME tyr valhalla::odin valhalla::proto ${valhalla_protobuf_targets} - Boost::boost - ${gdal_target} + Boost::boost + ${GDAL_TARGET} ) diff --git a/src/tyr/isochrone_serializer.cc b/src/tyr/isochrone_serializer.cc index a1b504158e..22943370e4 100644 --- a/src/tyr/isochrone_serializer.cc +++ b/src/tyr/isochrone_serializer.cc @@ -16,6 +16,23 @@ using namespace valhalla::baldr::json; namespace { + +// allows us to only ever register the driver once per process without having to put it +// in every executable that might call into this code +struct gdal_singleton_t { + static const gdal_singleton_t& get() { + static const gdal_singleton_t instance; + return instance; + } + +private: + gdal_singleton_t() { +#ifdef ENABLE_GDAL + GDALRegister_GTiff(); +#endif + } +}; + using rgba_t = std::tuple; using namespace valhalla; @@ -169,7 +186,7 @@ std::string serializeGeoTIFF(Api& request, const std::shared_ptrGetDriverByName("GTiff"); auto geotiff_dataset = diff --git a/src/valhalla_run_isochrone.cc b/src/valhalla_run_isochrone.cc index a96394caf0..ac61bedff9 100644 --- a/src/valhalla_run_isochrone.cc +++ b/src/valhalla_run_isochrone.cc @@ -20,10 +20,6 @@ #include "tyr/serializers.h" #include "worker.h" -#ifdef ENABLE_GDAL -#include -#endif - using namespace valhalla; using namespace valhalla::midgard; using namespace valhalla::baldr; @@ -84,11 +80,6 @@ int main(int argc, char* argv[]) { Api request; ParseApi(json_str, valhalla::Options::isochrone, request); -#ifdef ENABLE_GDAL - if (request.options().format() == Options_Format_geotiff) { - GDALRegister_GTiff(); - } -#endif auto& options = *request.mutable_options(); // Get the denoise parameter diff --git a/src/valhalla_service.cc b/src/valhalla_service.cc index 4defc13195..95c753259d 100644 --- a/src/valhalla_service.cc +++ b/src/valhalla_service.cc @@ -17,10 +17,6 @@ using namespace prime_server; #endif -#ifdef ENABLE_GDAL -#include -#endif - #include "config.h" #include "midgard/logging.h" @@ -43,10 +39,6 @@ int main(int argc, char** argv) { } #endif -#ifdef ENABLE_GDAL - GDALRegister_GTiff(); -#endif - // config file // TODO: validate the config std::string config_file(argv[1]); diff --git a/test/isochrone.cc b/test/isochrone.cc index 1d890b15b9..667402fb6b 100644 --- a/test/isochrone.cc +++ b/test/isochrone.cc @@ -355,7 +355,6 @@ void check_raster_edges(size_t x, size_t y, uint16_t* data) { } TEST(Isochrones, test_geotiff_output_distance) { - GDALRegister_GTiff(); loki_worker_t loki_worker(cfg); thor_worker_t thor_worker(cfg); @@ -401,7 +400,6 @@ TEST(Isochrones, test_geotiff_output_distance) { } TEST(Isochrones, test_geotiff_output_time) { - GDALRegister_GTiff(); loki_worker_t loki_worker(cfg); thor_worker_t thor_worker(cfg); @@ -448,7 +446,6 @@ TEST(Isochrones, test_geotiff_output_time) { // test request with two metrics TEST(Isochrones, test_geotiff_output_time_distance) { - GDALRegister_GTiff(); loki_worker_t loki_worker(cfg); thor_worker_t thor_worker(cfg);