From 4c2c0f0b70d817fd46300216b5dc5f19a25239aa Mon Sep 17 00:00:00 2001 From: Christian Beiwinkel Date: Tue, 20 Feb 2024 12:28:04 +0100 Subject: [PATCH 01/27] add gdal to project --- CMakeLists.txt | 10 ++++++++++ cmake/ValhallaPkgConfig.cmake | 3 +++ src/thor/CMakeLists.txt | 3 ++- src/tyr/CMakeLists.txt | 6 +++++- 4 files changed, 20 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 63ccd718c0..58af53bfe9 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -36,6 +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) set(LOGGING_LEVEL "" CACHE STRING "Logging level, default is INFO") set_property(CACHE LOGGING_LEVEL PROPERTY STRINGS "NONE;ALL;ERROR;WARN;INFO;DEBUG;TRACE") @@ -203,6 +204,15 @@ if(ENABLE_SERVICES) endif() endif() +# gdal +set(gdal_target "") +if (ENABLE_GDAL) + find_package(GDAL REQUIRED) + add_compile_definitions(ENABLE_GDAL) + set(gdal_target GDAL::GDAL) +endif() + + ## Mjolnir and associated executables if(ENABLE_DATA_TOOLS) add_compile_definitions(DATA_TOOLS) diff --git a/cmake/ValhallaPkgConfig.cmake b/cmake/ValhallaPkgConfig.cmake index 3649a24dd3..7a7bfd1a20 100644 --- a/cmake/ValhallaPkgConfig.cmake +++ b/cmake/ValhallaPkgConfig.cmake @@ -33,6 +33,9 @@ function(configure_valhalla_pc) if(ENABLE_SERVICES) list(APPEND REQUIRES libprime_server) endif() + if(ENABLE_GDAL) + list(APPEND REQUIRES GDAL::GDAL) + endif() if(WIN32 AND NOT MINGW) list(APPEND LIBS_PRIVATE -lole32 -lshell32) else() diff --git a/src/thor/CMakeLists.txt b/src/thor/CMakeLists.txt index adcd0af750..2d0bb1ec97 100644 --- a/src/thor/CMakeLists.txt +++ b/src/thor/CMakeLists.txt @@ -56,4 +56,5 @@ valhalla_module(NAME thor valhalla::meili ${valhalla_protobuf_targets} Boost::boost - ${libprime_server_targets}) + ${libprime_server_targets} + ${gdal_target}) diff --git a/src/tyr/CMakeLists.txt b/src/tyr/CMakeLists.txt index 197e14b53b..9c0b22dee2 100644 --- a/src/tyr/CMakeLists.txt +++ b/src/tyr/CMakeLists.txt @@ -20,6 +20,8 @@ set(system_includes ${date_include_dir} $<$:${dirent_include_dir}>) + + valhalla_module(NAME tyr SOURCES ${sources} @@ -44,4 +46,6 @@ valhalla_module(NAME tyr valhalla::odin valhalla::proto ${valhalla_protobuf_targets} - Boost::boost) + Boost::boost + ${gdal_target} + ) From d9e9ba72307415af2c944d25c49f1d30d6e206a7 Mon Sep 17 00:00:00 2001 From: Christian Beiwinkel Date: Tue, 20 Feb 2024 12:28:24 +0100 Subject: [PATCH 02/27] geotiff serialization --- proto/options.proto | 1 + src/loki/worker.cc | 6 ++ src/proto_conversions.cc | 12 ++-- src/thor/isochrone_action.cc | 15 ++-- src/thor/worker.cc | 12 ++++ src/tyr/isochrone_serializer.cc | 122 ++++++++++++++++++++++++++++++-- src/valhalla_run_isochrone.cc | 30 +++++--- src/valhalla_service.cc | 8 +++ src/worker.cc | 2 + test/isochrone.cc | 49 +++++++++++++ valhalla/midgard/gridded_data.h | 36 +++++++++- valhalla/thor/worker.h | 11 +++ valhalla/tyr/serializers.h | 16 +++-- 13 files changed, 283 insertions(+), 37 deletions(-) diff --git a/proto/options.proto b/proto/options.proto index f6c4ca3194..122db3b360 100644 --- a/proto/options.proto +++ b/proto/options.proto @@ -340,6 +340,7 @@ message Options { gpx = 1; osrm = 2; pbf = 3; + geotiff = 4; } enum Action { diff --git a/src/loki/worker.cc b/src/loki/worker.cc index 0a927f21e8..eac29e5331 100644 --- a/src/loki/worker.cc +++ b/src/loki/worker.cc @@ -357,6 +357,12 @@ loki_worker_t::work(const std::list& job, result.messages.emplace_back(request.SerializeAsString()); break; case Options::isochrone: +// return early if geotiff was requested but GDAL isn't enabled +#ifndef ENABLE_GDAL + if (options.format() == Options_Format_geotiff) { + throw valhalla_exception_t{504}; + } +#endif isochrones(request); result.messages.emplace_back(request.SerializeAsString()); break; diff --git a/src/proto_conversions.cc b/src/proto_conversions.cc index bcab5517f3..f4172b5b0c 100644 --- a/src/proto_conversions.cc +++ b/src/proto_conversions.cc @@ -268,10 +268,8 @@ const std::string& ShapeMatch_Enum_Name(const ShapeMatch match) { bool Options_Format_Enum_Parse(const std::string& format, Options::Format* f) { static const std::unordered_map formats{ - {"json", Options::json}, - {"gpx", Options::gpx}, - {"osrm", Options::osrm}, - {"pbf", Options::pbf}, + {"json", Options::json}, {"gpx", Options::gpx}, {"osrm", Options::osrm}, + {"pbf", Options::pbf}, {"geotiff", Options::geotiff}, }; auto i = formats.find(format); if (i == formats.cend()) @@ -282,10 +280,8 @@ bool Options_Format_Enum_Parse(const std::string& format, Options::Format* f) { const std::string& Options_Format_Enum_Name(const Options::Format match) { static const std::unordered_map formats{ - {Options::json, "json"}, - {Options::gpx, "gpx"}, - {Options::osrm, "osrm"}, - {Options::pbf, "pbf"}, + {Options::json, "json"}, {Options::gpx, "gpx"}, {Options::osrm, "osrm"}, + {Options::pbf, "pbf"}, {Options::geotiff, "geotiff"}, }; auto i = formats.find(match); return i == formats.cend() ? empty_str : i->second; diff --git a/src/thor/isochrone_action.cc b/src/thor/isochrone_action.cc index 9bb3630d64..f32cde3bce 100644 --- a/src/thor/isochrone_action.cc +++ b/src/thor/isochrone_action.cc @@ -43,15 +43,12 @@ std::string thor_worker_t::isochrones(Api& request) { if (options.action() == Options_Action_expansion) return ""; - // we have parallel vectors of contour properties and the actual geojson features - // this method sorts the contour specifications by metric (time or distance) and then by value - // with the largest values coming first. eg (60min, 30min, 10min, 40km, 10km) - auto contours = - grid->GenerateContours(intervals, options.polygons(), options.denoise(), options.generalize()); - - // make the final output (pbf or json) - std::string ret = tyr::serializeIsochrones(request, intervals, contours, options.polygons(), - options.show_locations()); +// make the final output (pbf or json) +#ifdef ENABLE_GDAL + std::string ret = tyr::serializeIsochrones(request, intervals, grid, geotiff_driver); +#else + std::string ret = tyr::serializeIsochrones(request, intervals, grid); +#endif return ret; } diff --git a/src/thor/worker.cc b/src/thor/worker.cc index afe1e8373e..ab91d1652d 100644 --- a/src/thor/worker.cc +++ b/src/thor/worker.cc @@ -15,6 +15,10 @@ #include +#ifdef ENABLE_GDAL +#include +#endif + using namespace valhalla; using namespace valhalla::tyr; using namespace valhalla::midgard; @@ -106,11 +110,19 @@ thor_worker_t::thor_worker_t(const boost::property_tree::ptree& config, max_timedep_distance = config.get("service_limits.max_timedep_distance", kDefaultMaxTimeDependentDistance); +#ifdef ENABLE_GDAL + auto driver_manager = GetGDALDriverManager(); + geotiff_driver = driver_manager->GetDriverByName("GTiff"); +#endif + // signal that the worker started successfully started(); } thor_worker_t::~thor_worker_t() { +#ifdef ENABLE_GDAL + GDALDestroyDriverManager(); +#endif } #ifdef ENABLE_SERVICES diff --git a/src/tyr/isochrone_serializer.cc b/src/tyr/isochrone_serializer.cc index 6f549e5e05..abc0cb98d5 100644 --- a/src/tyr/isochrone_serializer.cc +++ b/src/tyr/isochrone_serializer.cc @@ -2,12 +2,17 @@ #include "baldr/json.h" #include "midgard/point2.h" #include "midgard/pointll.h" +#include "thor/worker.h" #include "tyr/serializers.h" #include #include #include +#ifdef ENABLE_GDAL +#include +#endif + using namespace valhalla::baldr::json; namespace { @@ -21,6 +26,7 @@ using feature_t = std::list; // rings per interval using contours_t = std::vector>; // all rings using contour_group_t = std::vector; using grouped_contours_t = std::vector; +// dimension, value (seconds/meters), name (time/distance), color using contour_interval_t = std::tuple; grouped_contours_t GroupContours(const bool polygons, const feature_t& contours) { @@ -134,6 +140,91 @@ void addLocations(Api& request, valhalla::baldr::json::ArrayPtr& features) { } } +#ifdef ENABLE_GDAL +// get a temporary file name suffix for GDAL's virtual file system +std::string GenerateTmpFName() { + std::stringstream ss; + ss << "/vsimem/" << std::this_thread::get_id() << "_" + << std::chrono::high_resolution_clock::now().time_since_epoch().count(); + return ss.str(); +} + +std::string serializeGeoTIFF(Api& request, + std::vector intervals, + std::shared_ptr> isogrid, + valhalla::thor::geotiff_driver_t geotiff_driver) { + // time, distance + std::vector metrics{false, false}; + for (auto& contour : request.options().contours()) { + metrics[0] = metrics[0] || contour.has_time_case(); + metrics[1] = metrics[1] || contour.has_distance_case(); + } + + auto box = isogrid->MinExtent(); + int32_t ext_x = box[2] - box[0]; + int32_t ext_y = box[3] - box[1]; + + // for GDALs virtual fs + std::string name = GenerateTmpFName(); + + auto nbands = std::count(metrics.begin(), metrics.end(), true); + char** geotiff_options = NULL; + geotiff_options = CSLSetNameValue(geotiff_options, "COMPRESS", "PACKBITS"); + + auto geotiff_dataset = + geotiff_driver->Create(name.c_str(), ext_x, ext_y, nbands, GDT_UInt16, geotiff_options); + + OGRSpatialReference spatial_ref; + spatial_ref.SetWellKnownGeogCS("EPSG:4326"); + double geo_transform[6] = {isogrid->TileBounds(isogrid->TileId(box[0], box[1])).minx(), // minx + isogrid->TileSize(), + 0, + isogrid->TileBounds(isogrid->TileId(box[0], box[1])).miny(), // miny + 0, + isogrid->TileSize()}; + + geotiff_dataset->SetGeoTransform(geo_transform); + geotiff_dataset->SetSpatialRef(const_cast(&spatial_ref)); + + for (size_t metric_idx = 0; metric_idx < metrics.size(); ++metric_idx) { + if (!metrics[metric_idx]) + continue; // only create bands for requested metrics + uint16_t dataArray[ext_x * ext_y]; + + // seconds or 10 meter steps + float scale_factor = metric_idx == 0 ? 3600 : 100; + for (int32_t i = 0; i < ext_y; ++i) { + for (int32_t j = 0; j < ext_x; ++j) { + auto tileid = isogrid->TileId(j + box[0], i + box[1]); + dataArray[i * ext_x + j] = + static_cast(isogrid->getData(tileid, metric_idx) * scale_factor); + } + } + auto band = geotiff_dataset->GetRasterBand(nbands == 2 ? (metric_idx + 1) : 1); + band->SetDescription(metric_idx == 0 ? "Time (seconds)" : "Distance (10m)"); + + CPLErr err = + band->RasterIO(GF_Write, 0, 0, ext_x, ext_y, dataArray, ext_x, ext_y, GDT_UInt16, 0, 0); + + // free some memory + delete[] geotiff_options; + + if (err != CE_None) { + throw valhalla_exception_t{599, "Unknown error when writing GeoTIFF."}; + } + } + + GDALClose(geotiff_dataset); + vsi_l_offset bufferlength; + GByte* bytes = VSIGetMemFileBuffer(name.c_str(), &bufferlength, TRUE); + + // TODO: there must be a way to do this without copying + std::string data(reinterpret_cast(bytes), bufferlength); + + return data; +}; +#endif + std::string serializeIsochroneJson(Api& request, std::vector& intervals, contours_t& contours, @@ -267,15 +358,36 @@ namespace tyr { std::string serializeIsochrones(Api& request, std::vector::contour_interval_t>& intervals, - midgard::GriddedData<2>::contours_t& contours, - bool polygons, - bool show_locations) { + std::shared_ptr> isogrid +#ifdef ENABLE_GDAL + , + valhalla::thor::geotiff_driver_t geotiff_driver) { +#else +) { +#endif + + // only generate if json or pbf output is requested + contours_t contours; switch (request.options().format()) { case Options_Format_pbf: - return serializeIsochronePbf(request, intervals, contours); case Options_Format_json: - return serializeIsochroneJson(request, intervals, contours, show_locations, polygons); + // we have parallel vectors of contour properties and the actual geojson features + // this method sorts the contour specifications by metric (time or distance) and then by value + // with the largest values coming first. eg (60min, 30min, 10min, 40km, 10km) + contours = + isogrid->GenerateContours(intervals, request.options().polygons(), + request.options().denoise(), request.options().generalize()); + return request.options().format() == Options_Format_json + ? serializeIsochroneJson(request, intervals, contours, + request.options().show_locations(), + request.options().polygons()) + : serializeIsochronePbf(request, intervals, contours); + +#ifdef ENABLE_GDAL + case Options_Format_geotiff: + return serializeGeoTIFF(request, intervals, isogrid, geotiff_driver); +#endif default: throw; } diff --git a/src/valhalla_run_isochrone.cc b/src/valhalla_run_isochrone.cc index 44eda59701..04b565cb47 100644 --- a/src/valhalla_run_isochrone.cc +++ b/src/valhalla_run_isochrone.cc @@ -2,6 +2,7 @@ #include #include #include +#include #include #include #include @@ -51,7 +52,7 @@ int main(int argc, char* argv[]) { "\"auto\",\"contours\":[{\"time\":15,\"color\":\"ff0000\"}]}'", cxxopts::value()) ("c,config", "Valhalla configuration file", cxxopts::value()) ("i,inline-config", "Inline JSON config", cxxopts::value()) - ("f,file", "GeoJSON file name. If omitted program will print to stdout.", cxxopts::value()); + ("f,file", "file name. If omitted program will print to stdout.", cxxopts::value()); // clang-format on auto result = options.parse(argc, argv); @@ -79,6 +80,12 @@ int main(int argc, char* argv[]) { // Process json request 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 @@ -182,7 +189,7 @@ int main(int argc, char* argv[]) { auto expansion_type = routetype == "multimodal" ? ExpansionType::multimodal : (reverse ? ExpansionType::reverse : ExpansionType::forward); - auto isotile = isochrone.Expand(expansion_type, request, reader, mode_costing, mode); + auto isogrid = isochrone.Expand(expansion_type, request, reader, mode_costing, mode); auto t2 = std::chrono::high_resolution_clock::now(); uint32_t msecs = std::chrono::duration_cast(t2 - t1).count(); @@ -190,27 +197,30 @@ int main(int argc, char* argv[]) { // Generate contours t2 = std::chrono::high_resolution_clock::now(); - auto contours = isotile->GenerateContours(contour_times, polygons, denoise, generalize); auto t3 = std::chrono::high_resolution_clock::now(); msecs = std::chrono::duration_cast(t3 - t2).count(); LOG_INFO("Contour Generation took " + std::to_string(msecs) + " ms"); // Serialize to GeoJSON - std::string geojson = - valhalla::tyr::serializeIsochrones(request, contour_times, contours, polygons, show_locations); + std::string res = valhalla::tyr::serializeIsochrones(request, contour_times, isogrid); +#ifdef ENABLE_GDAL + if (request.options().format() == Options_Format_geotiff) { + GDALDestroyDriverManager(); + } +#endif auto t4 = std::chrono::high_resolution_clock::now(); msecs = std::chrono::duration_cast(t4 - t3).count(); - LOG_INFO("GeoJSON serialization took " + std::to_string(msecs) + " ms"); + LOG_INFO("Isochrone serialization took " + std::to_string(msecs) + " ms"); msecs = std::chrono::duration_cast(t4 - t1).count(); LOG_INFO("Isochrone took " + std::to_string(msecs) + " ms"); if (!filename.empty()) { - std::ofstream geojsonOut(filename, std::ofstream::out); - geojsonOut << geojson; - geojsonOut.close(); + std::ofstream resOut(filename, std::ofstream::out); + resOut << res; + resOut.close(); } else { - std::cout << "\n" << geojson << std::endl; + std::cout << "\n" << res << std::endl; } // Shutdown protocol buffer library diff --git a/src/valhalla_service.cc b/src/valhalla_service.cc index 95c753259d..4defc13195 100644 --- a/src/valhalla_service.cc +++ b/src/valhalla_service.cc @@ -17,6 +17,10 @@ using namespace prime_server; #endif +#ifdef ENABLE_GDAL +#include +#endif + #include "config.h" #include "midgard/logging.h" @@ -39,6 +43,10 @@ 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/src/worker.cc b/src/worker.cc index bacc47eabf..c069bd23a3 100644 --- a/src/worker.cc +++ b/src/worker.cc @@ -138,6 +138,8 @@ const std::unordered_map error_codes{ {445, {445, "Shape match algorithm specification in api request is incorrect. Please see documentation for valid shape_match input.", 400, HTTP_400, OSRM_INVALID_URL, "wrong_match_type"}}, {499, {499, "Unknown", 400, HTTP_400, OSRM_INVALID_URL, "unknown"}}, {503, {503, "Leg count mismatch", 400, HTTP_400, OSRM_INVALID_URL, "wrong_number_of_legs"}}, + {504, {504, "This service does not support GeoTIFF serialization.", 400, HTTP_400, OSRM_INVALID_VALUE, "wrong_number_of_legs"}}, + {599, {599, "Unknown serialization error", 400, HTTP_400, OSRM_INVALID_VALUE, "unknown"}}, }; // unordered map for warning pairs diff --git a/test/isochrone.cc b/test/isochrone.cc index 37eed4be9a..ddb020675e 100644 --- a/test/isochrone.cc +++ b/test/isochrone.cc @@ -14,6 +14,10 @@ #include #include +#ifdef ENABLE_GDAL +#include +#endif + using point_type = boost::geometry::model::d2::point_xy; using polygon_type = boost::geometry::model::polygon; using boost::geometry::within; @@ -329,6 +333,51 @@ TEST(Isochrones, test_max_reserved_labels_count) { isochrone.Clear(); } +#ifdef ENABLE_GDAL +TEST(Isochrones, test_geotiff_output) { + GDALRegister_GTiff(); + loki_worker_t loki_worker(cfg); + thor_worker_t thor_worker(cfg); + + const auto request = + R"({"costing":"auto","locations":[{"lon":5.042799,"lat":52.093199}],"contours":[{"time":1}], "format": "geotiff"})"; + Api request_pbf; + ParseApi(request, Options::isochrone, request_pbf); + loki_worker.isochrones(request_pbf); + std::string geotiff = thor_worker.isochrones(request_pbf); + + std::string name = "/vsimem/test_isogrid_geotiff.tif"; + unsigned char buffer[geotiff.length()]; + memcpy(buffer, geotiff.data(), geotiff.length()); + auto handle = VSIFileFromMemBuffer(name.c_str(), buffer, static_cast(geotiff.size()), 0); + auto geotiff_dataset = GDALDataset::FromHandle(GDALOpen(name.c_str(), GA_ReadOnly)); + int x = geotiff_dataset->GetRasterXSize(); + int y = geotiff_dataset->GetRasterYSize(); + GDALRasterBand* band = geotiff_dataset->GetRasterBand(1); + uint16_t dataArray[x * y]; + std::cerr << x << ", " << y << std::endl; + CPLErr err = band->RasterIO(GF_Read, 0, 0, x, y, dataArray, x, y, GDT_UInt16, 0, 0); + double min_max[2]; + + band->ComputeRasterMinMax(0, min_max); + + ASSERT_EQ(err, CE_None); + ASSERT_EQ(x, 145); + ASSERT_EQ(y, 97); + ASSERT_EQ(static_cast(min_max[0]), 0); + size_t array_size = x * y; + + // make sure there are some grid cells whose metric value is neither 0 nor the max + bool no_intermediate_values = true; + for (size_t i = 0; i < static_cast(array_size); ++i) { + if (dataArray[i] > 0 && dataArray[i] < min_max[1]) + no_intermediate_values = false; + } + ASSERT_EQ(no_intermediate_values, false); + VSIFCloseL(handle); +} +#endif + } // namespace int main(int argc, char* argv[]) { diff --git a/valhalla/midgard/gridded_data.h b/valhalla/midgard/gridded_data.h index da1d675487..700418cae7 100644 --- a/valhalla/midgard/gridded_data.h +++ b/valhalla/midgard/gridded_data.h @@ -61,6 +61,14 @@ template class GriddedData : public Tiles { } } + float getData(size_t tileid, size_t metricid) const { + return data_[tileid][metricid]; + } + + float MaxValue(size_t metrix_idx) const { + return max_value_[metrix_idx]; + } + using contour_t = std::list; using feature_t = std::list; using contours_t = std::vector>; @@ -365,7 +373,6 @@ template class GriddedData : public Tiles { } // some info about the area the image covers - auto c = this->TileBounds().Center(); auto h = this->tilesize_ / 2; // for each contour for (auto& collection : contours) { @@ -414,6 +421,33 @@ template class GriddedData : public Tiles { return contours; } + /** + * Determine the smallest subgrid that contains all valid (i.e. non-max) values + + * @return array with 4 elements: minimum column, minimum row, maximum column, maximum row + */ + const std::array MinExtent() const { + // minx, miny, maxx, maxy + std::array box = {this->ncolumns_ / 2, this->nrows_ / 2, this->ncolumns_ / 2, + this->nrows_ / 2}; + + for (int32_t i = 0; i < this->nrows_; ++i) { + for (int32_t j = 0; j < this->ncolumns_; ++j) { + if (data_[this->TileId(j, i)][0] < max_value_[0] || + data_[this->TileId(j, i)][1] < max_value_[1]) { + // pad by 1 row/column as a sanity check + box[0] = std::min(std::max(j - 1, 0), box[0]); + box[1] = std::min(std::max(i - 1, 0), box[1]); + // +1 extra because range is exclusive + box[2] = std::max(std::min(j + 2, this->ncolumns_ - 1), box[2]); + box[3] = std::max(std::min(i + 2, this->ncolumns_ - 1), box[3]); + } + } + } + + return box; + } + protected: value_type max_value_; // Maximum value stored in the tile std::vector data_; // Data value within each tile diff --git a/valhalla/thor/worker.h b/valhalla/thor/worker.h index 7334cdcac2..776c6d9b2e 100644 --- a/valhalla/thor/worker.h +++ b/valhalla/thor/worker.h @@ -6,6 +6,9 @@ #include #include +#ifdef ENABLE_GDAL +#include +#endif #include #include @@ -35,6 +38,10 @@ namespace valhalla { namespace thor { +#ifdef ENABLE_GDAL +typedef struct GDALDriver* geotiff_driver_t; +#endif + #ifdef ENABLE_SERVICES void run_service(const boost::property_tree::ptree& config); #endif @@ -140,6 +147,10 @@ class thor_worker_t : public service_worker_t { std::string service_name() const override { return "thor"; } + +#ifdef ENABLE_GDAL + geotiff_driver_t geotiff_driver; +#endif }; } // namespace thor diff --git a/valhalla/tyr/serializers.h b/valhalla/tyr/serializers.h index ecb9fdb319..b5a4f00107 100644 --- a/valhalla/tyr/serializers.h +++ b/valhalla/tyr/serializers.h @@ -7,6 +7,11 @@ #include #include +#ifdef ENABLE_GDAL +#include "valhalla/thor/worker.h" +#include +#endif + #include #include #include @@ -40,10 +45,13 @@ std::string serializeMatrix(Api& request); */ std::string serializeIsochrones(Api& request, std::vector::contour_interval_t>& intervals, - midgard::GriddedData<2>::contours_t& contours, - bool polygons = true, - bool show_locations = false); - + std::shared_ptr> isogrid +#ifdef ENABLE_GDAL + , + valhalla::thor::geotiff_driver_t geotiff_driver); +#else +); +#endif /** * Turn heights and ranges into a height response * From 2ba6064528a965a532dad5a174a98f6000ebbe00 Mon Sep 17 00:00:00 2001 From: Christian Beiwinkel Date: Tue, 20 Feb 2024 12:28:31 +0100 Subject: [PATCH 03/27] docs, changelog --- CHANGELOG.md | 1 + docs/docs/api/isochrone/api-reference.md | 8 +++++--- docs/docs/api/turn-by-turn/api-reference.md | 1 + docs/docs/thor/isochrones.md | 2 +- 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 49d9dfe985..7f92f46aaf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -93,6 +93,7 @@ * ADDED: Improved instructions for blind users [#3694](https://github.com/valhalla/valhalla/pull/3694) * FIXED: Fixed roundoff issue in Tiles Row and Col methods [#4585](https://github.com/valhalla/valhalla/pull/4585) * ADDED: isochrone proper polygon support & pbf output for isochrone [#4575](https://github.com/valhalla/valhalla/pull/4575) + * ADDED: return isotile grid as geotiff []() ## Release Date: 2023-05-11 Valhalla 3.4.0 * **Removed** diff --git a/docs/docs/api/isochrone/api-reference.md b/docs/docs/api/isochrone/api-reference.md index 9972a5ad87..27d82a047b 100644 --- a/docs/docs/api/isochrone/api-reference.md +++ b/docs/docs/api/isochrone/api-reference.md @@ -51,9 +51,9 @@ The isochrone service uses the `auto`, `bicycle`, `pedestrian`, and `multimodal` ## Outputs of the Isochrone service -In the service response, the isochrone contours are returned as [GeoJSON](http://geojson.org/), which can be integrated into mapping applications. +In the service response, the isochrone contours are returned as [GeoJSON](http://geojson.org/), which can be integrated into mapping applications. Alternatively, the grid data that underlies these contours can be returned as a [GeoTIFF](https://www.ogc.org/standard/geotiff/). -The contours are calculated using rasters and are returned as either polygon or line features, depending on your input setting for the `polygons` parameter. If an isochrone request has been named using the optional `&id=` input, then the `id` is returned as a name property for the feature collection within the GeoJSON response. A `metric` attribute lets you know whether it's a `distance` or `time` contour. A warnings array may also be included. This array may contain warning objects informing about deprecated request parameters, clamped values etc. | +The isochrone service returns contours as GeoJSON line or polygon features for the requested intervals (depending on the value of the `polygons` request parameter). These contours are calculated using a two dimensional grid. If the `format` request parameter is set to `geotiff`, the underlying grid data is returned directly instead of the contours derived from it. It will return one band for each requested metric (i.e. one for `time` and one for `distance`). If an isochrone request has been named using the optional `&id=` input, then the `id` is returned as a name property for the feature collection within the GeoJSON response. A `metric` attribute lets you know whether it's a `distance` or `time` contour. A warnings array may also be included. This array may contain warning objects informing about deprecated request parameters, clamped values etc. | See the [HTTP return codes](../turn-by-turn/api-reference.md#http-status-codes-and-conditions) for more on messages you might receive from the service. @@ -63,6 +63,8 @@ Most JavaScript-based GeoJSON renderers, including [Leaflet](http://leafletjs.co When making a map, drawing the isochrone contours as lines is more straightforward than polygons, and, therefore, currently is the default and recommended method. When deciding between the output as lines and polygons, consider your use case and the additional styling considerations involved with polygons. For example, fills should be rendered as semi-transparent over the other map layers so they are visible, although you may have more flexibility when using a vector-based map. In addition, polygons from multiple contour levels do not have overlapping areas cut out or removed. In other words, the outer contours include the areas of any inner contours, causing the colors and transparencies to blend when multiple contour polygons are drawn at the same time. +(TODO: write something about rendering the GeoTIFF output.) + ## Future work on the isochrone service The Isochrone service is in active development. To report software issues or suggest enhancements, open an issue in the [Valhalla GitHub repository](https://github.com/valhalla/valhalla/issues). @@ -75,7 +77,7 @@ Several other options are being considered as future service enhancements. These * ~~Removing self intersections from polygonal contours.~~ * ~~Allowing multiple locations to compute the region reachable from any of the locations within a specified time.~~ * ~~Generating contours with reverse access logic to see the region that can reach a specific location within the specified time.~~ -* Returning raster data for potential animation using OpenGL shaders. This also has analysis use for being able to query thousands of locations to determine the time to each location, including improvements with one-to-many requests to the Valhalla Time-Distance Matrix service. +* ~~Returning raster data for potential animation using OpenGL shaders. This also has analysis use for being able to query thousands of locations to determine the time to each location, including improvements with one-to-many requests to the Valhalla Time-Distance Matrix service.~~ ## Data credits diff --git a/docs/docs/api/turn-by-turn/api-reference.md b/docs/docs/api/turn-by-turn/api-reference.md index 18c131d0a6..1ee45e701e 100644 --- a/docs/docs/api/turn-by-turn/api-reference.md +++ b/docs/docs/api/turn-by-turn/api-reference.md @@ -620,4 +620,5 @@ The codes correspond to code returned from a particular [Valhalla project](https |**5xx** | **Tyr project codes** | |500 | Failed to parse intermediate request format | |501 | Failed to parse TripDirections | +|504 | GeoTIFF serialization not supported by service | |599 | Unknown | diff --git a/docs/docs/thor/isochrones.md b/docs/docs/thor/isochrones.md index 74e4eedbe0..54d47675b3 100644 --- a/docs/docs/thor/isochrones.md +++ b/docs/docs/thor/isochrones.md @@ -27,7 +27,7 @@ The 2-D grid is used to find the isocrhone contours by using a well-known contou After forming sets of contour polygons, KEVIN -please write a paragraph or 2 to describe how the contours are formed and output! -This 2-D grid of times can be useful for other purposes as well. It provides a very fast way to query a single location to see how long it takes to get there from the test location. Ultimately this could be a way to do very large one-to-many matrices. At this time we do not return the 2-D array of times, but this is a possibility in the future. +This 2-D grid can be useful for other purposes as well. It provides a very fast way to query a single location to see how long it takes to get there from the test location. Ultimately this could be a way to do very large one-to-many matrices. Where is it? ------------ From 91bc25065cd6bdba80bfc0be6f6a76211db3c918 Mon Sep 17 00:00:00 2001 From: Christian Beiwinkel Date: Tue, 20 Feb 2024 13:39:35 +0100 Subject: [PATCH 04/27] fix run_isochrones, changelog entry --- CHANGELOG.md | 2 +- src/valhalla_run_isochrone.cc | 15 +++++++++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f92f46aaf..bb00b58ea7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -93,7 +93,7 @@ * ADDED: Improved instructions for blind users [#3694](https://github.com/valhalla/valhalla/pull/3694) * FIXED: Fixed roundoff issue in Tiles Row and Col methods [#4585](https://github.com/valhalla/valhalla/pull/4585) * ADDED: isochrone proper polygon support & pbf output for isochrone [#4575](https://github.com/valhalla/valhalla/pull/4575) - * ADDED: return isotile grid as geotiff []() + * ADDED: return isotile grid as geotiff [#4594](https://github.com/valhalla/valhalla/pull/4594) ## Release Date: 2023-05-11 Valhalla 3.4.0 * **Removed** diff --git a/src/valhalla_run_isochrone.cc b/src/valhalla_run_isochrone.cc index 04b565cb47..bd94dbf76b 100644 --- a/src/valhalla_run_isochrone.cc +++ b/src/valhalla_run_isochrone.cc @@ -2,7 +2,6 @@ #include #include #include -#include #include #include #include @@ -21,6 +20,10 @@ #include "tyr/serializers.h" #include "worker.h" +#ifdef ENABLE_GDAL +#include +#endif + using namespace valhalla; using namespace valhalla::midgard; using namespace valhalla::baldr; @@ -201,13 +204,17 @@ int main(int argc, char* argv[]) { msecs = std::chrono::duration_cast(t3 - t2).count(); LOG_INFO("Contour Generation took " + std::to_string(msecs) + " ms"); - // Serialize to GeoJSON - std::string res = valhalla::tyr::serializeIsochrones(request, contour_times, isogrid); - #ifdef ENABLE_GDAL + auto driver_manager = GetGDALDriverManager(); + geotiff_driver_t driver = driver_manager->GetDriverByName("GTiff"); + // Serialize to GeoTIFF + std::string res = valhalla::tyr::serializeIsochrones(request, contour_times, isogrid, driver); if (request.options().format() == Options_Format_geotiff) { GDALDestroyDriverManager(); } +#else + // Serialize to GeoJSON + std::string res = valhalla::tyr::serializeIsochrones(request, contour_times, isogrid); #endif auto t4 = std::chrono::high_resolution_clock::now(); msecs = std::chrono::duration_cast(t4 - t3).count(); From df965af5d4041414df62dc64f357482366932f6e Mon Sep 17 00:00:00 2001 From: Christian Beiwinkel Date: Tue, 20 Feb 2024 14:24:24 +0100 Subject: [PATCH 05/27] clang tidy --- src/tyr/isochrone_serializer.cc | 2 +- valhalla/tyr/serializers.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tyr/isochrone_serializer.cc b/src/tyr/isochrone_serializer.cc index abc0cb98d5..4039f1ebdf 100644 --- a/src/tyr/isochrone_serializer.cc +++ b/src/tyr/isochrone_serializer.cc @@ -358,7 +358,7 @@ namespace tyr { std::string serializeIsochrones(Api& request, std::vector::contour_interval_t>& intervals, - std::shared_ptr> isogrid + const std::shared_ptr>& isogrid #ifdef ENABLE_GDAL , valhalla::thor::geotiff_driver_t geotiff_driver) { diff --git a/valhalla/tyr/serializers.h b/valhalla/tyr/serializers.h index b5a4f00107..2958048457 100644 --- a/valhalla/tyr/serializers.h +++ b/valhalla/tyr/serializers.h @@ -45,7 +45,7 @@ std::string serializeMatrix(Api& request); */ std::string serializeIsochrones(Api& request, std::vector::contour_interval_t>& intervals, - std::shared_ptr> isogrid + const std::shared_ptr>& isogrid #ifdef ENABLE_GDAL , valhalla::thor::geotiff_driver_t geotiff_driver); From a3235015b12bcbd4f330c3cd7043a9b549fab5de Mon Sep 17 00:00:00 2001 From: Christian Beiwinkel Date: Tue, 20 Feb 2024 14:46:00 +0100 Subject: [PATCH 06/27] add to dep install script, add cmake flag to docs --- docs/docs/building.md | 1 + scripts/install-linux-deps.sh | 1 + 2 files changed, 2 insertions(+) diff --git a/docs/docs/building.md b/docs/docs/building.md index 30ad5fde32..ea8ea0c315 100644 --- a/docs/docs/building.md +++ b/docs/docs/building.md @@ -27,6 +27,7 @@ Important build options include: | `-DENABLE_ADDRESS_SANITIZER` (`ON` / `OFF`) | Build with address sanitizer (defaults to off).| | `-DENABLE_UNDEFINED_SANITIZER` (`ON` / `OFF`) | Build with undefined behavior sanitizer (defaults to off).| | `-DPREFER_SYSTEM_DEPS` (`ON` / `OFF`) | Whether to use internally vendored headers or find the equivalent external package (defaults to off).| +| `-DENABLE_GDAL` (`ON` / `OFF`) | Whether to include GDAL as a dependency (used for GeoTIFF serialization of isochrone grid) (defaults to off).| ### Building with `vcpkg` - any platform diff --git a/scripts/install-linux-deps.sh b/scripts/install-linux-deps.sh index 4b3e3bae80..eb2b65d16d 100755 --- a/scripts/install-linux-deps.sh +++ b/scripts/install-linux-deps.sh @@ -22,6 +22,7 @@ env DEBIAN_FRONTEND=noninteractive sudo apt install --yes --quiet \ libboost-all-dev \ libcurl4-openssl-dev \ libczmq-dev \ + libgdal-dev \ libgeos++-dev \ libgeos-dev \ libluajit-5.1-dev \ From 461870fb4238adb4fa05167ced0de6359659bba5 Mon Sep 17 00:00:00 2001 From: Christian Beiwinkel Date: Tue, 20 Feb 2024 14:55:03 +0100 Subject: [PATCH 07/27] tidy complains about this macro --- src/tyr/isochrone_serializer.cc | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/tyr/isochrone_serializer.cc b/src/tyr/isochrone_serializer.cc index 4039f1ebdf..2c4511a69e 100644 --- a/src/tyr/isochrone_serializer.cc +++ b/src/tyr/isochrone_serializer.cc @@ -356,14 +356,15 @@ std::string serializeIsochronePbf(Api& request, namespace valhalla { namespace tyr { +#ifdef ENABLE_GDAL std::string serializeIsochrones(Api& request, std::vector::contour_interval_t>& intervals, - const std::shared_ptr>& isogrid -#ifdef ENABLE_GDAL - , + const std::shared_ptr>& isogrid, valhalla::thor::geotiff_driver_t geotiff_driver) { #else -) { +std::string serializeIsochrones(Api& request, + std::vector::contour_interval_t>& intervals, + const std::shared_ptr>& isogrid) { #endif // only generate if json or pbf output is requested From 7c78627aea4e5ae83bdf84cc1426939e3de6dd6b Mon Sep 17 00:00:00 2001 From: Christian Beiwinkel Date: Tue, 20 Feb 2024 18:21:14 +0100 Subject: [PATCH 08/27] minor fixes, added more tests, cleanup --- src/tyr/isochrone_serializer.cc | 15 ++--- test/isochrone.cc | 103 ++++++++++++++++++++++++++++++-- valhalla/midgard/gridded_data.h | 6 +- 3 files changed, 106 insertions(+), 18 deletions(-) diff --git a/src/tyr/isochrone_serializer.cc b/src/tyr/isochrone_serializer.cc index 2c4511a69e..556b400bf3 100644 --- a/src/tyr/isochrone_serializer.cc +++ b/src/tyr/isochrone_serializer.cc @@ -189,25 +189,21 @@ std::string serializeGeoTIFF(Api& request, for (size_t metric_idx = 0; metric_idx < metrics.size(); ++metric_idx) { if (!metrics[metric_idx]) continue; // only create bands for requested metrics - uint16_t dataArray[ext_x * ext_y]; + uint16_t data[ext_x * ext_y]; // seconds or 10 meter steps - float scale_factor = metric_idx == 0 ? 3600 : 100; + float scale_factor = metric_idx == 0 ? 60 : 100; for (int32_t i = 0; i < ext_y; ++i) { for (int32_t j = 0; j < ext_x; ++j) { auto tileid = isogrid->TileId(j + box[0], i + box[1]); - dataArray[i * ext_x + j] = - static_cast(isogrid->getData(tileid, metric_idx) * scale_factor); + data[i * ext_x + j] = + static_cast(isogrid->DataAt(tileid, metric_idx) * scale_factor); } } auto band = geotiff_dataset->GetRasterBand(nbands == 2 ? (metric_idx + 1) : 1); band->SetDescription(metric_idx == 0 ? "Time (seconds)" : "Distance (10m)"); - CPLErr err = - band->RasterIO(GF_Write, 0, 0, ext_x, ext_y, dataArray, ext_x, ext_y, GDT_UInt16, 0, 0); - - // free some memory - delete[] geotiff_options; + CPLErr err = band->RasterIO(GF_Write, 0, 0, ext_x, ext_y, data, ext_x, ext_y, GDT_UInt16, 0, 0); if (err != CE_None) { throw valhalla_exception_t{599, "Unknown error when writing GeoTIFF."}; @@ -218,7 +214,6 @@ std::string serializeGeoTIFF(Api& request, vsi_l_offset bufferlength; GByte* bytes = VSIGetMemFileBuffer(name.c_str(), &bufferlength, TRUE); - // TODO: there must be a way to do this without copying std::string data(reinterpret_cast(bytes), bufferlength); return data; diff --git a/test/isochrone.cc b/test/isochrone.cc index ddb020675e..68afc70ee4 100644 --- a/test/isochrone.cc +++ b/test/isochrone.cc @@ -334,7 +334,50 @@ TEST(Isochrones, test_max_reserved_labels_count) { } #ifdef ENABLE_GDAL -TEST(Isochrones, test_geotiff_output) { +TEST(Isochrones, test_geotiff_output_distance) { + GDALRegister_GTiff(); + loki_worker_t loki_worker(cfg); + thor_worker_t thor_worker(cfg); + + const auto request = + R"({"costing":"auto","locations":[{"lon":5.042799,"lat":52.093199}],"contours":[{"distance":1}], "format": "geotiff"})"; + Api request_pbf; + ParseApi(request, Options::isochrone, request_pbf); + loki_worker.isochrones(request_pbf); + std::string geotiff = thor_worker.isochrones(request_pbf); + + std::string name = "/vsimem/test_isogrid_geotiff_d.tif"; + unsigned char buffer[geotiff.length()]; + std::copy(geotiff.cbegin(), geotiff.cend(), buffer); + auto handle = VSIFileFromMemBuffer(name.c_str(), buffer, static_cast(geotiff.size()), 0); + auto geotiff_dataset = GDALDataset::FromHandle(GDALOpen(name.c_str(), GA_ReadOnly)); + int x = geotiff_dataset->GetRasterXSize(); + int y = geotiff_dataset->GetRasterYSize(); + GDALRasterBand* band = geotiff_dataset->GetRasterBand(1); + uint16_t dataArray[x * y]; + CPLErr err = band->RasterIO(GF_Read, 0, 0, x, y, dataArray, x, y, GDT_UInt16, 0, 0); + double min_max[2]; + + band->ComputeRasterMinMax(0, min_max); + + ASSERT_EQ(err, CE_None); + ASSERT_EQ(x, 144); + ASSERT_EQ(y, 97); + ASSERT_EQ(static_cast(min_max[0]), 0); + ASSERT_EQ(static_cast(min_max[1]), 1100); + size_t array_size = x * y; + + // make sure there are some grid cells whose metric value is neither 0 nor the max + bool no_intermediate_values = true; + for (size_t i = 0; i < array_size; ++i) { + if (dataArray[i] > 0 && dataArray[i] < min_max[1]) + no_intermediate_values = false; + } + ASSERT_EQ(no_intermediate_values, false); + VSIFCloseL(handle); +} + +TEST(Isochrones, test_geotiff_output_time) { GDALRegister_GTiff(); loki_worker_t loki_worker(cfg); thor_worker_t thor_worker(cfg); @@ -346,16 +389,15 @@ TEST(Isochrones, test_geotiff_output) { loki_worker.isochrones(request_pbf); std::string geotiff = thor_worker.isochrones(request_pbf); - std::string name = "/vsimem/test_isogrid_geotiff.tif"; + std::string name = "/vsimem/test_isogrid_geotiff_t.tif"; unsigned char buffer[geotiff.length()]; - memcpy(buffer, geotiff.data(), geotiff.length()); + std::copy(geotiff.cbegin(), geotiff.cend(), buffer); auto handle = VSIFileFromMemBuffer(name.c_str(), buffer, static_cast(geotiff.size()), 0); auto geotiff_dataset = GDALDataset::FromHandle(GDALOpen(name.c_str(), GA_ReadOnly)); int x = geotiff_dataset->GetRasterXSize(); int y = geotiff_dataset->GetRasterYSize(); GDALRasterBand* band = geotiff_dataset->GetRasterBand(1); uint16_t dataArray[x * y]; - std::cerr << x << ", " << y << std::endl; CPLErr err = band->RasterIO(GF_Read, 0, 0, x, y, dataArray, x, y, GDT_UInt16, 0, 0); double min_max[2]; @@ -365,17 +407,68 @@ TEST(Isochrones, test_geotiff_output) { ASSERT_EQ(x, 145); ASSERT_EQ(y, 97); ASSERT_EQ(static_cast(min_max[0]), 0); + ASSERT_EQ(static_cast(min_max[1]), 660); size_t array_size = x * y; // make sure there are some grid cells whose metric value is neither 0 nor the max bool no_intermediate_values = true; - for (size_t i = 0; i < static_cast(array_size); ++i) { + for (size_t i = 0; i < array_size; ++i) { if (dataArray[i] > 0 && dataArray[i] < min_max[1]) no_intermediate_values = false; } ASSERT_EQ(no_intermediate_values, false); VSIFCloseL(handle); } + +// 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); + + const auto request = + R"({"costing":"auto","locations":[{"lon":5.042799,"lat":52.093199}],"contours":[{"time":1},{"distance":2}], "format": "geotiff"})"; + Api request_pbf; + ParseApi(request, Options::isochrone, request_pbf); + loki_worker.isochrones(request_pbf); + std::string geotiff = thor_worker.isochrones(request_pbf); + + std::string name = "/vsimem/test_isogrid_geotiff_td.tif"; + unsigned char buffer[geotiff.length()]; + std::copy(geotiff.cbegin(), geotiff.cend(), buffer); + auto handle = VSIFileFromMemBuffer(name.c_str(), buffer, static_cast(geotiff.size()), 0); + auto geotiff_dataset = GDALDataset::FromHandle(GDALOpen(name.c_str(), GA_ReadOnly)); + int x = geotiff_dataset->GetRasterXSize(); + int y = geotiff_dataset->GetRasterYSize(); + + // time, distance + std::array expected_max{660, 1200}; + + for (int i = 1; i <= 2; ++i) { + GDALRasterBand* band = geotiff_dataset->GetRasterBand(i); + uint16_t dataArray[x * y]; + CPLErr err = band->RasterIO(GF_Read, 0, 0, x, y, dataArray, x, y, GDT_UInt16, 0, 0); + double min_max[2]; + + band->ComputeRasterMinMax(0, min_max); + + ASSERT_EQ(err, CE_None); + ASSERT_EQ(x, 147); + ASSERT_EQ(y, 97); + ASSERT_EQ(static_cast(min_max[0]), 0); + ASSERT_EQ(static_cast(min_max[1]), expected_max[i - 1]); + size_t array_size = x * y; + + // make sure there are some grid cells whose metric value is neither 0 nor the max + bool no_intermediate_values = true; + for (size_t j = 0; j < array_size; ++j) { + if (dataArray[j] > 0 && dataArray[j] < min_max[1]) + no_intermediate_values = false; + } + ASSERT_EQ(no_intermediate_values, false); + } + VSIFCloseL(handle); +} #endif } // namespace diff --git a/valhalla/midgard/gridded_data.h b/valhalla/midgard/gridded_data.h index 700418cae7..11d4e0d55f 100644 --- a/valhalla/midgard/gridded_data.h +++ b/valhalla/midgard/gridded_data.h @@ -61,12 +61,12 @@ template class GriddedData : public Tiles { } } - float getData(size_t tileid, size_t metricid) const { + float DataAt(size_t tileid, size_t metricid) const { return data_[tileid][metricid]; } - float MaxValue(size_t metrix_idx) const { - return max_value_[metrix_idx]; + float MaxValue(size_t metricidx) const { + return max_value_[metricidx]; } using contour_t = std::list; From d7bd63a010809d8031bd1ef1455fc1a76999a4e8 Mon Sep 17 00:00:00 2001 From: Christian Beiwinkel Date: Wed, 21 Feb 2024 11:03:43 +0100 Subject: [PATCH 09/27] use pkgconfig, try out CI build with GDAL --- .github/workflows/osx.yml | 4 ++-- CMakeLists.txt | 4 ++-- cmake/ValhallaPkgConfig.cmake | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/osx.yml b/.github/workflows/osx.yml index 004233182c..100c22dd4d 100644 --- a/.github/workflows/osx.yml +++ b/.github/workflows/osx.yml @@ -40,7 +40,7 @@ jobs: - name: Install dependencies run: | - HOMEBREW_NO_AUTO_UPDATE=1 brew install autoconf automake protobuf cmake ccache libtool sqlite3 libspatialite luajit curl wget czmq lz4 spatialite-tools unzip boost + HOMEBREW_NO_AUTO_UPDATE=1 brew install autoconf automake protobuf cmake ccache libtool sqlite3 libspatialite luajit curl wget czmq lz4 spatialite-tools unzip boost gdal sudo python -m pip install --break-system-packages requests shapely git clone https://github.com/kevinkreiser/prime_server --recurse-submodules && cd prime_server && ./autogen.sh && ./configure && make -j$(sysctl -n hw.logicalcpu) && sudo make install @@ -58,7 +58,7 @@ jobs: ccache-osx- - name: Configure CMake - run: cmake -B build -DENABLE_SINGLE_FILES_WERROR=OFF + run: cmake -B build -DENABLE_SINGLE_FILES_WERROR=OFF -DENABLE_GDAL=ON - name: Build Valhalla run: make -C build -j$(sysctl -n hw.logicalcpu) diff --git a/CMakeLists.txt b/CMakeLists.txt index 58af53bfe9..d975993134 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -207,9 +207,9 @@ endif() # gdal set(gdal_target "") if (ENABLE_GDAL) - find_package(GDAL REQUIRED) + pkg_check_modules(GDAL REQUIRED IMPORTED_TARGET gdal) add_compile_definitions(ENABLE_GDAL) - set(gdal_target GDAL::GDAL) + set(gdal_target PkgConfig::GDAL) endif() diff --git a/cmake/ValhallaPkgConfig.cmake b/cmake/ValhallaPkgConfig.cmake index 7a7bfd1a20..a0483c801a 100644 --- a/cmake/ValhallaPkgConfig.cmake +++ b/cmake/ValhallaPkgConfig.cmake @@ -34,7 +34,7 @@ function(configure_valhalla_pc) list(APPEND REQUIRES libprime_server) endif() if(ENABLE_GDAL) - list(APPEND REQUIRES GDAL::GDAL) + list(APPEND REQUIRES gdal) endif() if(WIN32 AND NOT MINGW) list(APPEND LIBS_PRIVATE -lole32 -lshell32) From 8f986649cd6d9bc76e1c180adde9e3fd38a4f8ce Mon Sep 17 00:00:00 2001 From: Christian Beiwinkel Date: Wed, 21 Feb 2024 14:03:57 +0100 Subject: [PATCH 10/27] properly hide geotiff driver --- src/thor/worker.cc | 30 +++++++++++++++++++++--------- src/tyr/isochrone_serializer.cc | 2 +- valhalla/thor/worker.h | 17 ++++++++++++++++- 3 files changed, 38 insertions(+), 11 deletions(-) diff --git a/src/thor/worker.cc b/src/thor/worker.cc index ab91d1652d..5afeaebd0c 100644 --- a/src/thor/worker.cc +++ b/src/thor/worker.cc @@ -78,7 +78,7 @@ thor_worker_t::thor_worker_t(const boost::property_tree::ptree& config, time_distance_bss_matrix_(config.get_child("thor")), isochrone_gen(config.get_child("thor")), reader(graph_reader ? graph_reader : std::make_shared(config.get_child("mjolnir"))), - matcher_factory(config, reader), controller{} { + matcher_factory(config, reader), controller{}, geotiff_driver(geotiff_driver_t{}) { // Select the matrix algorithm based on the conf file (defaults to // select_optimal if not present) @@ -110,19 +110,11 @@ thor_worker_t::thor_worker_t(const boost::property_tree::ptree& config, max_timedep_distance = config.get("service_limits.max_timedep_distance", kDefaultMaxTimeDependentDistance); -#ifdef ENABLE_GDAL - auto driver_manager = GetGDALDriverManager(); - geotiff_driver = driver_manager->GetDriverByName("GTiff"); -#endif - // signal that the worker started successfully started(); } thor_worker_t::~thor_worker_t() { -#ifdef ENABLE_GDAL - GDALDestroyDriverManager(); -#endif } #ifdef ENABLE_SERVICES @@ -342,5 +334,25 @@ void thor_worker_t::set_interrupt(const std::function* interrupt_functio interrupt = interrupt_function; reader->SetInterrupt(interrupt); } + +#ifdef ENABLE_GDAL +geotiff_driver_t::geotiff_driver_t() { + auto driver_manager = GetGDALDriverManager(); + this->geotiff_driver = driver_manager->GetDriverByName("GTiff"); +} + +geotiff_driver_t::~geotiff_driver_t() { + // GDALDestroyDriverManager(); +} + +GDALDataset* geotiff_driver_t::CreateDataSet(const char* pszName, + int nXSize, + int nYSize, + int nBands, + GDALDataType eType, + CSLConstList papszOptions) { + return this->geotiff_driver->Create(pszName, nXSize, nYSize, nBands, eType, papszOptions); +} +#endif } // namespace thor } // namespace valhalla diff --git a/src/tyr/isochrone_serializer.cc b/src/tyr/isochrone_serializer.cc index 556b400bf3..a6dbcc6c1d 100644 --- a/src/tyr/isochrone_serializer.cc +++ b/src/tyr/isochrone_serializer.cc @@ -172,7 +172,7 @@ std::string serializeGeoTIFF(Api& request, geotiff_options = CSLSetNameValue(geotiff_options, "COMPRESS", "PACKBITS"); auto geotiff_dataset = - geotiff_driver->Create(name.c_str(), ext_x, ext_y, nbands, GDT_UInt16, geotiff_options); + geotiff_driver.CreateDataSet(name.c_str(), ext_x, ext_y, nbands, GDT_UInt16, geotiff_options); OGRSpatialReference spatial_ref; spatial_ref.SetWellKnownGeogCS("EPSG:4326"); diff --git a/valhalla/thor/worker.h b/valhalla/thor/worker.h index 776c6d9b2e..3674b344a4 100644 --- a/valhalla/thor/worker.h +++ b/valhalla/thor/worker.h @@ -39,7 +39,22 @@ namespace valhalla { namespace thor { #ifdef ENABLE_GDAL -typedef struct GDALDriver* geotiff_driver_t; + +class geotiff_driver_t { + +public: + geotiff_driver_t(); + ~geotiff_driver_t(); + GDALDataset* CreateDataSet(const char* pszName, + int nXSize, + int nYSize, + int nBands, + GDALDataType eType, + CSLConstList papszOptions); + +private: + GDALDriver* geotiff_driver; +}; #endif #ifdef ENABLE_SERVICES From 2ba988681ce3f1ab4e816ff23efd5f0e8cad599f Mon Sep 17 00:00:00 2001 From: Christian Beiwinkel Date: Wed, 21 Feb 2024 14:16:28 +0100 Subject: [PATCH 11/27] forgot to add a macro there --- src/thor/worker.cc | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/thor/worker.cc b/src/thor/worker.cc index 5afeaebd0c..2a9178f8e1 100644 --- a/src/thor/worker.cc +++ b/src/thor/worker.cc @@ -78,7 +78,12 @@ thor_worker_t::thor_worker_t(const boost::property_tree::ptree& config, time_distance_bss_matrix_(config.get_child("thor")), isochrone_gen(config.get_child("thor")), reader(graph_reader ? graph_reader : std::make_shared(config.get_child("mjolnir"))), - matcher_factory(config, reader), controller{}, geotiff_driver(geotiff_driver_t{}) { + matcher_factory(config, reader), controller{} +#ifdef ENABLE_GDAL + , + geotiff_driver(geotiff_driver_t{}) +#endif +{ // Select the matrix algorithm based on the conf file (defaults to // select_optimal if not present) From 657aa59feda8b6e7b72787c1301e11ba5778a397 Mon Sep 17 00:00:00 2001 From: Christian Beiwinkel Date: Wed, 21 Feb 2024 14:19:58 +0100 Subject: [PATCH 12/27] tidy --- src/proto_conversions.cc | 10 +++++----- src/tyr/isochrone_serializer.cc | 5 +++-- test/isochrone.cc | 10 +++++----- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/proto_conversions.cc b/src/proto_conversions.cc index f4172b5b0c..32ba1a367b 100644 --- a/src/proto_conversions.cc +++ b/src/proto_conversions.cc @@ -10,7 +10,7 @@ const std::string& MatrixAlgoToString(const valhalla::Matrix::Algorithm algo) { static const std::unordered_map algos{ {valhalla::Matrix::CostMatrix, "costmatrix"}, {valhalla::Matrix::TimeDistanceMatrix, "timedistancematrix"}, - {valhalla::Matrix::TimeDistanceBSSMatrix, "timedistancbssematrix"}, + {valhalla::Matrix::TimeDistanceMatrix, "timedistancbssematrix"}, }; auto i = algos.find(algo); return i == algos.cend() ? empty_str : i->second; @@ -268,8 +268,8 @@ const std::string& ShapeMatch_Enum_Name(const ShapeMatch match) { bool Options_Format_Enum_Parse(const std::string& format, Options::Format* f) { static const std::unordered_map formats{ - {"json", Options::json}, {"gpx", Options::gpx}, {"osrm", Options::osrm}, - {"pbf", Options::pbf}, {"geotiff", Options::geotiff}, + {"json", Options::json}, {"gpx", Options::gpx}, {"osrm", Options::osrm}, + {"pbf", Options::pbf}, {"geotiff", gettid}, }; auto i = formats.find(format); if (i == formats.cend()) @@ -280,8 +280,8 @@ bool Options_Format_Enum_Parse(const std::string& format, Options::Format* f) { const std::string& Options_Format_Enum_Name(const Options::Format match) { static const std::unordered_map formats{ - {Options::json, "json"}, {Options::gpx, "gpx"}, {Options::osrm, "osrm"}, - {Options::pbf, "pbf"}, {Options::geotiff, "geotiff"}, + {Options::json, "json"}, {Options::gpx, "gpx"}, {Options::osrm, "osrm"}, + {Options::pbf, "pbf"}, {gettid, "geotiff"}, }; auto i = formats.find(match); return i == formats.cend() ? empty_str : i->second; diff --git a/src/tyr/isochrone_serializer.cc b/src/tyr/isochrone_serializer.cc index a6dbcc6c1d..cbe86a2a7e 100644 --- a/src/tyr/isochrone_serializer.cc +++ b/src/tyr/isochrone_serializer.cc @@ -309,7 +309,7 @@ std::string serializeIsochronePbf(Api& request, std::vector& intervals, const contours_t& contours) { // construct pbf output - Isochrone& isochrone = *request.mutable_isochrone(); + valhalla::thor::Isochrone& isochrone = *request.mutable_isochrone(); // construct contours for (size_t isoline_index = 0; isoline_index < contours.size(); ++isoline_index) { @@ -317,7 +317,8 @@ std::string serializeIsochronePbf(Api& request, const auto& interval = intervals[isoline_index]; auto* interval_pbf = isochrone.mutable_intervals()->Add(); - interval_pbf->set_metric(std::get<2>(interval) == "time" ? Isochrone::time : Isochrone::distance); + interval_pbf->set_metric(std::get<2>(interval) == "time" ? valhalla::thor::Isochrone::time + : valhalla::thor::Isochrone::distance); interval_pbf->set_metric_value(std::get<1>(interval)); diff --git a/test/isochrone.cc b/test/isochrone.cc index 68afc70ee4..ceb4eb027e 100644 --- a/test/isochrone.cc +++ b/test/isochrone.cc @@ -180,7 +180,7 @@ std::vector polygon_from_geojson(const std::string& geojson) { return {}; } -TEST(Isochrones, Basic) { +TEST(Basic) { // Test setup loki_worker_t loki_worker(cfg); thor_worker_t thor_worker(cfg); @@ -229,7 +229,7 @@ TEST(Isochrones, Basic) { } } -TEST(Isochrones, OriginEdge) { +TEST(OriginEdge) { const std::string ascii_map = R"( a-b-c )"; @@ -259,7 +259,7 @@ TEST(Isochrones, OriginEdge) { EXPECT_EQ(within(WaypointToBoostPoint("c"), polygon), false); } -TEST(Isochrones, LongEdge) { +TEST(LongEdge) { const std::string ascii_map = R"( c----d / @@ -317,7 +317,7 @@ class IsochroneTest : public thor::Isochrone { } }; -TEST(Isochrones, test_clear_reserved_memory) { +TEST(test_clear_reserved_memory) { boost::property_tree::ptree config; config.put("clear_reserved_memory", true); @@ -325,7 +325,7 @@ TEST(Isochrones, test_clear_reserved_memory) { isochrone.Clear(); } -TEST(Isochrones, test_max_reserved_labels_count) { +TEST(test_max_reserved_labels_count) { boost::property_tree::ptree config; config.put("max_reserved_labels_count_dijkstras", 10); From 77424eaf266edff6054eca258abcf2efca54e156 Mon Sep 17 00:00:00 2001 From: Christian Beiwinkel Date: Wed, 21 Feb 2024 14:20:47 +0100 Subject: [PATCH 13/27] format --- src/thor/worker.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/thor/worker.cc b/src/thor/worker.cc index 2a9178f8e1..989eacf4b5 100644 --- a/src/thor/worker.cc +++ b/src/thor/worker.cc @@ -78,10 +78,10 @@ thor_worker_t::thor_worker_t(const boost::property_tree::ptree& config, time_distance_bss_matrix_(config.get_child("thor")), isochrone_gen(config.get_child("thor")), reader(graph_reader ? graph_reader : std::make_shared(config.get_child("mjolnir"))), - matcher_factory(config, reader), controller{} + matcher_factory(config, reader), controller { +} #ifdef ENABLE_GDAL - , - geotiff_driver(geotiff_driver_t{}) +, geotiff_driver(geotiff_driver_t{}) #endif { From b2fa4cce0cb89bef4d3513c648ca3010dd3b591f Mon Sep 17 00:00:00 2001 From: Christian Beiwinkel Date: Wed, 21 Feb 2024 14:29:20 +0100 Subject: [PATCH 14/27] Revert "tidy" This reverts commit 657aa59feda8b6e7b72787c1301e11ba5778a397. --- src/proto_conversions.cc | 10 +++++----- src/tyr/isochrone_serializer.cc | 5 ++--- test/isochrone.cc | 10 +++++----- 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/src/proto_conversions.cc b/src/proto_conversions.cc index 32ba1a367b..f4172b5b0c 100644 --- a/src/proto_conversions.cc +++ b/src/proto_conversions.cc @@ -10,7 +10,7 @@ const std::string& MatrixAlgoToString(const valhalla::Matrix::Algorithm algo) { static const std::unordered_map algos{ {valhalla::Matrix::CostMatrix, "costmatrix"}, {valhalla::Matrix::TimeDistanceMatrix, "timedistancematrix"}, - {valhalla::Matrix::TimeDistanceMatrix, "timedistancbssematrix"}, + {valhalla::Matrix::TimeDistanceBSSMatrix, "timedistancbssematrix"}, }; auto i = algos.find(algo); return i == algos.cend() ? empty_str : i->second; @@ -268,8 +268,8 @@ const std::string& ShapeMatch_Enum_Name(const ShapeMatch match) { bool Options_Format_Enum_Parse(const std::string& format, Options::Format* f) { static const std::unordered_map formats{ - {"json", Options::json}, {"gpx", Options::gpx}, {"osrm", Options::osrm}, - {"pbf", Options::pbf}, {"geotiff", gettid}, + {"json", Options::json}, {"gpx", Options::gpx}, {"osrm", Options::osrm}, + {"pbf", Options::pbf}, {"geotiff", Options::geotiff}, }; auto i = formats.find(format); if (i == formats.cend()) @@ -280,8 +280,8 @@ bool Options_Format_Enum_Parse(const std::string& format, Options::Format* f) { const std::string& Options_Format_Enum_Name(const Options::Format match) { static const std::unordered_map formats{ - {Options::json, "json"}, {Options::gpx, "gpx"}, {Options::osrm, "osrm"}, - {Options::pbf, "pbf"}, {gettid, "geotiff"}, + {Options::json, "json"}, {Options::gpx, "gpx"}, {Options::osrm, "osrm"}, + {Options::pbf, "pbf"}, {Options::geotiff, "geotiff"}, }; auto i = formats.find(match); return i == formats.cend() ? empty_str : i->second; diff --git a/src/tyr/isochrone_serializer.cc b/src/tyr/isochrone_serializer.cc index cbe86a2a7e..a6dbcc6c1d 100644 --- a/src/tyr/isochrone_serializer.cc +++ b/src/tyr/isochrone_serializer.cc @@ -309,7 +309,7 @@ std::string serializeIsochronePbf(Api& request, std::vector& intervals, const contours_t& contours) { // construct pbf output - valhalla::thor::Isochrone& isochrone = *request.mutable_isochrone(); + Isochrone& isochrone = *request.mutable_isochrone(); // construct contours for (size_t isoline_index = 0; isoline_index < contours.size(); ++isoline_index) { @@ -317,8 +317,7 @@ std::string serializeIsochronePbf(Api& request, const auto& interval = intervals[isoline_index]; auto* interval_pbf = isochrone.mutable_intervals()->Add(); - interval_pbf->set_metric(std::get<2>(interval) == "time" ? valhalla::thor::Isochrone::time - : valhalla::thor::Isochrone::distance); + interval_pbf->set_metric(std::get<2>(interval) == "time" ? Isochrone::time : Isochrone::distance); interval_pbf->set_metric_value(std::get<1>(interval)); diff --git a/test/isochrone.cc b/test/isochrone.cc index ceb4eb027e..68afc70ee4 100644 --- a/test/isochrone.cc +++ b/test/isochrone.cc @@ -180,7 +180,7 @@ std::vector polygon_from_geojson(const std::string& geojson) { return {}; } -TEST(Basic) { +TEST(Isochrones, Basic) { // Test setup loki_worker_t loki_worker(cfg); thor_worker_t thor_worker(cfg); @@ -229,7 +229,7 @@ TEST(Basic) { } } -TEST(OriginEdge) { +TEST(Isochrones, OriginEdge) { const std::string ascii_map = R"( a-b-c )"; @@ -259,7 +259,7 @@ TEST(OriginEdge) { EXPECT_EQ(within(WaypointToBoostPoint("c"), polygon), false); } -TEST(LongEdge) { +TEST(Isochrones, LongEdge) { const std::string ascii_map = R"( c----d / @@ -317,7 +317,7 @@ class IsochroneTest : public thor::Isochrone { } }; -TEST(test_clear_reserved_memory) { +TEST(Isochrones, test_clear_reserved_memory) { boost::property_tree::ptree config; config.put("clear_reserved_memory", true); @@ -325,7 +325,7 @@ TEST(test_clear_reserved_memory) { isochrone.Clear(); } -TEST(test_max_reserved_labels_count) { +TEST(Isochrones, test_max_reserved_labels_count) { boost::property_tree::ptree config; config.put("max_reserved_labels_count_dijkstras", 10); From f15cb7e6c43ada0c0deaaf23080c2069f7c281f1 Mon Sep 17 00:00:00 2001 From: Christian Beiwinkel Date: Wed, 21 Feb 2024 17:08:24 +0100 Subject: [PATCH 15/27] address all comments; forget about having the geotiff driver inside thor, update tests, activate gdal flag on all ci builds --- .circleci/config.yml | 6 +-- .github/workflows/mingw-build.yml | 2 + .github/workflows/win.yml | 3 +- src/thor/CMakeLists.txt | 3 +- src/thor/isochrone_action.cc | 6 +-- src/thor/worker.cc | 31 +------------- src/tyr/isochrone_serializer.cc | 27 +++++++----- src/valhalla_run_isochrone.cc | 11 ----- test/isochrone.cc | 71 ++++++++++++++++++++++--------- valhalla/thor/worker.h | 26 ----------- valhalla/tyr/serializers.h | 8 +--- vcpkg.json | 1 + 12 files changed, 78 insertions(+), 117 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 8dd5cde962..68f9e8b288 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -35,7 +35,7 @@ jobs: cd build cmake .. -DCMAKE_BUILD_TYPE=Debug -DENABLE_COVERAGE=On -DCPACK_GENERATOR=DEB \ -DENABLE_COMPILER_WARNINGS=On -DENABLE_WERROR=Off -DCMAKE_EXPORT_COMPILE_COMMANDS=On \ - -DCMAKE_CXX_FLAGS="-fuse-ld=lld" -DLOGGING_LEVEL=INFO -DENABLE_PYTHON_BINDINGS=On + -DCMAKE_CXX_FLAGS="-fuse-ld=lld" -DLOGGING_LEVEL=INFO -DENABLE_PYTHON_BINDINGS=On -DENABLE_GDAL=On - run: python3 ./scripts/valhalla_build_config - run: make -C build -j8 - run: make -C build utrecht_tiles @@ -74,7 +74,7 @@ jobs: cd build cmake .. -DCMAKE_BUILD_TYPE=Release -DBUILD_SHARED_LIBS=On -DENABLE_PYTHON_BINDINGS=On -DLOGGING_LEVEL=TRACE \ -DCPACK_GENERATOR=DEB -DCPACK_PACKAGE_VERSION_SUFFIX="-0ubuntu1-$(lsb_release -sc)" -DENABLE_SANITIZERS=ON \ - -DENABLE_SINGLE_FILES_WERROR=Off + -DENABLE_SINGLE_FILES_WERROR=Off -DENABLE_GDAL=On - run: make -C build -j8 - run: make -C build utrecht_tiles - run: make -C build -j8 tests @@ -106,7 +106,7 @@ jobs: mkdir build cd build cmake .. -DCMAKE_BUILD_TYPE=Release -DBUILD_SHARED_LIBS=On -DENABLE_PYTHON_BINDINGS=On -DCPACK_GENERATOR=DEB \ - -DCPACK_PACKAGE_VERSION_SUFFIX="-0ubuntu1-$(lsb_release -sc)" -DENABLE_SINGLE_FILES_WERROR=Off + -DCPACK_PACKAGE_VERSION_SUFFIX="-0ubuntu1-$(lsb_release -sc)" -DENABLE_SINGLE_FILES_WERROR=Off -DENABLE_GDAL=On - run: make -C build -j8 - run: make -C build utrecht_tiles - run: make -C build -j8 tests diff --git a/.github/workflows/mingw-build.yml b/.github/workflows/mingw-build.yml index 18cac6ffc7..e3dfd581ea 100644 --- a/.github/workflows/mingw-build.yml +++ b/.github/workflows/mingw-build.yml @@ -18,6 +18,7 @@ jobs: mingw64-boost \ mingw64-protobuf \ mingw64-geos \ + mingw64-gdal \ mingw64-python3 \ protobuf-compiler - uses: actions/checkout@v2 @@ -39,6 +40,7 @@ jobs: -DENABLE_PYTHON_BINDINGS=ON \ -DENABLE_CCACHE=OFF \ -DENABLE_TESTS=OFF \ + -DENABLE_GDAL=ON \ -DLOGGING_LEVEL=DEBUG \ -DBoost_PROGRAM_OPTIONS_LIBRARY=/usr/x86_64-w64-mingw32/sys-root/mingw/lib/libboost_program_options-mt-x64.dll.a \ -DPYTHON_EXECUTABLE=/usr/x86_64-w64-mingw32/bin/python3 \ diff --git a/.github/workflows/win.yml b/.github/workflows/win.yml index 80a0abcc14..e3755c8df1 100644 --- a/.github/workflows/win.yml +++ b/.github/workflows/win.yml @@ -106,7 +106,8 @@ jobs: -DENABLE_TESTS=OFF \ -DENABLE_CCACHE=OFF \ -DENABLE_SERVICES=OFF \ - -DPREFER_EXTERNAL_DEPS=ON + -DPREFER_EXTERNAL_DEPS=ON \ + -DENABLE_GDAL=ON - if: ${{ steps.vcpkg-restore.outputs.cache-hit != 'true' }} name: Save vcpkg packages (if cache miss) diff --git a/src/thor/CMakeLists.txt b/src/thor/CMakeLists.txt index 2d0bb1ec97..d486d202b2 100644 --- a/src/thor/CMakeLists.txt +++ b/src/thor/CMakeLists.txt @@ -56,5 +56,4 @@ valhalla_module(NAME thor valhalla::meili ${valhalla_protobuf_targets} Boost::boost - ${libprime_server_targets} - ${gdal_target}) + ${libprime_server_targets}) \ No newline at end of file diff --git a/src/thor/isochrone_action.cc b/src/thor/isochrone_action.cc index f32cde3bce..6faf094e06 100644 --- a/src/thor/isochrone_action.cc +++ b/src/thor/isochrone_action.cc @@ -43,12 +43,8 @@ std::string thor_worker_t::isochrones(Api& request) { if (options.action() == Options_Action_expansion) return ""; -// make the final output (pbf or json) -#ifdef ENABLE_GDAL - std::string ret = tyr::serializeIsochrones(request, intervals, grid, geotiff_driver); -#else + // make the final output (pbf, json or geotiff) std::string ret = tyr::serializeIsochrones(request, intervals, grid); -#endif return ret; } diff --git a/src/thor/worker.cc b/src/thor/worker.cc index 989eacf4b5..afe1e8373e 100644 --- a/src/thor/worker.cc +++ b/src/thor/worker.cc @@ -15,10 +15,6 @@ #include -#ifdef ENABLE_GDAL -#include -#endif - using namespace valhalla; using namespace valhalla::tyr; using namespace valhalla::midgard; @@ -78,12 +74,7 @@ thor_worker_t::thor_worker_t(const boost::property_tree::ptree& config, time_distance_bss_matrix_(config.get_child("thor")), isochrone_gen(config.get_child("thor")), reader(graph_reader ? graph_reader : std::make_shared(config.get_child("mjolnir"))), - matcher_factory(config, reader), controller { -} -#ifdef ENABLE_GDAL -, geotiff_driver(geotiff_driver_t{}) -#endif -{ + matcher_factory(config, reader), controller{} { // Select the matrix algorithm based on the conf file (defaults to // select_optimal if not present) @@ -339,25 +330,5 @@ void thor_worker_t::set_interrupt(const std::function* interrupt_functio interrupt = interrupt_function; reader->SetInterrupt(interrupt); } - -#ifdef ENABLE_GDAL -geotiff_driver_t::geotiff_driver_t() { - auto driver_manager = GetGDALDriverManager(); - this->geotiff_driver = driver_manager->GetDriverByName("GTiff"); -} - -geotiff_driver_t::~geotiff_driver_t() { - // GDALDestroyDriverManager(); -} - -GDALDataset* geotiff_driver_t::CreateDataSet(const char* pszName, - int nXSize, - int nYSize, - int nBands, - GDALDataType eType, - CSLConstList papszOptions) { - return this->geotiff_driver->Create(pszName, nXSize, nYSize, nBands, eType, papszOptions); -} -#endif } // namespace thor } // namespace valhalla diff --git a/src/tyr/isochrone_serializer.cc b/src/tyr/isochrone_serializer.cc index a6dbcc6c1d..b9b07cc38a 100644 --- a/src/tyr/isochrone_serializer.cc +++ b/src/tyr/isochrone_serializer.cc @@ -151,8 +151,8 @@ std::string GenerateTmpFName() { std::string serializeGeoTIFF(Api& request, std::vector intervals, - std::shared_ptr> isogrid, - valhalla::thor::geotiff_driver_t geotiff_driver) { + std::shared_ptr> isogrid) { + // time, distance std::vector metrics{false, false}; for (auto& contour : request.options().contours()) { @@ -171,8 +171,11 @@ std::string serializeGeoTIFF(Api& request, char** geotiff_options = NULL; geotiff_options = CSLSetNameValue(geotiff_options, "COMPRESS", "PACKBITS"); + // TODO: check if there's an easy way to only instantiate the driver once + auto driver_manager = GetGDALDriverManager(); + auto geotiff_driver = driver_manager->GetDriverByName("GTiff"); auto geotiff_dataset = - geotiff_driver.CreateDataSet(name.c_str(), ext_x, ext_y, nbands, GDT_UInt16, geotiff_options); + geotiff_driver->Create(name.c_str(), ext_x, ext_y, nbands, GDT_UInt16, geotiff_options); OGRSpatialReference spatial_ref; spatial_ref.SetWellKnownGeogCS("EPSG:4326"); @@ -186,6 +189,13 @@ std::string serializeGeoTIFF(Api& request, geotiff_dataset->SetGeoTransform(geo_transform); geotiff_dataset->SetSpatialRef(const_cast(&spatial_ref)); + // geotiff doesn't support different nodata values for each band, so we just get the larger one + float max_time = isogrid->MaxValue(0); + float max_distance = isogrid->MaxValue(1); + + // they will be scaled to seconds/10 meters + float no_data_value = max_time > max_distance ? max_time * 60 : max_distance * 100; + for (size_t metric_idx = 0; metric_idx < metrics.size(); ++metric_idx) { if (!metrics[metric_idx]) continue; // only create bands for requested metrics @@ -201,6 +211,7 @@ std::string serializeGeoTIFF(Api& request, } } auto band = geotiff_dataset->GetRasterBand(nbands == 2 ? (metric_idx + 1) : 1); + band->SetNoDataValue(static_cast(no_data_value)); band->SetDescription(metric_idx == 0 ? "Time (seconds)" : "Distance (10m)"); CPLErr err = band->RasterIO(GF_Write, 0, 0, ext_x, ext_y, data, ext_x, ext_y, GDT_UInt16, 0, 0); @@ -214,6 +225,7 @@ std::string serializeGeoTIFF(Api& request, vsi_l_offset bufferlength; GByte* bytes = VSIGetMemFileBuffer(name.c_str(), &bufferlength, TRUE); + // TODO: there's gotta be way to do this without copying std::string data(reinterpret_cast(bytes), bufferlength); return data; @@ -351,16 +363,9 @@ std::string serializeIsochronePbf(Api& request, namespace valhalla { namespace tyr { -#ifdef ENABLE_GDAL -std::string serializeIsochrones(Api& request, - std::vector::contour_interval_t>& intervals, - const std::shared_ptr>& isogrid, - valhalla::thor::geotiff_driver_t geotiff_driver) { -#else std::string serializeIsochrones(Api& request, std::vector::contour_interval_t>& intervals, const std::shared_ptr>& isogrid) { -#endif // only generate if json or pbf output is requested contours_t contours; @@ -382,7 +387,7 @@ std::string serializeIsochrones(Api& request, #ifdef ENABLE_GDAL case Options_Format_geotiff: - return serializeGeoTIFF(request, intervals, isogrid, geotiff_driver); + return serializeGeoTIFF(request, intervals, isogrid); #endif default: throw; diff --git a/src/valhalla_run_isochrone.cc b/src/valhalla_run_isochrone.cc index bd94dbf76b..a96394caf0 100644 --- a/src/valhalla_run_isochrone.cc +++ b/src/valhalla_run_isochrone.cc @@ -204,18 +204,7 @@ int main(int argc, char* argv[]) { msecs = std::chrono::duration_cast(t3 - t2).count(); LOG_INFO("Contour Generation took " + std::to_string(msecs) + " ms"); -#ifdef ENABLE_GDAL - auto driver_manager = GetGDALDriverManager(); - geotiff_driver_t driver = driver_manager->GetDriverByName("GTiff"); - // Serialize to GeoTIFF - std::string res = valhalla::tyr::serializeIsochrones(request, contour_times, isogrid, driver); - if (request.options().format() == Options_Format_geotiff) { - GDALDestroyDriverManager(); - } -#else - // Serialize to GeoJSON std::string res = valhalla::tyr::serializeIsochrones(request, contour_times, isogrid); -#endif auto t4 = std::chrono::high_resolution_clock::now(); msecs = std::chrono::duration_cast(t4 - t3).count(); LOG_INFO("Isochrone serialization took " + std::to_string(msecs) + " ms"); diff --git a/test/isochrone.cc b/test/isochrone.cc index 68afc70ee4..0259869745 100644 --- a/test/isochrone.cc +++ b/test/isochrone.cc @@ -334,6 +334,26 @@ TEST(Isochrones, test_max_reserved_labels_count) { } #ifdef ENABLE_GDAL + +void check_raster_edges(size_t x, size_t y, uint16_t* data) { + + // make sure the outer "edges" are not 0 + for (size_t i = 0; i < y; ++i) { + // if not in first or last row + if (i != 0 || i != y - 1) { + // just check first and last element in the row + ASSERT_NE(data[i * y], 0); + ASSERT_NE(data[i * y + x], 0); + continue; + } + + // else check the whole row + for (size_t j = 0; j < x; ++j) { + ASSERT_NE(data[i * y + j], 0); + } + } +} + TEST(Isochrones, test_geotiff_output_distance) { GDALRegister_GTiff(); loki_worker_t loki_worker(cfg); @@ -354,23 +374,26 @@ TEST(Isochrones, test_geotiff_output_distance) { int x = geotiff_dataset->GetRasterXSize(); int y = geotiff_dataset->GetRasterYSize(); GDALRasterBand* band = geotiff_dataset->GetRasterBand(1); - uint16_t dataArray[x * y]; - CPLErr err = band->RasterIO(GF_Read, 0, 0, x, y, dataArray, x, y, GDT_UInt16, 0, 0); + uint16_t data_array[x * y]; + CPLErr err = band->RasterIO(GF_Read, 0, 0, x, y, data_array, x, y, GDT_UInt16, 0, 0); double min_max[2]; band->ComputeRasterMinMax(0, min_max); ASSERT_EQ(err, CE_None); - ASSERT_EQ(x, 144); - ASSERT_EQ(y, 97); + ASSERT_NE(x, 0); + ASSERT_NE(y, 0); ASSERT_EQ(static_cast(min_max[0]), 0); - ASSERT_EQ(static_cast(min_max[1]), 1100); + ASSERT_EQ(static_cast(min_max[1]), 1099); + ASSERT_EQ(band->GetNoDataValue(), 1100); size_t array_size = x * y; + check_raster_edges(x, y, data_array); + // make sure there are some grid cells whose metric value is neither 0 nor the max bool no_intermediate_values = true; for (size_t i = 0; i < array_size; ++i) { - if (dataArray[i] > 0 && dataArray[i] < min_max[1]) + if (data_array[i] > 0 && data_array[i] < min_max[1]) no_intermediate_values = false; } ASSERT_EQ(no_intermediate_values, false); @@ -397,23 +420,26 @@ TEST(Isochrones, test_geotiff_output_time) { int x = geotiff_dataset->GetRasterXSize(); int y = geotiff_dataset->GetRasterYSize(); GDALRasterBand* band = geotiff_dataset->GetRasterBand(1); - uint16_t dataArray[x * y]; - CPLErr err = band->RasterIO(GF_Read, 0, 0, x, y, dataArray, x, y, GDT_UInt16, 0, 0); + uint16_t data_array[x * y]; + CPLErr err = band->RasterIO(GF_Read, 0, 0, x, y, data_array, x, y, GDT_UInt16, 0, 0); double min_max[2]; band->ComputeRasterMinMax(0, min_max); ASSERT_EQ(err, CE_None); - ASSERT_EQ(x, 145); - ASSERT_EQ(y, 97); + ASSERT_GT(x, 0); + ASSERT_GT(y, 0); ASSERT_EQ(static_cast(min_max[0]), 0); - ASSERT_EQ(static_cast(min_max[1]), 660); + ASSERT_EQ(static_cast(min_max[1]), 659); // nodata value is excluded from min/max + ASSERT_EQ(band->GetNoDataValue(), 660); size_t array_size = x * y; + check_raster_edges(x, y, data_array); + // make sure there are some grid cells whose metric value is neither 0 nor the max bool no_intermediate_values = true; for (size_t i = 0; i < array_size; ++i) { - if (dataArray[i] > 0 && dataArray[i] < min_max[1]) + if (data_array[i] > 0 && data_array[i] < min_max[1]) no_intermediate_values = false; } ASSERT_EQ(no_intermediate_values, false); @@ -442,27 +468,30 @@ TEST(Isochrones, test_geotiff_output_time_distance) { int y = geotiff_dataset->GetRasterYSize(); // time, distance - std::array expected_max{660, 1200}; + std::array expected_max{660, 1199}; - for (int i = 1; i <= 2; ++i) { - GDALRasterBand* band = geotiff_dataset->GetRasterBand(i); - uint16_t dataArray[x * y]; - CPLErr err = band->RasterIO(GF_Read, 0, 0, x, y, dataArray, x, y, GDT_UInt16, 0, 0); + for (int b = 1; b <= 2; ++b) { + GDALRasterBand* band = geotiff_dataset->GetRasterBand(b); + uint16_t data_array[x * y]; + CPLErr err = band->RasterIO(GF_Read, 0, 0, x, y, data_array, x, y, GDT_UInt16, 0, 0); double min_max[2]; band->ComputeRasterMinMax(0, min_max); ASSERT_EQ(err, CE_None); - ASSERT_EQ(x, 147); - ASSERT_EQ(y, 97); + ASSERT_NE(x, 0); + ASSERT_NE(y, 0); ASSERT_EQ(static_cast(min_max[0]), 0); - ASSERT_EQ(static_cast(min_max[1]), expected_max[i - 1]); + ASSERT_EQ(static_cast(min_max[1]), expected_max[b - 1]); + ASSERT_EQ(band->GetNoDataValue(), 1200); // set for the whole dataset size_t array_size = x * y; + check_raster_edges(x, y, data_array); + // make sure there are some grid cells whose metric value is neither 0 nor the max bool no_intermediate_values = true; for (size_t j = 0; j < array_size; ++j) { - if (dataArray[j] > 0 && dataArray[j] < min_max[1]) + if (data_array[j] > 0 && data_array[j] < min_max[1]) no_intermediate_values = false; } ASSERT_EQ(no_intermediate_values, false); diff --git a/valhalla/thor/worker.h b/valhalla/thor/worker.h index 3674b344a4..7334cdcac2 100644 --- a/valhalla/thor/worker.h +++ b/valhalla/thor/worker.h @@ -6,9 +6,6 @@ #include #include -#ifdef ENABLE_GDAL -#include -#endif #include #include @@ -38,25 +35,6 @@ namespace valhalla { namespace thor { -#ifdef ENABLE_GDAL - -class geotiff_driver_t { - -public: - geotiff_driver_t(); - ~geotiff_driver_t(); - GDALDataset* CreateDataSet(const char* pszName, - int nXSize, - int nYSize, - int nBands, - GDALDataType eType, - CSLConstList papszOptions); - -private: - GDALDriver* geotiff_driver; -}; -#endif - #ifdef ENABLE_SERVICES void run_service(const boost::property_tree::ptree& config); #endif @@ -162,10 +140,6 @@ class thor_worker_t : public service_worker_t { std::string service_name() const override { return "thor"; } - -#ifdef ENABLE_GDAL - geotiff_driver_t geotiff_driver; -#endif }; } // namespace thor diff --git a/valhalla/tyr/serializers.h b/valhalla/tyr/serializers.h index 2958048457..66c7ffcf05 100644 --- a/valhalla/tyr/serializers.h +++ b/valhalla/tyr/serializers.h @@ -45,13 +45,7 @@ std::string serializeMatrix(Api& request); */ std::string serializeIsochrones(Api& request, std::vector::contour_interval_t>& intervals, - const std::shared_ptr>& isogrid -#ifdef ENABLE_GDAL - , - valhalla::thor::geotiff_driver_t geotiff_driver); -#else -); -#endif + const std::shared_ptr>& isogrid); /** * Turn heights and ranges into a height response * diff --git a/vcpkg.json b/vcpkg.json index cafb7d935f..fbb848fe00 100644 --- a/vcpkg.json +++ b/vcpkg.json @@ -17,6 +17,7 @@ "name": "dirent", "platform": "windows" }, + "gdal", "geos", "libspatialite", "pkgconf", From b1014ba872ad0a94b618f56bb18df34eebc2156f Mon Sep 17 00:00:00 2001 From: Christian Beiwinkel Date: Wed, 21 Feb 2024 17:58:30 +0100 Subject: [PATCH 16/27] missed some things --- src/loki/worker.cc | 6 ------ src/worker.cc | 5 +++++ valhalla/tyr/serializers.h | 1 - 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/loki/worker.cc b/src/loki/worker.cc index eac29e5331..0a927f21e8 100644 --- a/src/loki/worker.cc +++ b/src/loki/worker.cc @@ -357,12 +357,6 @@ loki_worker_t::work(const std::list& job, result.messages.emplace_back(request.SerializeAsString()); break; case Options::isochrone: -// return early if geotiff was requested but GDAL isn't enabled -#ifndef ENABLE_GDAL - if (options.format() == Options_Format_geotiff) { - throw valhalla_exception_t{504}; - } -#endif isochrones(request); result.messages.emplace_back(request.SerializeAsString()); break; diff --git a/src/worker.cc b/src/worker.cc index c069bd23a3..31795a90c7 100644 --- a/src/worker.cc +++ b/src/worker.cc @@ -697,6 +697,11 @@ void from_json(rapidjson::Document& doc, Options::Action action, Api& api) { options.clear_jsonp(); } } +#ifndef ENABLE_GDAL + else if (options.format() == Options::geotiff) { + throw valhalla_exception_t{504}; + } +#endif auto units = rapidjson::get_optional(doc, "/units"); if (units && ((*units == "miles") || (*units == "mi"))) { diff --git a/valhalla/tyr/serializers.h b/valhalla/tyr/serializers.h index 66c7ffcf05..a2095bb8a4 100644 --- a/valhalla/tyr/serializers.h +++ b/valhalla/tyr/serializers.h @@ -8,7 +8,6 @@ #include #ifdef ENABLE_GDAL -#include "valhalla/thor/worker.h" #include #endif From 32c24053dd4f2b6e4d2105b81e60ad61535fdcec Mon Sep 17 00:00:00 2001 From: Christian Beiwinkel Date: Wed, 21 Feb 2024 18:30:03 +0100 Subject: [PATCH 17/27] try to fix ci fails --- src/tyr/isochrone_serializer.cc | 2 +- src/valhalla_run_isochrone.cc | 2 +- src/valhalla_service.cc | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/tyr/isochrone_serializer.cc b/src/tyr/isochrone_serializer.cc index b9b07cc38a..7ffa3bf101 100644 --- a/src/tyr/isochrone_serializer.cc +++ b/src/tyr/isochrone_serializer.cc @@ -10,7 +10,7 @@ #include #ifdef ENABLE_GDAL -#include +#include #endif using namespace valhalla::baldr::json; diff --git a/src/valhalla_run_isochrone.cc b/src/valhalla_run_isochrone.cc index a96394caf0..e3db6dca6f 100644 --- a/src/valhalla_run_isochrone.cc +++ b/src/valhalla_run_isochrone.cc @@ -21,7 +21,7 @@ #include "worker.h" #ifdef ENABLE_GDAL -#include +#include #endif using namespace valhalla; diff --git a/src/valhalla_service.cc b/src/valhalla_service.cc index 4defc13195..89214213d4 100644 --- a/src/valhalla_service.cc +++ b/src/valhalla_service.cc @@ -18,7 +18,7 @@ using namespace prime_server; #endif #ifdef ENABLE_GDAL -#include +#include #endif #include "config.h" From 5ead0445d6b2cc2d20ab118e26ff823fb7a1cb74 Mon Sep 17 00:00:00 2001 From: Christian Beiwinkel Date: Wed, 21 Feb 2024 18:51:43 +0100 Subject: [PATCH 18/27] whoops forgot some --- test/isochrone.cc | 2 +- valhalla/tyr/serializers.h | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/test/isochrone.cc b/test/isochrone.cc index 0259869745..a93fc0ccd5 100644 --- a/test/isochrone.cc +++ b/test/isochrone.cc @@ -15,7 +15,7 @@ #include #ifdef ENABLE_GDAL -#include +#include #endif using point_type = boost::geometry::model::d2::point_xy; diff --git a/valhalla/tyr/serializers.h b/valhalla/tyr/serializers.h index a2095bb8a4..062d9dc985 100644 --- a/valhalla/tyr/serializers.h +++ b/valhalla/tyr/serializers.h @@ -7,10 +7,6 @@ #include #include -#ifdef ENABLE_GDAL -#include -#endif - #include #include #include From 55e19dcd52c9500822da6e0a100ac922d63030fb Mon Sep 17 00:00:00 2001 From: nilsnolde Date: Wed, 21 Feb 2024 23:29:08 +0100 Subject: [PATCH 19/27] try to prefer CONFIG mode if available and if not use cmake findgdal --- CMakeLists.txt | 31 +++++++++++++++++-------------- docs/docs/building.md | 2 +- src/tyr/isochrone_serializer.cc | 2 +- src/valhalla_run_isochrone.cc | 2 +- src/valhalla_service.cc | 2 +- test/isochrone.cc | 2 +- 6 files changed, 22 insertions(+), 19 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index d975993134..9de2d0ea96 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -97,8 +97,6 @@ include(ValhallaSourceGroups) # We use pkg-config for (almost) all dependencies: # - CMake Find* modules are versioned with CMake, not the packages, see protobuf > 21.12 # - it's more trivial to use custom installations via PKG_CONFIG_PATH env var -find_package(PkgConfig REQUIRED) -find_package(Threads REQUIRED) pkg_check_modules(ZLIB REQUIRED IMPORTED_TARGET zlib) pkg_check_modules(LZ4 REQUIRED IMPORTED_TARGET liblz4) @@ -114,12 +112,19 @@ if (ENABLE_HTTP OR ENABLE_DATA_TOOLS) endif() endif() +# prefer CONFIG mode over MODULE mode, which versions configuration on the package, not CMake +# NOTE: this is only supported for cmake >= 3.15, but shouldn't be a problem in real life +set(CMAKE_FIND_PACKAGE_PREFER_CONFIG ON) + # Boost has no .pc file.. find_package(Boost 1.71 REQUIRED) add_definitions(-DBOOST_NO_CXX11_SCOPED_ENUMS) add_definitions(-DBOOST_ALLOW_DEPRECATED_HEADERS) add_definitions(-DBOOST_BIND_GLOBAL_PLACEHOLDERS) +find_package(PkgConfig REQUIRED) +find_package(Threads REQUIRED) + # resolve vendored libraries set(date_include_dir ${VALHALLA_SOURCE_DIR}/third_party/date/include) set(rapidjson_include_dir ${CMAKE_SOURCE_DIR}/third_party/rapidjson/include) @@ -168,14 +173,10 @@ endif() # Protobuf is non-trivial to include via pkg-config, pkg_check_modules has no way to check # for protoc location in a platform agnostic manner -# prefer CONFIG mode over MODULE mode, which versions configuration on the package, not CMake -# NOTE: this is only supported for cmake >= 3.15, but shouldn't be a problem in real life -set(CMAKE_FIND_PACKAGE_PREFER_CONFIG ON) # newer protobuf versions require a compat bool set(protobuf_MODULE_COMPATIBLE ON CACHE BOOL "") find_package(Protobuf REQUIRED) # and turn it off again -set(CMAKE_FIND_PACKAGE_PREFER_CONFIG OFF) message(STATUS "Using protoc from ${Protobuf_PROTOC_EXECUTABLE}") message(STATUS "Using pbf headers from ${Protobuf_INCLUDE_DIRS}") message(STATUS "Using pbf libs from ${PROTOBUF_LIBRARIES}") @@ -190,6 +191,16 @@ else() message(FATAL_ERROR "Required target protobuf::libprotobuf-lite or protobuf::libprotobuf is not defined") endif() +# gdal +set(gdal_target "") +if (ENABLE_GDAL) + find_package(GDAL REQUIRED) + add_compile_definitions(ENABLE_GDAL) + set(gdal_target GDAL::GDAL) +endif() + +set(CMAKE_FIND_PACKAGE_PREFER_CONFIG OFF) + # libprime_server # see https://gitlab.kitware.com/cmake/cmake/-/issues/19467 set(libprime_server_targets "") @@ -204,14 +215,6 @@ if(ENABLE_SERVICES) endif() endif() -# gdal -set(gdal_target "") -if (ENABLE_GDAL) - pkg_check_modules(GDAL REQUIRED IMPORTED_TARGET gdal) - add_compile_definitions(ENABLE_GDAL) - set(gdal_target PkgConfig::GDAL) -endif() - ## Mjolnir and associated executables if(ENABLE_DATA_TOOLS) diff --git a/docs/docs/building.md b/docs/docs/building.md index ea8ea0c315..7296b815bc 100644 --- a/docs/docs/building.md +++ b/docs/docs/building.md @@ -139,7 +139,7 @@ When importing `libvalhalla` as a dependency in a project, it's important to kno To resolve `libvalhalla`'s linker/library paths/options, we recommend to use `pkg-config` or `pkg_check_modules` (in CMake). -Currently, `rapidjson`, `date` & `dirent` (Win only) headers are vendored in `third_party`. Consuming applications are encouraged to use `pkg-config` to resolve Valhalla and its dependencies which will automatically install those headers to `/path/to/include/valhalla/third_pary/{rapidjson, date, dirent.h}` and can be `#include`d appropriately. +Currently, `rapidjson`, `date` & `dirent` (Win only) headers are vendored in `third_party`. Consuming applications are encouraged to use `pkg-config` to resolve Valhalla and its dependencies which will automatically install those headers to `/path/to/include/valhalla/third_party/{rapidjson, date, dirent.h}` and can be `#include`d appropriately. ## Running Valhalla server on Unix diff --git a/src/tyr/isochrone_serializer.cc b/src/tyr/isochrone_serializer.cc index 7ffa3bf101..b9b07cc38a 100644 --- a/src/tyr/isochrone_serializer.cc +++ b/src/tyr/isochrone_serializer.cc @@ -10,7 +10,7 @@ #include #ifdef ENABLE_GDAL -#include +#include #endif using namespace valhalla::baldr::json; diff --git a/src/valhalla_run_isochrone.cc b/src/valhalla_run_isochrone.cc index e3db6dca6f..a96394caf0 100644 --- a/src/valhalla_run_isochrone.cc +++ b/src/valhalla_run_isochrone.cc @@ -21,7 +21,7 @@ #include "worker.h" #ifdef ENABLE_GDAL -#include +#include #endif using namespace valhalla; diff --git a/src/valhalla_service.cc b/src/valhalla_service.cc index 89214213d4..4defc13195 100644 --- a/src/valhalla_service.cc +++ b/src/valhalla_service.cc @@ -18,7 +18,7 @@ using namespace prime_server; #endif #ifdef ENABLE_GDAL -#include +#include #endif #include "config.h" diff --git a/test/isochrone.cc b/test/isochrone.cc index a93fc0ccd5..0259869745 100644 --- a/test/isochrone.cc +++ b/test/isochrone.cc @@ -15,7 +15,7 @@ #include #ifdef ENABLE_GDAL -#include +#include #endif using point_type = boost::geometry::model::d2::point_xy; From 85bf2182528cc29847794881626366019110dc6a Mon Sep 17 00:00:00 2001 From: Christian Beiwinkel Date: Thu, 22 Feb 2024 09:00:12 +0100 Subject: [PATCH 20/27] shutting up msvc --- src/tyr/isochrone_serializer.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/tyr/isochrone_serializer.cc b/src/tyr/isochrone_serializer.cc index b9b07cc38a..48ebc75e2c 100644 --- a/src/tyr/isochrone_serializer.cc +++ b/src/tyr/isochrone_serializer.cc @@ -199,7 +199,7 @@ std::string serializeGeoTIFF(Api& request, for (size_t metric_idx = 0; metric_idx < metrics.size(); ++metric_idx) { if (!metrics[metric_idx]) continue; // only create bands for requested metrics - uint16_t data[ext_x * ext_y]; + uint16_t* data = new uint16_t[ext_x * ext_y]; // seconds or 10 meter steps float scale_factor = metric_idx == 0 ? 60 : 100; @@ -216,6 +216,8 @@ std::string serializeGeoTIFF(Api& request, CPLErr err = band->RasterIO(GF_Write, 0, 0, ext_x, ext_y, data, ext_x, ext_y, GDT_UInt16, 0, 0); + delete[] data; + if (err != CE_None) { throw valhalla_exception_t{599, "Unknown error when writing GeoTIFF."}; } From cf60652af92226f7d36fb6558cdfd7c71e635ec1 Mon Sep 17 00:00:00 2001 From: Christian Beiwinkel Date: Thu, 22 Feb 2024 10:22:45 +0100 Subject: [PATCH 21/27] tidy --- src/tyr/isochrone_serializer.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tyr/isochrone_serializer.cc b/src/tyr/isochrone_serializer.cc index 48ebc75e2c..e22f7d581d 100644 --- a/src/tyr/isochrone_serializer.cc +++ b/src/tyr/isochrone_serializer.cc @@ -150,8 +150,8 @@ std::string GenerateTmpFName() { } std::string serializeGeoTIFF(Api& request, - std::vector intervals, - std::shared_ptr> isogrid) { + const std::vector intervals, + const std::shared_ptr>& isogrid) { // time, distance std::vector metrics{false, false}; From 3937387557182bba0d92c9e959423bdd5ad81594 Mon Sep 17 00:00:00 2001 From: Christian Beiwinkel Date: Thu, 22 Feb 2024 10:41:39 +0100 Subject: [PATCH 22/27] tidy again --- src/tyr/isochrone_serializer.cc | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/tyr/isochrone_serializer.cc b/src/tyr/isochrone_serializer.cc index e22f7d581d..01e801a094 100644 --- a/src/tyr/isochrone_serializer.cc +++ b/src/tyr/isochrone_serializer.cc @@ -149,9 +149,7 @@ std::string GenerateTmpFName() { return ss.str(); } -std::string serializeGeoTIFF(Api& request, - const std::vector intervals, - const std::shared_ptr>& isogrid) { +std::string serializeGeoTIFF(Api& request, const std::shared_ptr>& isogrid) { // time, distance std::vector metrics{false, false}; @@ -389,7 +387,7 @@ std::string serializeIsochrones(Api& request, #ifdef ENABLE_GDAL case Options_Format_geotiff: - return serializeGeoTIFF(request, intervals, isogrid); + return serializeGeoTIFF(request, isogrid); #endif default: throw; From f767375420d7f4f78f2f6926adf8f3e26dda8919 Mon Sep 17 00:00:00 2001 From: Christian Beiwinkel Date: Thu, 22 Feb 2024 10:54:09 +0100 Subject: [PATCH 23/27] bump cmake to 3.14 --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 9de2d0ea96..b1f238201a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -7,7 +7,7 @@ # This is secondary build configuration provided for convenient development # on Windows and using CMake-enabled IDEs. # -cmake_minimum_required(VERSION 3.12 FATAL_ERROR) +cmake_minimum_required(VERSION 3.14 FATAL_ERROR) project(valhalla LANGUAGES CXX C) include(FindPkgConfig) From 7edd0383433e16fe031bce032289d53d083c6005 Mon Sep 17 00:00:00 2001 From: Christian Beiwinkel Date: Thu, 22 Feb 2024 12:01:48 +0100 Subject: [PATCH 24/27] includes for boost missing in some tests --- test/gurka/test_barrier_uturns.cc | 1 + test/gurka/test_bidir_search.cc | 1 + 2 files changed, 2 insertions(+) diff --git a/test/gurka/test_barrier_uturns.cc b/test/gurka/test_barrier_uturns.cc index 247e4563a4..56d140b949 100644 --- a/test/gurka/test_barrier_uturns.cc +++ b/test/gurka/test_barrier_uturns.cc @@ -1,4 +1,5 @@ #include "gurka.h" +#include #include using namespace valhalla; diff --git a/test/gurka/test_bidir_search.cc b/test/gurka/test_bidir_search.cc index 55ca43e2eb..48016dc587 100644 --- a/test/gurka/test_bidir_search.cc +++ b/test/gurka/test_bidir_search.cc @@ -1,5 +1,6 @@ #include "gurka.h" #include "test.h" +#include #include From 8cba25cf1bfbb5212566135b027e6e99abfd1b06 Mon Sep 17 00:00:00 2001 From: Christian <58629404+chrstnbwnkl@users.noreply.github.com> Date: Thu, 22 Feb 2024 12:55:38 +0100 Subject: [PATCH 25/27] Update src/worker.cc Co-authored-by: Nils --- src/worker.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/worker.cc b/src/worker.cc index 31795a90c7..fed19befb2 100644 --- a/src/worker.cc +++ b/src/worker.cc @@ -138,7 +138,7 @@ const std::unordered_map error_codes{ {445, {445, "Shape match algorithm specification in api request is incorrect. Please see documentation for valid shape_match input.", 400, HTTP_400, OSRM_INVALID_URL, "wrong_match_type"}}, {499, {499, "Unknown", 400, HTTP_400, OSRM_INVALID_URL, "unknown"}}, {503, {503, "Leg count mismatch", 400, HTTP_400, OSRM_INVALID_URL, "wrong_number_of_legs"}}, - {504, {504, "This service does not support GeoTIFF serialization.", 400, HTTP_400, OSRM_INVALID_VALUE, "wrong_number_of_legs"}}, + {504, {504, "This service does not support GeoTIFF serialization.", 400, HTTP_400, OSRM_INVALID_VALUE, "unknown"}}, {599, {599, "Unknown serialization error", 400, HTTP_400, OSRM_INVALID_VALUE, "unknown"}}, }; From d6771b8d247822d53b314344ec45cad668a53163 Mon Sep 17 00:00:00 2001 From: Christian Beiwinkel Date: Wed, 28 Feb 2024 15:22:46 +0100 Subject: [PATCH 26/27] set nodata value to numeric max limit --- src/tyr/isochrone_serializer.cc | 9 +-------- test/isochrone.cc | 12 ++++++------ 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/src/tyr/isochrone_serializer.cc b/src/tyr/isochrone_serializer.cc index 01e801a094..a1b504158e 100644 --- a/src/tyr/isochrone_serializer.cc +++ b/src/tyr/isochrone_serializer.cc @@ -187,13 +187,6 @@ std::string serializeGeoTIFF(Api& request, const std::shared_ptrSetGeoTransform(geo_transform); geotiff_dataset->SetSpatialRef(const_cast(&spatial_ref)); - // geotiff doesn't support different nodata values for each band, so we just get the larger one - float max_time = isogrid->MaxValue(0); - float max_distance = isogrid->MaxValue(1); - - // they will be scaled to seconds/10 meters - float no_data_value = max_time > max_distance ? max_time * 60 : max_distance * 100; - for (size_t metric_idx = 0; metric_idx < metrics.size(); ++metric_idx) { if (!metrics[metric_idx]) continue; // only create bands for requested metrics @@ -209,7 +202,7 @@ std::string serializeGeoTIFF(Api& request, const std::shared_ptrGetRasterBand(nbands == 2 ? (metric_idx + 1) : 1); - band->SetNoDataValue(static_cast(no_data_value)); + band->SetNoDataValue(std::numeric_limits::max()); band->SetDescription(metric_idx == 0 ? "Time (seconds)" : "Distance (10m)"); CPLErr err = band->RasterIO(GF_Write, 0, 0, ext_x, ext_y, data, ext_x, ext_y, GDT_UInt16, 0, 0); diff --git a/test/isochrone.cc b/test/isochrone.cc index 0259869745..1d890b15b9 100644 --- a/test/isochrone.cc +++ b/test/isochrone.cc @@ -384,8 +384,8 @@ TEST(Isochrones, test_geotiff_output_distance) { ASSERT_NE(x, 0); ASSERT_NE(y, 0); ASSERT_EQ(static_cast(min_max[0]), 0); - ASSERT_EQ(static_cast(min_max[1]), 1099); - ASSERT_EQ(band->GetNoDataValue(), 1100); + ASSERT_EQ(static_cast(min_max[1]), 1100); + ASSERT_EQ(band->GetNoDataValue(), std::numeric_limits::max()); size_t array_size = x * y; check_raster_edges(x, y, data_array); @@ -430,8 +430,8 @@ TEST(Isochrones, test_geotiff_output_time) { ASSERT_GT(x, 0); ASSERT_GT(y, 0); ASSERT_EQ(static_cast(min_max[0]), 0); - ASSERT_EQ(static_cast(min_max[1]), 659); // nodata value is excluded from min/max - ASSERT_EQ(band->GetNoDataValue(), 660); + ASSERT_EQ(static_cast(min_max[1]), 660); + ASSERT_EQ(band->GetNoDataValue(), std::numeric_limits::max()); size_t array_size = x * y; check_raster_edges(x, y, data_array); @@ -468,7 +468,7 @@ TEST(Isochrones, test_geotiff_output_time_distance) { int y = geotiff_dataset->GetRasterYSize(); // time, distance - std::array expected_max{660, 1199}; + std::array expected_max{660, 1200}; for (int b = 1; b <= 2; ++b) { GDALRasterBand* band = geotiff_dataset->GetRasterBand(b); @@ -483,7 +483,7 @@ TEST(Isochrones, test_geotiff_output_time_distance) { ASSERT_NE(y, 0); ASSERT_EQ(static_cast(min_max[0]), 0); ASSERT_EQ(static_cast(min_max[1]), expected_max[b - 1]); - ASSERT_EQ(band->GetNoDataValue(), 1200); // set for the whole dataset + ASSERT_EQ(band->GetNoDataValue(), std::numeric_limits::max()); size_t array_size = x * y; check_raster_edges(x, y, data_array); From 6da3a08f92c0549b4050486cf7e7a4405eabeb93 Mon Sep 17 00:00:00 2001 From: Christian Beiwinkel Date: Wed, 28 Feb 2024 17:37:12 +0100 Subject: [PATCH 27/27] hopefully the last missing boost header --- test/gurka/test_avoids.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/test/gurka/test_avoids.cc b/test/gurka/test_avoids.cc index c8d6ad8c55..64a9d1547b 100644 --- a/test/gurka/test_avoids.cc +++ b/test/gurka/test_avoids.cc @@ -1,4 +1,5 @@ #include "gurka.h" +#include #include #include #include