From 1e9f266a9dd3b5334a9c5598a9894c35cf3a720c Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Wed, 11 Jul 2018 15:34:44 +0100 Subject: [PATCH 01/39] use property_mapper (#68) --- src/feature_builder.hpp | 19 +++++++++++++------ src/vtcomposite.cpp | 9 +++++---- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/feature_builder.hpp b/src/feature_builder.hpp index 7ed1680..0428b65 100644 --- a/src/feature_builder.hpp +++ b/src/feature_builder.hpp @@ -5,6 +5,7 @@ #include // vtzero #include +#include // boost #include #include @@ -127,23 +128,28 @@ struct overzoomed_feature_builder { using coordinate_type = CoordinateType; overzoomed_feature_builder(vtzero::layer_builder& layer_builder, + vtzero::property_mapper& mapper, mapbox::geometry::box const& bbox, int dx, int dy, int zoom_factor) : layer_builder_{layer_builder}, + mapper_{mapper}, bbox_{bbox}, dx_{dx}, dy_{dy}, zoom_factor_{zoom_factor} {} template - void finalize(FeatureBuilder& builder, vtzero::feature const& feature) + void finalize(FeatureBuilder& builder, vtzero::feature& feature) { // add properties - builder.copy_properties(feature); + while (auto idxs = feature.next_property_indexes()) + { + builder.add_property(mapper_(idxs)); + } builder.commit(); } - void apply_geometry_point(vtzero::feature const& feature) + void apply_geometry_point(vtzero::feature& feature) { mapbox::geometry::multi_point multi_point; vtzero::decode_point_geometry(feature.geometry(), detail::point_handler(multi_point, dx_, dy_, zoom_factor_)); @@ -161,7 +167,7 @@ struct overzoomed_feature_builder } } - void apply_geometry_linestring(vtzero::feature const& feature) + void apply_geometry_linestring(vtzero::feature& feature) { mapbox::geometry::multi_line_string multi_line; vtzero::decode_linestring_geometry(feature.geometry(), detail::line_string_handler(multi_line, dx_, dy_, zoom_factor_)); @@ -185,7 +191,7 @@ struct overzoomed_feature_builder } if (valid) finalize(feature_builder, feature); } - void apply_geometry_polygon(vtzero::feature const& feature) + void apply_geometry_polygon(vtzero::feature& feature) { std::vector> rings; vtzero::decode_polygon_geometry(feature.geometry(), detail::polygon_handler(rings, dx_, dy_, zoom_factor_)); @@ -232,7 +238,7 @@ struct overzoomed_feature_builder if (valid) finalize(feature_builder, feature); } - void apply(vtzero::feature const& feature) + void apply(vtzero::feature& feature) { switch (feature.geometry_type()) { @@ -252,6 +258,7 @@ struct overzoomed_feature_builder } } vtzero::layer_builder& layer_builder_; + vtzero::property_mapper& mapper_; mapbox::geometry::box const& bbox_; int dx_; int dy_; diff --git a/src/vtcomposite.cpp b/src/vtcomposite.cpp index 49f0a34..6027722 100644 --- a/src/vtcomposite.cpp +++ b/src/vtcomposite.cpp @@ -136,17 +136,18 @@ struct CompositeWorker : Nan::AsyncWorker else { vtzero::layer_builder layer_builder{builder, name, MVT_VERSION_2, extent}; + vtzero::property_mapper mapper{layer, layer_builder}; using coordinate_type = std::int64_t; int dx, dy; std::tie(dx, dy) = vtile::displacement(tile_obj->z, static_cast(extent), target_z, target_x, target_y); mapbox::geometry::box bbox{{-buffer_size, -buffer_size}, {static_cast(extent) + buffer_size, static_cast(extent) + buffer_size}}; - vtile::overzoomed_feature_builder feature_builder{layer_builder, bbox, dx, dy, zoom_factor}; - layer.for_each_feature([&](vtzero::feature const& feature) { + vtile::overzoomed_feature_builder feature_builder{layer_builder, mapper, bbox, dx, dy, zoom_factor}; + while (auto feature = layer.next_feature()) + { feature_builder.apply(feature); - return true; - }); + } } } } From 4f2d2e5812ab1826ae39f8dbe26187dac2270aca Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Sat, 21 Jul 2018 19:30:20 -0700 Subject: [PATCH 02/39] use latest vtzero API - refs #68 --- src/feature_builder.hpp | 101 ++++++++++++++++++++-------------------- src/vtcomposite.cpp | 8 ++-- 2 files changed, 55 insertions(+), 54 deletions(-) diff --git a/src/feature_builder.hpp b/src/feature_builder.hpp index 0428b65..cb86b25 100644 --- a/src/feature_builder.hpp +++ b/src/feature_builder.hpp @@ -139,22 +139,18 @@ struct overzoomed_feature_builder zoom_factor_{zoom_factor} {} template - void finalize(FeatureBuilder& builder, vtzero::feature& feature) + void finalize(FeatureBuilder& builder, vtzero::feature const& feature) { // add properties - while (auto idxs = feature.next_property_indexes()) - { - builder.add_property(mapper_(idxs)); - } + builder.copy_properties(feature, mapper_); builder.commit(); } - void apply_geometry_point(vtzero::feature& feature) + void apply_geometry_point(vtzero::feature const& feature) { mapbox::geometry::multi_point multi_point; vtzero::decode_point_geometry(feature.geometry(), detail::point_handler(multi_point, dx_, dy_, zoom_factor_)); vtzero::point_feature_builder feature_builder{layer_builder_}; - feature_builder.copy_id(feature); multi_point.erase(std::remove_if(multi_point.begin(), multi_point.end(), [this](auto const& pt) { return !boost::geometry::covered_by(pt, bbox_); }), @@ -162,83 +158,88 @@ struct overzoomed_feature_builder if (!multi_point.empty()) { + feature_builder.copy_id(feature); feature_builder.add_points_from_container(multi_point); finalize(feature_builder, feature); } } - void apply_geometry_linestring(vtzero::feature& feature) + void apply_geometry_linestring(vtzero::feature const& feature) { mapbox::geometry::multi_line_string multi_line; vtzero::decode_linestring_geometry(feature.geometry(), detail::line_string_handler(multi_line, dx_, dy_, zoom_factor_)); std::vector> result; boost::geometry::intersection(multi_line, bbox_, result); - bool valid = false; - vtzero::linestring_feature_builder feature_builder{layer_builder_}; - feature_builder.copy_id(feature); - - for (auto const& l : result) + if (!result.empty()) { - if (l.size() > 1) + bool valid = false; + vtzero::linestring_feature_builder feature_builder{layer_builder_}; + feature_builder.copy_id(feature); + for (auto const& l : result) { - valid = true; - feature_builder.add_linestring(static_cast(l.size())); - for (auto const& pt : l) + if (l.size() > 1) { - feature_builder.set_point(static_cast(pt.x), static_cast(pt.y)); + valid = true; + feature_builder.add_linestring(static_cast(l.size())); + for (auto const& pt : l) + { + feature_builder.set_point(static_cast(pt.x), static_cast(pt.y)); + } } } + if (valid) finalize(feature_builder, feature); } - if (valid) finalize(feature_builder, feature); } - void apply_geometry_polygon(vtzero::feature& feature) + void apply_geometry_polygon(vtzero::feature const& feature) { std::vector> rings; vtzero::decode_polygon_geometry(feature.geometry(), detail::polygon_handler(rings, dx_, dy_, zoom_factor_)); - vtzero::polygon_feature_builder feature_builder{layer_builder_}; - feature_builder.copy_id(feature); - - bool valid = false; - bool process = false; - for (auto& r : rings) + if (!rings.empty()) { - if (r.second == vtzero::ring_type::outer) + vtzero::polygon_feature_builder feature_builder{layer_builder_}; + feature_builder.copy_id(feature); + bool valid = false; + bool process = false; + for (auto& r : rings) { - auto extent = mapbox::geometry::envelope(r.first); - process = boost::geometry::intersects(extent, bbox_); - } - if (process) - { - std::vector> result; - if (r.second == vtzero::ring_type::inner) boost::geometry::reverse(r.first); - // ^^ reverse inner rings before clipping as we're dealing with a disassembled polygon - boost::geometry::intersection(r.first, bbox_, result); - for (auto const& p : result) + if (r.second == vtzero::ring_type::outer) + { + auto extent = mapbox::geometry::envelope(r.first); + process = boost::geometry::intersects(extent, bbox_); + } + if (process) { - for (auto const& ring : p) + std::vector> result; + if (r.second == vtzero::ring_type::inner) boost::geometry::reverse(r.first); + // ^^ reverse inner rings before clipping as we're dealing with a disassembled polygon + boost::geometry::intersection(r.first, bbox_, result); + for (auto const& p : result) { - if (ring.size() > 3) + for (auto const& ring : p) { - valid = true; - feature_builder.add_ring(static_cast(ring.size())); - if (r.second == vtzero::ring_type::outer) - { - std::for_each(ring.begin(), ring.end(), [&feature_builder](auto const& pt) { feature_builder.set_point(static_cast(pt.x), static_cast(pt.y)); }); - } - else + if (ring.size() > 3) { - // apply points in reverse to preserve original winding order of inner rings - std::for_each(ring.rbegin(), ring.rend(), [&feature_builder](auto const& pt) { feature_builder.set_point(static_cast(pt.x), static_cast(pt.y)); }); + valid = true; + feature_builder.add_ring(static_cast(ring.size())); + if (r.second == vtzero::ring_type::outer) + { + std::for_each(ring.begin(), ring.end(), [&feature_builder](auto const& pt) { feature_builder.set_point(static_cast(pt.x), static_cast(pt.y)); }); + } + else + { + // apply points in reverse to preserve original winding order of inner rings + std::for_each(ring.rbegin(), ring.rend(), [&feature_builder](auto const& pt) { feature_builder.set_point(static_cast(pt.x), static_cast(pt.y)); }); + } } } } } } + if (valid) finalize(feature_builder, feature); } - if (valid) finalize(feature_builder, feature); } - void apply(vtzero::feature& feature) + void apply(vtzero::feature const& feature) { switch (feature.geometry_type()) { diff --git a/src/vtcomposite.cpp b/src/vtcomposite.cpp index 6027722..4dd92da 100644 --- a/src/vtcomposite.cpp +++ b/src/vtcomposite.cpp @@ -144,10 +144,10 @@ struct CompositeWorker : Nan::AsyncWorker {static_cast(extent) + buffer_size, static_cast(extent) + buffer_size}}; vtile::overzoomed_feature_builder feature_builder{layer_builder, mapper, bbox, dx, dy, zoom_factor}; - while (auto feature = layer.next_feature()) - { - feature_builder.apply(feature); - } + layer.for_each_feature([&](vtzero::feature const& feature) { + feature_builder.apply(feature); + return true; + }); } } } From 25053e51fad9c6c14045980db9c59e4a1fa1f262 Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Sun, 22 Jul 2018 14:05:46 -0700 Subject: [PATCH 03/39] optimize point filtering --- src/feature_builder.hpp | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/feature_builder.hpp b/src/feature_builder.hpp index cb86b25..d1a8b30 100644 --- a/src/feature_builder.hpp +++ b/src/feature_builder.hpp @@ -22,8 +22,9 @@ template struct point_handler { using geom_type = mapbox::geometry::multi_point; - point_handler(geom_type& geom, int dx, int dy, int zoom_factor) + point_handler(geom_type& geom, int dx, int dy, int zoom_factor, mapbox::geometry::box const& bbox) : geom_(geom), + bbox_(bbox), dx_(dx), dy_(dy), zoom_factor_(zoom_factor) @@ -32,19 +33,27 @@ struct point_handler void points_begin(std::uint32_t count) { - geom_.reserve(count); + if (count > 1) + { + geom_.reserve(count); + } } void points_point(vtzero::point const& pt) { CoordinateType x = pt.x * zoom_factor_ - dx_; CoordinateType y = pt.y * zoom_factor_ - dy_; - geom_.emplace_back(x, y); + mapbox::geometry::point pt0{x,y}; + if (boost::geometry::covered_by(pt0, bbox_)) + { + geom_.push_back(std::move(pt0)); + } } void points_end() {} geom_type& geom_; + mapbox::geometry::box const& bbox_; int dx_; int dy_; int zoom_factor_; @@ -149,15 +158,10 @@ struct overzoomed_feature_builder void apply_geometry_point(vtzero::feature const& feature) { mapbox::geometry::multi_point multi_point; - vtzero::decode_point_geometry(feature.geometry(), detail::point_handler(multi_point, dx_, dy_, zoom_factor_)); - vtzero::point_feature_builder feature_builder{layer_builder_}; - multi_point.erase(std::remove_if(multi_point.begin(), multi_point.end(), [this](auto const& pt) { - return !boost::geometry::covered_by(pt, bbox_); - }), - multi_point.end()); - + vtzero::decode_point_geometry(feature.geometry(), detail::point_handler(multi_point, dx_, dy_, zoom_factor_, bbox_)); if (!multi_point.empty()) { + vtzero::point_feature_builder feature_builder{layer_builder_}; feature_builder.copy_id(feature); feature_builder.add_points_from_container(multi_point); finalize(feature_builder, feature); From c381ccd1e18162f389ba93d726e3868977c777a8 Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Sun, 22 Jul 2018 14:14:01 -0700 Subject: [PATCH 04/39] run clang-format --- src/feature_builder.hpp | 2 +- src/vtcomposite.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/feature_builder.hpp b/src/feature_builder.hpp index d1a8b30..0c766c2 100644 --- a/src/feature_builder.hpp +++ b/src/feature_builder.hpp @@ -43,7 +43,7 @@ struct point_handler { CoordinateType x = pt.x * zoom_factor_ - dx_; CoordinateType y = pt.y * zoom_factor_ - dy_; - mapbox::geometry::point pt0{x,y}; + mapbox::geometry::point pt0{x, y}; if (boost::geometry::covered_by(pt0, bbox_)) { geom_.push_back(std::move(pt0)); diff --git a/src/vtcomposite.cpp b/src/vtcomposite.cpp index 4dd92da..e0a0388 100644 --- a/src/vtcomposite.cpp +++ b/src/vtcomposite.cpp @@ -145,8 +145,8 @@ struct CompositeWorker : Nan::AsyncWorker static_cast(extent) + buffer_size}}; vtile::overzoomed_feature_builder feature_builder{layer_builder, mapper, bbox, dx, dy, zoom_factor}; layer.for_each_feature([&](vtzero::feature const& feature) { - feature_builder.apply(feature); - return true; + feature_builder.apply(feature); + return true; }); } } From d257c192126893ec41c0194ab028696bb4c19383 Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Fri, 27 Jul 2018 17:38:17 -0700 Subject: [PATCH 05/39] Remove `reserve` in the point_handler - Because, in the overzooming case, we are very likely to throw out points this reserve is going to over-allocate leading to CPU time requesting memory that will never be used and holding onto more memory than is needed - So, it makese sense to avoid calling this to ensure that the overzooming case is fast - Note: the same advantage is not the same with lines or polygons because we currently have no code that filters them. Rather they need to be fully constructed and therefore the reserve used in those handlers makes sense. --- src/feature_builder.hpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/feature_builder.hpp b/src/feature_builder.hpp index 0c766c2..e19a99e 100644 --- a/src/feature_builder.hpp +++ b/src/feature_builder.hpp @@ -33,10 +33,6 @@ struct point_handler void points_begin(std::uint32_t count) { - if (count > 1) - { - geom_.reserve(count); - } } void points_point(vtzero::point const& pt) From 63b212a85b11ece138ded434ed799e25f313631a Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Fri, 27 Jul 2018 17:52:00 -0700 Subject: [PATCH 06/39] remove unused things --- src/feature_builder.hpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/feature_builder.hpp b/src/feature_builder.hpp index efcbb4f..2b02817 100644 --- a/src/feature_builder.hpp +++ b/src/feature_builder.hpp @@ -7,7 +7,6 @@ #include #include // boost -#include #include #include // stl @@ -31,7 +30,7 @@ struct point_handler { } - void points_begin(std::uint32_t count) + void points_begin(std::uint32_t) { } From 774f53d337f993a75f34d5d25ed3c51f19883b5f Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Fri, 27 Jul 2018 17:58:16 -0700 Subject: [PATCH 07/39] fix broken code from incorrect merge --- src/feature_builder.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/feature_builder.hpp b/src/feature_builder.hpp index 2b02817..1b996db 100644 --- a/src/feature_builder.hpp +++ b/src/feature_builder.hpp @@ -192,9 +192,9 @@ struct overzoomed_feature_builder last_pt = *itr; } } - } - if (valid) finalize(feature_builder, feature); + } } + if (valid) finalize(feature_builder, feature); } } void apply_geometry_polygon(vtzero::feature const& feature) From c563018a724a8364149fb16e01a9fa800578da8e Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Fri, 27 Jul 2018 19:47:18 -0700 Subject: [PATCH 08/39] start porting to libdeflate - mapbox/gzip-hpp#25 --- binding.gyp | 3 ++- src/vtcomposite.cpp | 14 ++++++++------ 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/binding.gyp b/binding.gyp index e904562..f4da872 100644 --- a/binding.gyp +++ b/binding.gyp @@ -94,7 +94,8 @@ ], 'xcode_settings': { 'OTHER_LDFLAGS':[ - '-Wl,-bind_at_load' + '-Wl,-bind_at_load', + '<(module_root_dir)/mason_packages/.link/lib/libdeflate.a' ], 'OTHER_CPLUSPLUSFLAGS': [ '<@(system_includes)', diff --git a/src/vtcomposite.cpp b/src/vtcomposite.cpp index e0a0388..06b8b76 100644 --- a/src/vtcomposite.cpp +++ b/src/vtcomposite.cpp @@ -101,7 +101,9 @@ struct CompositeWorker : Nan::AsyncWorker int const target_x = baton_data_->x; int const target_y = baton_data_->y; - std::vector>> buffer_cache; + std::vector buffer_cache; + gzip::Decompressor decompressor; + gzip::Compressor compressor; for (auto const& tile_obj : baton_data_->tiles) { @@ -110,10 +112,10 @@ struct CompositeWorker : Nan::AsyncWorker vtzero::data_view tile_view{}; if (gzip::is_compressed(tile_obj->data.data(), tile_obj->data.size())) { - buffer_cache.push_back(std::make_unique>()); - gzip::Decompressor decompressor; - decompressor.decompress(*buffer_cache.back(), tile_obj->data.data(), tile_obj->data.size()); - tile_view = protozero::data_view{buffer_cache.back()->data(), buffer_cache.back()->size()}; + buffer_cache.emplace_back(); + std::string & buf = buffer_cache.back(); + decompressor.decompress(buf, tile_obj->data.data(), tile_obj->data.size()); + tile_view = vtzero::data_view{buf}; } else { @@ -166,7 +168,7 @@ struct CompositeWorker : Nan::AsyncWorker { std::string temp; builder.serialize(temp); - tile_buffer = gzip::compress(temp.data(), temp.size()); + compressor.compress(tile_buffer, temp.data(), temp.size()); } else { From 50b1a850c5e5c66ef5eb18145f5dfef65a9c63f0 Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Fri, 27 Jul 2018 20:00:37 -0700 Subject: [PATCH 09/39] build against local gzip-hpp --- .travis.yml | 1 + Makefile | 4 ++-- mason-versions.ini | 4 ++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/.travis.yml b/.travis.yml index 35a2f7b..01a8eb5 100644 --- a/.travis.yml +++ b/.travis.yml @@ -13,6 +13,7 @@ install: - which node - clang++ -v - which clang++ + - git clone -b libdeflate-redux https://github.com/mapbox/gzip-hpp ../gzip-hpp - make ${BUILDTYPE} # *Here we run tests* diff --git a/Makefile b/Makefile index ba7bd3b..611a418 100644 --- a/Makefile +++ b/Makefile @@ -36,11 +36,11 @@ mason_packages/.link/include: mason_packages/headers build-deps: mason_packages/.link/include release: build-deps - V=1 ./node_modules/.bin/node-pre-gyp configure build --error_on_warnings=$(WERROR) --loglevel=error + V=1 CXXFLAGS=-I`pwd`/../gzip-hpp/include ./node_modules/.bin/node-pre-gyp configure build --error_on_warnings=$(WERROR) --loglevel=error @echo "run 'make clean' for full rebuild" debug: mason_packages/.link/include - V=1 ./node_modules/.bin/node-pre-gyp configure build --error_on_warnings=$(WERROR) --loglevel=error --debug + V=1 CXXFLAGS=-I`pwd`/../gzip-hpp/include ./node_modules/.bin/node-pre-gyp configure build --error_on_warnings=$(WERROR) --loglevel=error --debug @echo "run 'make clean' for full rebuild" coverage: build-deps diff --git a/mason-versions.ini b/mason-versions.ini index 4e646fe..7f11d00 100644 --- a/mason-versions.ini +++ b/mason-versions.ini @@ -1,14 +1,14 @@ [headers] boost=1.66.0 geometry=96d3505 -gzip-hpp=0.1.0 protozero=1.6.2 spatial-algorithms=0.1.0 variant=1.1.5 vtzero=2915725 [compiled] +libdeflate=e9d1014 clang++=5.0.1 clang-tidy=5.0.1 clang-format=5.0.1 llvm-cov=5.0.1 -binutils=2.30 \ No newline at end of file +binutils=2.30 From 89cf870914665169ce5983714ccf446e7a1a1f43 Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Fri, 27 Jul 2018 20:12:35 -0700 Subject: [PATCH 10/39] also link libdeflate on linux --- binding.gyp | 1 + 1 file changed, 1 insertion(+) diff --git a/binding.gyp b/binding.gyp index f4da872..a48e34a 100644 --- a/binding.gyp +++ b/binding.gyp @@ -79,6 +79,7 @@ ], 'ldflags': [ '-Wl,-z,now', + '<(module_root_dir)/mason_packages/.link/lib/libdeflate.a' ], 'conditions': [ ['error_on_warnings == "true"', { From cb239cc62a692197c353cafe7f8fdc5c95a2d410 Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Fri, 27 Jul 2018 20:25:32 -0700 Subject: [PATCH 11/39] try again to get linking working on linux --- binding.gyp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/binding.gyp b/binding.gyp index a48e34a..57840cf 100644 --- a/binding.gyp +++ b/binding.gyp @@ -77,9 +77,11 @@ './src/module.cpp', './src/vtcomposite.cpp' ], + 'libraries':[ + '<(module_root_dir)/mason_packages/.link/lib/libdeflate.a' + ], 'ldflags': [ - '-Wl,-z,now', - '<(module_root_dir)/mason_packages/.link/lib/libdeflate.a' + '-Wl,-z,now' ], 'conditions': [ ['error_on_warnings == "true"', { @@ -95,8 +97,7 @@ ], 'xcode_settings': { 'OTHER_LDFLAGS':[ - '-Wl,-bind_at_load', - '<(module_root_dir)/mason_packages/.link/lib/libdeflate.a' + '-Wl,-bind_at_load' ], 'OTHER_CPLUSPLUSFLAGS': [ '<@(system_includes)', From f43f059773357f9fc7dfc7245cce6909e097b28d Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Fri, 27 Jul 2018 20:30:15 -0700 Subject: [PATCH 12/39] fix local header search --- .travis.yml | 1 + Makefile | 4 ++-- src/vtcomposite.cpp | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index 01a8eb5..f42a9ab 100644 --- a/.travis.yml +++ b/.travis.yml @@ -13,6 +13,7 @@ install: - which node - clang++ -v - which clang++ + - export CXXFLAGS="-I`pwd`/../gzip-hpp/include" - git clone -b libdeflate-redux https://github.com/mapbox/gzip-hpp ../gzip-hpp - make ${BUILDTYPE} diff --git a/Makefile b/Makefile index 611a418..f8b509b 100644 --- a/Makefile +++ b/Makefile @@ -36,11 +36,11 @@ mason_packages/.link/include: mason_packages/headers build-deps: mason_packages/.link/include release: build-deps - V=1 CXXFLAGS=-I`pwd`/../gzip-hpp/include ./node_modules/.bin/node-pre-gyp configure build --error_on_warnings=$(WERROR) --loglevel=error + V=1 CXXFLAGS="-I`pwd`/../gzip-hpp/include" ./node_modules/.bin/node-pre-gyp configure build --error_on_warnings=$(WERROR) --loglevel=error @echo "run 'make clean' for full rebuild" debug: mason_packages/.link/include - V=1 CXXFLAGS=-I`pwd`/../gzip-hpp/include ./node_modules/.bin/node-pre-gyp configure build --error_on_warnings=$(WERROR) --loglevel=error --debug + V=1 CXXFLAGS="-I`pwd`/../gzip-hpp/include" ./node_modules/.bin/node-pre-gyp configure build --error_on_warnings=$(WERROR) --loglevel=error --debug @echo "run 'make clean' for full rebuild" coverage: build-deps diff --git a/src/vtcomposite.cpp b/src/vtcomposite.cpp index 06b8b76..e5ba1eb 100644 --- a/src/vtcomposite.cpp +++ b/src/vtcomposite.cpp @@ -113,7 +113,7 @@ struct CompositeWorker : Nan::AsyncWorker if (gzip::is_compressed(tile_obj->data.data(), tile_obj->data.size())) { buffer_cache.emplace_back(); - std::string & buf = buffer_cache.back(); + std::string& buf = buffer_cache.back(); decompressor.decompress(buf, tile_obj->data.data(), tile_obj->data.size()); tile_view = vtzero::data_view{buf}; } From dfb66bcb96ade1a7a0517f6dfe4c72dc2858bbd9 Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Fri, 27 Jul 2018 20:34:38 -0700 Subject: [PATCH 13/39] inherit flags --- Makefile | 4 ++-- scripts/sanitize.sh | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index f8b509b..5a89267 100644 --- a/Makefile +++ b/Makefile @@ -36,11 +36,11 @@ mason_packages/.link/include: mason_packages/headers build-deps: mason_packages/.link/include release: build-deps - V=1 CXXFLAGS="-I`pwd`/../gzip-hpp/include" ./node_modules/.bin/node-pre-gyp configure build --error_on_warnings=$(WERROR) --loglevel=error + V=1 CXXFLAGS="$(CXXFLAGS) -I`pwd`/../gzip-hpp/include" ./node_modules/.bin/node-pre-gyp configure build --error_on_warnings=$(WERROR) --loglevel=error @echo "run 'make clean' for full rebuild" debug: mason_packages/.link/include - V=1 CXXFLAGS="-I`pwd`/../gzip-hpp/include" ./node_modules/.bin/node-pre-gyp configure build --error_on_warnings=$(WERROR) --loglevel=error --debug + V=1 CXXFLAGS="$(CXXFLAGS) -I`pwd`/../gzip-hpp/include" ./node_modules/.bin/node-pre-gyp configure build --error_on_warnings=$(WERROR) --loglevel=error --debug @echo "run 'make clean' for full rebuild" coverage: build-deps diff --git a/scripts/sanitize.sh b/scripts/sanitize.sh index 669f936..670b455 100755 --- a/scripts/sanitize.sh +++ b/scripts/sanitize.sh @@ -51,4 +51,3 @@ else LD_PRELOAD=${MASON_LLVM_RT_PRELOAD} \ npm test fi - From 00e6296bbf2e6a004fd2a203391a83487e2d6061 Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Fri, 27 Jul 2018 21:22:32 -0700 Subject: [PATCH 14/39] set CXXFLAGS so everybody sees them --- .travis.yml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index f42a9ab..de7a205 100644 --- a/.travis.yml +++ b/.travis.yml @@ -8,13 +8,15 @@ addons: sources: [ 'ubuntu-toolchain-r-test' ] packages: [ 'libstdc++-4.9-dev' ] +before_install: + - export CXXFLAGS="-I`pwd`/../gzip-hpp/include" + - git clone -b libdeflate-redux https://github.com/mapbox/gzip-hpp ../gzip-hpp + install: - node -v - which node - clang++ -v - which clang++ - - export CXXFLAGS="-I`pwd`/../gzip-hpp/include" - - git clone -b libdeflate-redux https://github.com/mapbox/gzip-hpp ../gzip-hpp - make ${BUILDTYPE} # *Here we run tests* From 7090aa8d4ae199291c783964625522385bc14179 Mon Sep 17 00:00:00 2001 From: Ann Millspaugh Date: Thu, 2 Aug 2018 12:48:51 -0700 Subject: [PATCH 15/39] releasing 0.2.0-zlib [publish binary] From 65906152048aea99c528af6e8329502327decf41 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Thu, 4 Oct 2018 18:40:34 +0100 Subject: [PATCH 16/39] upgrade libdeflate to version 1.0 --- mason-versions.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mason-versions.ini b/mason-versions.ini index 7f11d00..5107ee5 100644 --- a/mason-versions.ini +++ b/mason-versions.ini @@ -6,7 +6,7 @@ spatial-algorithms=0.1.0 variant=1.1.5 vtzero=2915725 [compiled] -libdeflate=e9d1014 +libdeflate=1.0 clang++=5.0.1 clang-tidy=5.0.1 clang-format=5.0.1 From fafeb47b9748a5fcf3944a1835bd62bd2d0eee20 Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Mon, 8 Oct 2018 13:44:18 -0700 Subject: [PATCH 17/39] inherit CXXFLAGS to avoid breaking coverage build --- .travis.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index de7a205..995e853 100644 --- a/.travis.yml +++ b/.travis.yml @@ -9,9 +9,9 @@ addons: packages: [ 'libstdc++-4.9-dev' ] before_install: - - export CXXFLAGS="-I`pwd`/../gzip-hpp/include" + - export CXXFLAGS="-I`pwd`/../gzip-hpp/include ${CXXFLAGS}" - git clone -b libdeflate-redux https://github.com/mapbox/gzip-hpp ../gzip-hpp - + install: - node -v - which node From 70bac5f2a3f3b470ef845d7b8ae52da0edbc81a4 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Mon, 15 Oct 2018 10:37:53 +0100 Subject: [PATCH 18/39] use local libdeflate c++ wrapper [initial commit] --- .travis.yml | 4 - Makefile | 4 +- src/deflate.hpp | 175 ++++++++++++++++++++++++++++++++++++++++++++ src/vtcomposite.cpp | 15 ++-- 4 files changed, 183 insertions(+), 15 deletions(-) create mode 100644 src/deflate.hpp diff --git a/.travis.yml b/.travis.yml index 995e853..35a2f7b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -8,10 +8,6 @@ addons: sources: [ 'ubuntu-toolchain-r-test' ] packages: [ 'libstdc++-4.9-dev' ] -before_install: - - export CXXFLAGS="-I`pwd`/../gzip-hpp/include ${CXXFLAGS}" - - git clone -b libdeflate-redux https://github.com/mapbox/gzip-hpp ../gzip-hpp - install: - node -v - which node diff --git a/Makefile b/Makefile index 554b0be..d33e717 100644 --- a/Makefile +++ b/Makefile @@ -43,11 +43,11 @@ mason_packages/.link/include: mason_packages/headers build-deps: mason_packages/.link/include release: build-deps - V=1 CXXFLAGS="$(CXXFLAGS) -I`pwd`/../gzip-hpp/include" ./node_modules/.bin/node-pre-gyp configure build --error_on_warnings=$(WERROR) --loglevel=error + V=1 ./node_modules/.bin/node-pre-gyp configure build --error_on_warnings=$(WERROR) --loglevel=error @echo "run 'make clean' for full rebuild" debug: mason_packages/.link/include - V=1 CXXFLAGS="$(CXXFLAGS) -I`pwd`/../gzip-hpp/include" ./node_modules/.bin/node-pre-gyp configure build --error_on_warnings=$(WERROR) --loglevel=error --debug + V=1 ./node_modules/.bin/node-pre-gyp configure build --error_on_warnings=$(WERROR) --loglevel=error --debug @echo "run 'make clean' for full rebuild" coverage: build-deps diff --git a/src/deflate.hpp b/src/deflate.hpp new file mode 100644 index 0000000..0c75b01 --- /dev/null +++ b/src/deflate.hpp @@ -0,0 +1,175 @@ +#pragma once + +// libdeflate +#include +// std +#include +#include +#include +#include + +namespace deflate { + +inline bool is_zlib(char const* data, std::size_t size) noexcept +{ + return size > 2 && + (static_cast(data[0]) == 0x78 && + (static_cast(data[1]) == 0x9C || + static_cast(data[1]) == 0x01 || + static_cast(data[1]) == 0xDA || + static_cast(data[1]) == 0x5E)); +} + +inline bool is_gzip(char const* data, std::size_t size) noexcept +{ + return size > 2 && (static_cast(data[0]) == 0x1F && static_cast(data[1]) == 0x8B); +} + +inline bool is_compressed(char const* data, std::size_t size) noexcept +{ + return is_gzip(data, size) || is_zlib(data, size); +} + +class Compressor +{ + struct cleanup_compressor + { + void operator()(libdeflate_compressor* ptr) const + { + if (ptr) libdeflate_free_compressor(ptr); + } + }; + std::size_t max_; + int level_; + std::unique_ptr compressor_; + // make noncopyable + Compressor(Compressor const&) = delete; + Compressor& operator=(Compressor const&) = delete; + + public: + Compressor(int level = 6, + std::size_t max_bytes = 2000000000) // by default refuse operation if uncompressed data is > 2GB + : max_{max_bytes}, + level_{level}, + compressor_{libdeflate_alloc_compressor(level_)} + { + if (!compressor_) + { + throw std::runtime_error("libdeflate_alloc_compressor failed"); + } + } + + template + void operator()(OutputType& output, + char const* data, + std::size_t size) const + { + if (size > max_) + { + throw std::runtime_error("size may use more memory than intended when decompressing"); + } + + std::size_t max_compressed_size = libdeflate_gzip_compress_bound(compressor_.get(), size); + // TODO: sanity check this before allocating + if (max_compressed_size > output.size()) + { + output.resize(max_compressed_size); + } + + std::size_t actual_compressed_size = libdeflate_gzip_compress(compressor_.get(), + data, + size, + const_cast(output.data()), + max_compressed_size); + if (actual_compressed_size == 0) + { + throw std::runtime_error("actual_compressed_size 0"); + } + output.resize(actual_compressed_size); + } +}; + +class Decompressor +{ + struct cleanup_decompressor + { + void operator()(libdeflate_decompressor* ptr) const + { + if (ptr) libdeflate_free_decompressor(ptr); + } + }; + + std::size_t const max_; + std::unique_ptr decompressor_; + // noncopyable + Decompressor(Decompressor const&) = delete; + Decompressor& operator=(Decompressor const&) = delete; + + public: + Decompressor(std::size_t max_bytes = 2147483648u) // by default refuse operation if required uutput buffer is > 2GB + : max_{max_bytes}, + decompressor_{libdeflate_alloc_decompressor(), cleanup_decompressor()} + { + if (!decompressor_) + { + throw std::runtime_error("libdeflate_alloc_decompressor failed"); + } + } + + template + void operator()(OutputType& output, + char const* data, + std::size_t size) const + { + if (is_gzip(data, size)) + apply(output, libdeflate_gzip_decompress, data, size); + else if (is_zlib(data, size)) + apply(output, libdeflate_zlib_decompress, data, size); + //else throw std::runtime_error("bad data"); + } + + template + void apply(OutputType& output, Fun fun, + char const* data, + std::size_t size) const + { + std::size_t actual_size; + std::size_t uncompressed_size_guess = std::min(size * 4, max_); + output.resize(uncompressed_size_guess); + libdeflate_result result; + for (;;) + { + result = fun(decompressor_.get(), + data, + size, + const_cast(output.data()), + output.size(), &actual_size); + if (result != LIBDEFLATE_INSUFFICIENT_SPACE) + { + break; + } + if (output.size() == max_) + { + throw std::runtime_error("request to resize output buffer can't exceed maximum limit"); + } + std::size_t new_size = std::min((output.capacity() << 1) - output.size(), max_); + output.resize(new_size); + } + + if (result == LIBDEFLATE_SHORT_OUTPUT) + { + throw std::runtime_error("short output: did not succeed"); + } + else if (result == LIBDEFLATE_BAD_DATA) + { + throw std::runtime_error("bad data: did not succeed"); + } + else if (result != LIBDEFLATE_SUCCESS) + { + throw std::runtime_error("did not succeed"); + } + output.resize(actual_size); + } +}; + +} // namespace deflate diff --git a/src/vtcomposite.cpp b/src/vtcomposite.cpp index 0b1d686..eb7f91c 100644 --- a/src/vtcomposite.cpp +++ b/src/vtcomposite.cpp @@ -3,10 +3,7 @@ #include "module_utils.hpp" #include "zxy_math.hpp" #include "feature_builder.hpp" -// gzip-hpp -#include -#include -#include +#include "deflate.hpp" // vtzero #include #include @@ -141,19 +138,19 @@ struct CompositeWorker : Nan::AsyncWorker int const target_y = baton_data_->y; std::vector buffer_cache; - gzip::Decompressor decompressor; - gzip::Compressor compressor; + deflate::Decompressor decompressor; + deflate::Compressor compressor; for (auto const& tile_obj : baton_data_->tiles) { if (vtile::within_target(*tile_obj, target_z, target_x, target_y)) { vtzero::data_view tile_view{}; - if (gzip::is_compressed(tile_obj->data.data(), tile_obj->data.size())) + if (deflate::is_compressed(tile_obj->data.data(), tile_obj->data.size())) { buffer_cache.emplace_back(); std::string& buf = buffer_cache.back(); - decompressor.decompress(buf, tile_obj->data.data(), tile_obj->data.size()); + decompressor(buf, tile_obj->data.data(), tile_obj->data.size()); tile_view = vtzero::data_view{buf}; } else @@ -213,7 +210,7 @@ struct CompositeWorker : Nan::AsyncWorker { std::string temp; builder.serialize(temp); - compressor.compress(tile_buffer, temp.data(), temp.size()); + compressor(tile_buffer, temp.data(), temp.size()); } else { From 55d1a9664d4ef4ffb3ebf1e79e4a9099d19e772d Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Mon, 15 Oct 2018 10:39:41 +0100 Subject: [PATCH 19/39] remove unused header --- src/deflate.hpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/deflate.hpp b/src/deflate.hpp index 0c75b01..19d9537 100644 --- a/src/deflate.hpp +++ b/src/deflate.hpp @@ -5,7 +5,6 @@ // std #include #include -#include #include namespace deflate { From 5b8c152771ad9da5a3d3d40af8a56641b262e4f6 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Mon, 15 Oct 2018 10:44:42 +0100 Subject: [PATCH 20/39] simplify + format --- src/deflate.hpp | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/deflate.hpp b/src/deflate.hpp index 19d9537..694dec5 100644 --- a/src/deflate.hpp +++ b/src/deflate.hpp @@ -9,24 +9,23 @@ namespace deflate { -inline bool is_zlib(char const* data, std::size_t size) noexcept +inline bool is_zlib(char const* data) noexcept { - return size > 2 && - (static_cast(data[0]) == 0x78 && + return (static_cast(data[0]) == 0x78 && (static_cast(data[1]) == 0x9C || static_cast(data[1]) == 0x01 || static_cast(data[1]) == 0xDA || static_cast(data[1]) == 0x5E)); } -inline bool is_gzip(char const* data, std::size_t size) noexcept +inline bool is_gzip(char const* data) noexcept { - return size > 2 && (static_cast(data[0]) == 0x1F && static_cast(data[1]) == 0x8B); + return (static_cast(data[0]) == 0x1F && static_cast(data[1]) == 0x8B); } inline bool is_compressed(char const* data, std::size_t size) noexcept { - return is_gzip(data, size) || is_zlib(data, size); + return size > 2 && (is_gzip(data) || is_zlib(data)); } class Compressor @@ -120,11 +119,10 @@ class Decompressor char const* data, std::size_t size) const { - if (is_gzip(data, size)) + if (is_gzip(data)) apply(output, libdeflate_gzip_decompress, data, size); - else if (is_zlib(data, size)) + else if (is_zlib(data)) apply(output, libdeflate_zlib_decompress, data, size); - //else throw std::runtime_error("bad data"); } template From 7ef90b281c281906ae0e3fa4e2efc45f33e6d130 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Mon, 15 Oct 2018 13:16:02 +0100 Subject: [PATCH 21/39] sync with master --- src/vtcomposite.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/vtcomposite.cpp b/src/vtcomposite.cpp index eb7f91c..e59b4bf 100644 --- a/src/vtcomposite.cpp +++ b/src/vtcomposite.cpp @@ -137,7 +137,7 @@ struct CompositeWorker : Nan::AsyncWorker int const target_x = baton_data_->x; int const target_y = baton_data_->y; - std::vector buffer_cache; + std::vector>> buffer_cache; deflate::Decompressor decompressor; deflate::Compressor compressor; @@ -148,10 +148,9 @@ struct CompositeWorker : Nan::AsyncWorker vtzero::data_view tile_view{}; if (deflate::is_compressed(tile_obj->data.data(), tile_obj->data.size())) { - buffer_cache.emplace_back(); - std::string& buf = buffer_cache.back(); - decompressor(buf, tile_obj->data.data(), tile_obj->data.size()); - tile_view = vtzero::data_view{buf}; + buffer_cache.push_back(std::make_unique>()); + decompressor(*buffer_cache.back(), tile_obj->data.data(), tile_obj->data.size()); + tile_view = protozero::data_view{buffer_cache.back()->data(), buffer_cache.back()->size()}; } else { From 9282beea6c4881bfcdb8ca9d6566dea13cb63a3b Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Thu, 18 Oct 2018 11:16:34 +0100 Subject: [PATCH 22/39] Fixed usage of deprecated (node-v10.12.0) methods. --- src/vtcomposite.cpp | 76 ++++++++++++++++++++++++++++++++++++++------- 1 file changed, 64 insertions(+), 12 deletions(-) diff --git a/src/vtcomposite.cpp b/src/vtcomposite.cpp index e59b4bf..7fd3d31 100644 --- a/src/vtcomposite.cpp +++ b/src/vtcomposite.cpp @@ -275,6 +275,8 @@ NAN_METHOD(composite) std::unique_ptr baton_data = std::make_unique(num_tiles); + v8::Isolate *isolate = v8::Isolate::GetCurrent(); + for (unsigned t = 0; t < num_tiles; ++t) { v8::Local tile_val = tiles->Get(t); @@ -282,7 +284,12 @@ NAN_METHOD(composite) { return utils::CallbackError("items in 'tiles' array must be objects", callback); } - v8::Local tile_obj = tile_val->ToObject(); + v8::MaybeLocal maybe_tile_obj = tile_val->ToObject(isolate->GetCurrentContext()); + if (maybe_tile_obj.IsEmpty()) + { + return utils::CallbackError("items in 'tiles' can't be empty", callback); + } + v8::Local tile_obj = maybe_tile_obj.ToLocalChecked(); // check buffer value if (!tile_obj->Has(Nan::New("buffer").ToLocalChecked())) @@ -294,7 +301,13 @@ NAN_METHOD(composite) { return utils::CallbackError("buffer value in 'tiles' array item is null or undefined", callback); } - v8::Local buffer = buf_val->ToObject(); + v8::MaybeLocal maybe_buffer = buf_val->ToObject(isolate->GetCurrentContext()); + if (maybe_buffer.IsEmpty()) + { + return utils::CallbackError("buffer value in 'tiles' array is empty", callback); + } + v8::Local buffer = maybe_buffer.ToLocalChecked(); + if (!node::Buffer::HasInstance(buffer)) { return utils::CallbackError("buffer value in 'tiles' array item is not a true buffer", callback); @@ -310,7 +323,12 @@ NAN_METHOD(composite) { return utils::CallbackError("'z' value in 'tiles' array item is not an int32", callback); } - int z = z_val->Int32Value(); + v8::Maybe maybe_z = z_val->Int32Value(isolate->GetCurrentContext()); + if (maybe_z.IsNothing()) + { + return utils::CallbackError("'z' value is nothing", callback); + } + int z = maybe_z.FromJust(); if (z < 0) { return utils::CallbackError("'z' value must not be less than zero", callback); @@ -326,7 +344,12 @@ NAN_METHOD(composite) { return utils::CallbackError("'x' value in 'tiles' array item is not an int32", callback); } - int x = x_val->Int32Value(); + v8::Maybe maybe_x = x_val->Int32Value(isolate->GetCurrentContext()); + if (maybe_x.IsNothing()) + { + return utils::CallbackError("'x' value is nothing", callback); + } + int x = maybe_x.FromJust(); if (x < 0) { return utils::CallbackError("'x' value must not be less than zero", callback); @@ -342,7 +365,12 @@ NAN_METHOD(composite) { return utils::CallbackError("'y' value in 'tiles' array item is not an int32", callback); } - int y = y_val->Int32Value(); + v8::Maybe maybe_y = y_val->Int32Value(isolate->GetCurrentContext()); + if (maybe_y.IsNothing()) + { + return utils::CallbackError("'y' value is nothing", callback); + } + int y = maybe_y.FromJust(); if (y < 0) { return utils::CallbackError("'y' value must not be less than zero", callback); @@ -367,7 +395,12 @@ NAN_METHOD(composite) { return utils::CallbackError("'z' value in 'tiles' array item is not an int32", callback); } - int z_maprequest = z_val_maprequest->Int32Value(); + v8::Maybe maybe_z_maprequest = z_val_maprequest->Int32Value(isolate->GetCurrentContext()); + if (maybe_z_maprequest.IsNothing()) + { + return utils::CallbackError("'z' value in 'tiles' array item is nothing", callback); + } + int z_maprequest = maybe_z_maprequest.FromJust(); if (z_maprequest < 0) { return utils::CallbackError("'z' value must not be less than zero", callback); @@ -384,7 +417,12 @@ NAN_METHOD(composite) { return utils::CallbackError("'x' value in 'tiles' array item is not an int32", callback); } - int x_maprequest = x_val_maprequest->Int32Value(); + v8::Maybe maybe_x_maprequest = x_val_maprequest->Int32Value(isolate->GetCurrentContext()); + if (maybe_x_maprequest.IsNothing()) + { + return utils::CallbackError("'x' value in 'tiles' array item is nothing", callback); + } + int x_maprequest = maybe_x_maprequest.FromJust(); if (x_maprequest < 0) { return utils::CallbackError("'x' value must not be less than zero", callback); @@ -402,7 +440,12 @@ NAN_METHOD(composite) { return utils::CallbackError("'y' value in 'tiles' array item is not an int32", callback); } - int y_maprequest = y_val_maprequest->Int32Value(); + v8::Maybe maybe_y_maprequest = y_val_maprequest->Int32Value(isolate->GetCurrentContext()); + if (maybe_y_maprequest.IsNothing()) + { + return utils::CallbackError("'y' value in 'tiles' array item is nothing", callback); + } + int y_maprequest = maybe_y_maprequest.FromJust(); if (y_maprequest < 0) { return utils::CallbackError("'y' value must not be less than zero", callback); @@ -416,7 +459,7 @@ NAN_METHOD(composite) { return utils::CallbackError("'options' arg must be an object", callback); } - v8::Local options = info[2]->ToObject(); + v8::Local options = info[2]->ToObject(isolate->GetCurrentContext()).ToLocalChecked(); if (options->Has(Nan::New("buffer_size").ToLocalChecked())) { v8::Local bs_value = options->Get(Nan::New("buffer_size").ToLocalChecked()); @@ -424,8 +467,12 @@ NAN_METHOD(composite) { return utils::CallbackError("'buffer_size' must be an int32", callback); } - - int buffer_size = bs_value->Int32Value(); + v8::Maybe maybe_buffer_size = bs_value->Int32Value(isolate->GetCurrentContext()); + if (maybe_buffer_size.IsNothing()) + { + return utils::CallbackError("'buffer_size' is nothing", callback); + } + int buffer_size = maybe_buffer_size.FromJust(); if (buffer_size < 0) { return utils::CallbackError("'buffer_size' must be a positive int32", callback); @@ -439,7 +486,12 @@ NAN_METHOD(composite) { return utils::CallbackError("'compress' must be a boolean", callback); } - baton_data->compress = comp_value->BooleanValue(); + v8::Maybe maybe_compress = comp_value->BooleanValue(isolate->GetCurrentContext()); + if (maybe_compress.IsNothing()) + { + return utils::CallbackError("'compress' is nothing", callback); + } + baton_data->compress = maybe_compress.FromJust(); } } // enter the threadpool, then done in the callback function call the threadpool From d0ad0a73b75f54a8589b46e7640b544454afee38 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Thu, 18 Oct 2018 11:21:32 +0100 Subject: [PATCH 23/39] clang-format --- src/vtcomposite.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/vtcomposite.cpp b/src/vtcomposite.cpp index 7fd3d31..bf3e308 100644 --- a/src/vtcomposite.cpp +++ b/src/vtcomposite.cpp @@ -275,7 +275,7 @@ NAN_METHOD(composite) std::unique_ptr baton_data = std::make_unique(num_tiles); - v8::Isolate *isolate = v8::Isolate::GetCurrent(); + v8::Isolate* isolate = v8::Isolate::GetCurrent(); for (unsigned t = 0; t < num_tiles; ++t) { @@ -345,7 +345,7 @@ NAN_METHOD(composite) return utils::CallbackError("'x' value in 'tiles' array item is not an int32", callback); } v8::Maybe maybe_x = x_val->Int32Value(isolate->GetCurrentContext()); - if (maybe_x.IsNothing()) + if (maybe_x.IsNothing()) { return utils::CallbackError("'x' value is nothing", callback); } From 1224ec620af46bd164386c7ffbd5376cd74bbe75 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Thu, 18 Oct 2018 11:58:27 +0100 Subject: [PATCH 24/39] attempting to fix .travis.yml (/home/travis/.travis/job_stages: line 373: exit: : numeric argument required) --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 35a2f7b..206f01d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -120,7 +120,7 @@ matrix: install: - make ${BUILDTYPE} # Overrides `script` to disable publishing - script: [] + script: # Coverage build - os: linux env: BUILDTYPE=debug CXXFLAGS="--coverage" LDFLAGS="--coverage" From faf52cb6a7122f5f0b820307ee7c70a45228c884 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Fri, 19 Oct 2018 10:40:29 +0100 Subject: [PATCH 25/39] Use `Nan::To` converters --- src/vtcomposite.cpp | 44 ++++++++++++++++++++++++-------------------- 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/src/vtcomposite.cpp b/src/vtcomposite.cpp index bf3e308..d9e5efb 100644 --- a/src/vtcomposite.cpp +++ b/src/vtcomposite.cpp @@ -275,8 +275,6 @@ NAN_METHOD(composite) std::unique_ptr baton_data = std::make_unique(num_tiles); - v8::Isolate* isolate = v8::Isolate::GetCurrent(); - for (unsigned t = 0; t < num_tiles; ++t) { v8::Local tile_val = tiles->Get(t); @@ -284,7 +282,7 @@ NAN_METHOD(composite) { return utils::CallbackError("items in 'tiles' array must be objects", callback); } - v8::MaybeLocal maybe_tile_obj = tile_val->ToObject(isolate->GetCurrentContext()); + Nan::MaybeLocal maybe_tile_obj = Nan::To(tile_val); if (maybe_tile_obj.IsEmpty()) { return utils::CallbackError("items in 'tiles' can't be empty", callback); @@ -301,7 +299,7 @@ NAN_METHOD(composite) { return utils::CallbackError("buffer value in 'tiles' array item is null or undefined", callback); } - v8::MaybeLocal maybe_buffer = buf_val->ToObject(isolate->GetCurrentContext()); + Nan::MaybeLocal maybe_buffer = Nan::To(buf_val); if (maybe_buffer.IsEmpty()) { return utils::CallbackError("buffer value in 'tiles' array is empty", callback); @@ -323,12 +321,12 @@ NAN_METHOD(composite) { return utils::CallbackError("'z' value in 'tiles' array item is not an int32", callback); } - v8::Maybe maybe_z = z_val->Int32Value(isolate->GetCurrentContext()); + Nan::Maybe maybe_z = Nan::To(z_val); if (maybe_z.IsNothing()) { return utils::CallbackError("'z' value is nothing", callback); } - int z = maybe_z.FromJust(); + int32_t z = maybe_z.FromJust(); if (z < 0) { return utils::CallbackError("'z' value must not be less than zero", callback); @@ -344,12 +342,12 @@ NAN_METHOD(composite) { return utils::CallbackError("'x' value in 'tiles' array item is not an int32", callback); } - v8::Maybe maybe_x = x_val->Int32Value(isolate->GetCurrentContext()); + Nan::Maybe maybe_x = Nan::To(x_val); if (maybe_x.IsNothing()) { return utils::CallbackError("'x' value is nothing", callback); } - int x = maybe_x.FromJust(); + std::int32_t x = maybe_x.FromJust(); if (x < 0) { return utils::CallbackError("'x' value must not be less than zero", callback); @@ -365,12 +363,12 @@ NAN_METHOD(composite) { return utils::CallbackError("'y' value in 'tiles' array item is not an int32", callback); } - v8::Maybe maybe_y = y_val->Int32Value(isolate->GetCurrentContext()); + Nan::Maybe maybe_y = Nan::To(y_val); if (maybe_y.IsNothing()) { return utils::CallbackError("'y' value is nothing", callback); } - int y = maybe_y.FromJust(); + std::int32_t y = maybe_y.FromJust(); if (y < 0) { return utils::CallbackError("'y' value must not be less than zero", callback); @@ -395,12 +393,12 @@ NAN_METHOD(composite) { return utils::CallbackError("'z' value in 'tiles' array item is not an int32", callback); } - v8::Maybe maybe_z_maprequest = z_val_maprequest->Int32Value(isolate->GetCurrentContext()); + Nan::Maybe maybe_z_maprequest = Nan::To(z_val_maprequest); if (maybe_z_maprequest.IsNothing()) { return utils::CallbackError("'z' value in 'tiles' array item is nothing", callback); } - int z_maprequest = maybe_z_maprequest.FromJust(); + std::int32_t z_maprequest = maybe_z_maprequest.FromJust(); if (z_maprequest < 0) { return utils::CallbackError("'z' value must not be less than zero", callback); @@ -417,12 +415,12 @@ NAN_METHOD(composite) { return utils::CallbackError("'x' value in 'tiles' array item is not an int32", callback); } - v8::Maybe maybe_x_maprequest = x_val_maprequest->Int32Value(isolate->GetCurrentContext()); + Nan::Maybe maybe_x_maprequest = Nan::To(x_val_maprequest); if (maybe_x_maprequest.IsNothing()) { return utils::CallbackError("'x' value in 'tiles' array item is nothing", callback); } - int x_maprequest = maybe_x_maprequest.FromJust(); + std::int32_t x_maprequest = maybe_x_maprequest.FromJust(); if (x_maprequest < 0) { return utils::CallbackError("'x' value must not be less than zero", callback); @@ -440,12 +438,12 @@ NAN_METHOD(composite) { return utils::CallbackError("'y' value in 'tiles' array item is not an int32", callback); } - v8::Maybe maybe_y_maprequest = y_val_maprequest->Int32Value(isolate->GetCurrentContext()); + Nan::Maybe maybe_y_maprequest = Nan::To(y_val_maprequest); if (maybe_y_maprequest.IsNothing()) { return utils::CallbackError("'y' value in 'tiles' array item is nothing", callback); } - int y_maprequest = maybe_y_maprequest.FromJust(); + std::int32_t y_maprequest = maybe_y_maprequest.FromJust(); if (y_maprequest < 0) { return utils::CallbackError("'y' value must not be less than zero", callback); @@ -459,7 +457,13 @@ NAN_METHOD(composite) { return utils::CallbackError("'options' arg must be an object", callback); } - v8::Local options = info[2]->ToObject(isolate->GetCurrentContext()).ToLocalChecked(); + Nan::MaybeLocal maybe_options = Nan::To(info[2]); + if (maybe_options.IsEmpty()) + { + return utils::CallbackError("'options' value must not be empty", callback); + } + v8::Local options = maybe_options.ToLocalChecked(); + if (options->Has(Nan::New("buffer_size").ToLocalChecked())) { v8::Local bs_value = options->Get(Nan::New("buffer_size").ToLocalChecked()); @@ -467,12 +471,12 @@ NAN_METHOD(composite) { return utils::CallbackError("'buffer_size' must be an int32", callback); } - v8::Maybe maybe_buffer_size = bs_value->Int32Value(isolate->GetCurrentContext()); + Nan::Maybe maybe_buffer_size = Nan::To(bs_value); if (maybe_buffer_size.IsNothing()) { return utils::CallbackError("'buffer_size' is nothing", callback); } - int buffer_size = maybe_buffer_size.FromJust(); + std::int32_t buffer_size = maybe_buffer_size.FromJust(); if (buffer_size < 0) { return utils::CallbackError("'buffer_size' must be a positive int32", callback); @@ -486,7 +490,7 @@ NAN_METHOD(composite) { return utils::CallbackError("'compress' must be a boolean", callback); } - v8::Maybe maybe_compress = comp_value->BooleanValue(isolate->GetCurrentContext()); + Nan::Maybe maybe_compress = Nan::To(comp_value); if (maybe_compress.IsNothing()) { return utils::CallbackError("'compress' is nothing", callback); From c15df0941c0693d35090e511bd6e9c4534366b38 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Fri, 19 Oct 2018 11:03:20 +0100 Subject: [PATCH 26/39] use `std::vector>` as buffer cache. --- src/vtcomposite.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/vtcomposite.cpp b/src/vtcomposite.cpp index d9e5efb..e2c40e9 100644 --- a/src/vtcomposite.cpp +++ b/src/vtcomposite.cpp @@ -137,7 +137,7 @@ struct CompositeWorker : Nan::AsyncWorker int const target_x = baton_data_->x; int const target_y = baton_data_->y; - std::vector>> buffer_cache; + std::vector> buffer_cache; deflate::Decompressor decompressor; deflate::Compressor compressor; @@ -148,9 +148,9 @@ struct CompositeWorker : Nan::AsyncWorker vtzero::data_view tile_view{}; if (deflate::is_compressed(tile_obj->data.data(), tile_obj->data.size())) { - buffer_cache.push_back(std::make_unique>()); - decompressor(*buffer_cache.back(), tile_obj->data.data(), tile_obj->data.size()); - tile_view = protozero::data_view{buffer_cache.back()->data(), buffer_cache.back()->size()}; + buffer_cache.emplace_back(); + decompressor(buffer_cache.back(), tile_obj->data.data(), tile_obj->data.size()); + tile_view = protozero::data_view{buffer_cache.back().data(), buffer_cache.back().size()}; } else { From 698fa0fb9a78384ed2d2e4dddde0eb5ee1ca92cf Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Thu, 25 Oct 2018 14:24:24 +0100 Subject: [PATCH 27/39] Initial implementation useing `N-API` via `node-addon-api` --- binding.gyp | 7 +- package-lock.json | 10 +- package.json | 4 +- src/module.cpp | 10 +- src/module_utils.hpp | 26 +-- src/vtcomposite.cpp | 225 ++++++++++------------ src/vtcomposite.hpp | 5 +- test/vtcomposite-param-validation.test.js | 3 +- 8 files changed, 139 insertions(+), 151 deletions(-) diff --git a/binding.gyp b/binding.gyp index e904562..4ec0037 100644 --- a/binding.gyp +++ b/binding.gyp @@ -8,14 +8,14 @@ ['AR', '<(module_root_dir)/mason_packages/.link/bin/llvm-ar'], ['NM', '<(module_root_dir)/mason_packages/.link/bin/llvm-nm'] ], - 'includes': [ 'common.gypi' ], # brings in a default set of options that are inherited from gyp + 'includes': [ 'common.gypi'], # brings in a default set of options that are inherited from gyp 'variables': { # custom variables we use specific to this file 'error_on_warnings%':'true', # can be overriden by a command line variable because of the % sign using "WERROR" (defined in Makefile) - # Use this variable to silence warnings from mason dependencies and from NAN + # Use this variable to silence warnings from mason dependencies # It's a variable to make easy to pass to # cflags (linux) and xcode (mac) 'system_includes': [ - "-isystem <(module_root_dir)/ +#include +#include -NAN_MODULE_INIT(init) +Napi::Object init(Napi::Env env, Napi::Object exports) { - Nan::SetMethod(target, "composite", vtile::composite); + exports.Set(Napi::String::New(env, "composite"), Napi::Function::New(env, vtile::composite)); + return exports; } -NODE_MODULE(module, init) // NOLINT +NODE_API_MODULE(module, init) // NOLINT diff --git a/src/module_utils.hpp b/src/module_utils.hpp index 87a9691..cb363ad 100644 --- a/src/module_utils.hpp +++ b/src/module_utils.hpp @@ -1,5 +1,6 @@ #pragma once -#include +#include +#include namespace utils { @@ -8,21 +9,14 @@ namespace utils { * throwing errors. * Usage: * -* v8::Local callback; -* return CallbackError("error message", callback); // "return" is important to -* prevent duplicate callbacks from being fired! -* -* -* "inline" is important here as well. See for more contex: -* - https://github.com/mapbox/cpp/blob/master/glossary.md#inline-keyword -* - https://github.com/mapbox/node-cpp-skel/pull/52#discussion_r126847394 for -* context -* +* Napi::Function callback; +* return CallbackError("error message", callback); */ -inline void CallbackError(std::string message, v8::Local func) + +inline Napi::Value CallbackError(std::string const& message, Napi::CallbackInfo const& info, Napi::Function const& func) { - Nan::Callback cb(func); - v8::Local argv[1] = {Nan::Error(message.c_str())}; - Nan::Call(cb, 1, argv); + Napi::Object obj = Napi::Object::New(info.Env()); + obj.Set("message", message); + return func.Call({obj}); } -} // namespace utils \ No newline at end of file +} // namespace utils diff --git a/src/vtcomposite.cpp b/src/vtcomposite.cpp index 1eeae05..51f554a 100644 --- a/src/vtcomposite.cpp +++ b/src/vtcomposite.cpp @@ -26,14 +26,13 @@ struct TileObject TileObject(int z0, int x0, int y0, - v8::Local& buffer) + Napi::Object& buffer) : z{z0}, x{x0}, y{y0}, - data{node::Buffer::Data(buffer), node::Buffer::Length(buffer)}, - buffer_ref{} + data{buffer.As>().Data(), buffer.As>().Length()}, + buffer_ref{Napi::Persistent(buffer.As())} { - buffer_ref.Reset(buffer.As()); } ~TileObject() @@ -45,15 +44,11 @@ struct TileObject TileObject(TileObject const&) = delete; TileObject& operator=(TileObject const&) = delete; - // non-movable - TileObject(TileObject&&) = delete; - TileObject& operator=(TileObject&&) = delete; - int z; int x; int y; vtzero::data_view data; - Nan::Persistent buffer_ref; + Napi::ObjectReference buffer_ref; }; struct BatonType @@ -67,10 +62,6 @@ struct BatonType BatonType(BatonType const&) = delete; BatonType& operator=(BatonType const&) = delete; - // non-movable - BatonType(BatonType&&) = delete; - BatonType& operator=(BatonType&&) = delete; - // members std::vector> tiles{}; int z{}; @@ -119,12 +110,12 @@ struct build_feature_from_v2 } // namespace -struct CompositeWorker : Nan::AsyncWorker +struct CompositeWorker : Napi::AsyncWorker { - using Base = Nan::AsyncWorker; + using Base = Napi::AsyncWorker; - CompositeWorker(std::unique_ptr&& baton_data, Nan::Callback* cb) - : Base(cb, "skel:standalone-async-worker"), + CompositeWorker(std::unique_ptr&& baton_data, Napi::Function& cb) + : Base(cb), baton_data_{std::move(baton_data)}, output_buffer_{std::make_unique()} {} @@ -221,232 +212,226 @@ struct CompositeWorker : Nan::AsyncWorker // LCOV_EXCL_START catch (std::exception const& e) { - SetErrorMessage(e.what()); + SetError(e.what()); } // LCOV_EXCL_STOP } - - void HandleOKCallback() override + void OnOK() override { std::string& tile_buffer = *output_buffer_.get(); - Nan::HandleScope scope; - const auto argc = 2u; - v8::Local argv[argc] = { - Nan::Null(), - Nan::NewBuffer(&tile_buffer[0], - static_cast(tile_buffer.size()), - [](char*, void* hint) { - delete reinterpret_cast(hint); - }, - output_buffer_.release()) - .ToLocalChecked()}; - - // Static cast done here to avoid 'cppcoreguidelines-pro-bounds-array-to-pointer-decay' warning with clang-tidy - callback->Call(argc, static_cast*>(argv), async_resource); + Napi::HandleScope scope(Env()); + Napi::Value argv = Napi::Buffer::New(Env(), + const_cast(tile_buffer.data()), + tile_buffer.size(), + [](Napi::Env, char*, std::string* s) { + delete s; + }, + output_buffer_.release()); + + Callback().Call({Env().Null(), argv}); } std::unique_ptr const baton_data_; std::unique_ptr output_buffer_; }; -NAN_METHOD(composite) +Napi::Value composite(Napi::CallbackInfo const& info) { // validate callback function - v8::Local callback_val = info[info.Length() - 1]; - if (!callback_val->IsFunction()) + Napi::Value callback_val = info[info.Length() - 1]; + if (!callback_val.IsFunction()) { - Nan::ThrowError("last argument must be a callback function"); - return; + Napi::Error::New(info.Env(), "last argument must be a callback function").ThrowAsJavaScriptException(); + return info.Env().Null(); } - v8::Local callback = callback_val.As(); + Napi::Function callback = callback_val.As(); // validate tiles - if (!info[0]->IsArray()) + if (!info[0].IsArray()) { - return utils::CallbackError("first arg 'tiles' must be an array of tile objects", callback); + return utils::CallbackError("first arg 'tiles' must be an array of tile objects", info, callback); } - v8::Local tiles = info[0].As(); - unsigned num_tiles = tiles->Length(); + Napi::Array tiles = info[0].As(); + unsigned num_tiles = tiles.Length(); if (num_tiles <= 0) { - return utils::CallbackError("'tiles' array must be of length greater than 0", callback); + return utils::CallbackError("'tiles' array must be of length greater than 0", info, callback); } std::unique_ptr baton_data = std::make_unique(num_tiles); for (unsigned t = 0; t < num_tiles; ++t) { - v8::Local tile_val = tiles->Get(t); - if (!tile_val->IsObject()) + Napi::Value tile_val = tiles.Get(t); + if (!tile_val.IsObject()) { - return utils::CallbackError("items in 'tiles' array must be objects", callback); + return utils::CallbackError("items in 'tiles' array must be objects", info, callback); } - v8::Local tile_obj = tile_val->ToObject(); + Napi::Object tile_obj = tile_val.ToObject(); // check buffer value - if (!tile_obj->Has(Nan::New("buffer").ToLocalChecked())) + if (!tile_obj.Has(Napi::String::New(info.Env(), "buffer"))) { - return utils::CallbackError("item in 'tiles' array does not include a buffer value", callback); + return utils::CallbackError("item in 'tiles' array does not include a buffer value", info, callback); } - v8::Local buf_val = tile_obj->Get(Nan::New("buffer").ToLocalChecked()); - if (buf_val->IsNull() || buf_val->IsUndefined()) + Napi::Value buf_val = tile_obj.Get(Napi::String::New(info.Env(), "buffer")); + if (buf_val.IsNull() || buf_val.IsUndefined()) { - return utils::CallbackError("buffer value in 'tiles' array item is null or undefined", callback); + return utils::CallbackError("buffer value in 'tiles' array item is null or undefined", info, callback); } - v8::Local buffer = buf_val->ToObject(); - if (!node::Buffer::HasInstance(buffer)) + Napi::Object buffer = buf_val.ToObject(); + if (!buffer.IsBuffer()) { - return utils::CallbackError("buffer value in 'tiles' array item is not a true buffer", callback); + return utils::CallbackError("buffer value in 'tiles' array item is not a true buffer", info, callback); } // z value - if (!tile_obj->Has(Nan::New("z").ToLocalChecked())) + if (!tile_obj.Has(Napi::String::New(info.Env(), "z"))) { - return utils::CallbackError("item in 'tiles' array does not include a 'z' value", callback); + return utils::CallbackError("item in 'tiles' array does not include a 'z' value", info, callback); } - v8::Local z_val = tile_obj->Get(Nan::New("z").ToLocalChecked()); - if (!z_val->IsInt32()) + Napi::Value z_val = tile_obj.Get(Napi::String::New(info.Env(), "z")); + if (!z_val.IsNumber()) { - return utils::CallbackError("'z' value in 'tiles' array item is not an int32", callback); + return utils::CallbackError("'z' value in 'tiles' array item is not an int32", info, callback); } - int z = z_val->Int32Value(); + int z = z_val.As().Int32Value(); if (z < 0) { - return utils::CallbackError("'z' value must not be less than zero", callback); + return utils::CallbackError("'z' value must not be less than zero", info, callback); } // x value - if (!tile_obj->Has(Nan::New("x").ToLocalChecked())) + if (!tile_obj.Has(Napi::String::New(info.Env(), "x"))) { - return utils::CallbackError("item in 'tiles' array does not include a 'x' value", callback); + return utils::CallbackError("item in 'tiles' array does not include a 'x' value", info, callback); } - v8::Local x_val = tile_obj->Get(Nan::New("x").ToLocalChecked()); - if (!x_val->IsInt32()) + Napi::Value x_val = tile_obj.Get(Napi::String::New(info.Env(), "x")); + if (!x_val.IsNumber()) { - return utils::CallbackError("'x' value in 'tiles' array item is not an int32", callback); + return utils::CallbackError("'x' value in 'tiles' array item is not an int32", info, callback); } - int x = x_val->Int32Value(); + int x = x_val.As().Int32Value(); if (x < 0) { - return utils::CallbackError("'x' value must not be less than zero", callback); + return utils::CallbackError("'x' value must not be less than zero", info, callback); } // y value - if (!tile_obj->Has(Nan::New("y").ToLocalChecked())) + if (!tile_obj.Has(Napi::String::New(info.Env(), "y"))) { - return utils::CallbackError("item in 'tiles' array does not include a 'y' value", callback); + return utils::CallbackError("item in 'tiles' array does not include a 'y' value", info, callback); } - v8::Local y_val = tile_obj->Get(Nan::New("y").ToLocalChecked()); - if (!y_val->IsInt32()) + Napi::Value y_val = tile_obj.Get(Napi::String::New(info.Env(), "y")); + if (!y_val.IsNumber()) { - return utils::CallbackError("'y' value in 'tiles' array item is not an int32", callback); + return utils::CallbackError("'y' value in 'tiles' array item is not an int32", info, callback); } - int y = y_val->Int32Value(); + int y = y_val.As().Int32Value(); if (y < 0) { - return utils::CallbackError("'y' value must not be less than zero", callback); + return utils::CallbackError("'y' value must not be less than zero", info, callback); } baton_data->tiles.push_back(std::make_unique(z, x, y, buffer)); } //validate zxy maprequest object - if (!info[1]->IsObject()) + if (!info[1].IsObject()) { - return utils::CallbackError("'zxy_maprequest' must be an object", callback); + return utils::CallbackError("'zxy_maprequest' must be an object", info, callback); } - v8::Local zxy_maprequest = v8::Local::Cast(info[1]); + Napi::Object zxy_maprequest = info[1].As(); // z value of map request object - if (!zxy_maprequest->Has(Nan::New("z").ToLocalChecked())) + if (!zxy_maprequest.Has(Napi::String::New(info.Env(), "z"))) { - return utils::CallbackError("item in 'tiles' array does not include a 'z' value", callback); + return utils::CallbackError("item in 'tiles' array does not include a 'z' value", info, callback); } - v8::Local z_val_maprequest = zxy_maprequest->Get(Nan::New("z").ToLocalChecked()); - if (!z_val_maprequest->IsInt32()) + Napi::Value z_val_maprequest = zxy_maprequest.Get(Napi::String::New(info.Env(), "z")); + if (!z_val_maprequest.IsNumber()) { - return utils::CallbackError("'z' value in 'tiles' array item is not an int32", callback); + return utils::CallbackError("'z' value in 'tiles' array item is not an int32", info, callback); } - int z_maprequest = z_val_maprequest->Int32Value(); + int z_maprequest = z_val_maprequest.As().Int32Value(); if (z_maprequest < 0) { - return utils::CallbackError("'z' value must not be less than zero", callback); + return utils::CallbackError("'z' value must not be less than zero", info, callback); } baton_data->z = z_maprequest; // x value of map request object - if (!zxy_maprequest->Has(Nan::New("x").ToLocalChecked())) + if (!zxy_maprequest.Has(Napi::String::New(info.Env(), "x"))) { - return utils::CallbackError("item in 'tiles' array does not include a 'x' value", callback); + return utils::CallbackError("item in 'tiles' array does not include a 'x' value", info, callback); } - v8::Local x_val_maprequest = zxy_maprequest->Get(Nan::New("x").ToLocalChecked()); - if (!x_val_maprequest->IsInt32()) + Napi::Value x_val_maprequest = zxy_maprequest.Get(Napi::String::New(info.Env(), "x")); + if (!x_val_maprequest.IsNumber()) { - return utils::CallbackError("'x' value in 'tiles' array item is not an int32", callback); + return utils::CallbackError("'x' value in 'tiles' array item is not an int32", info, callback); } - int x_maprequest = x_val_maprequest->Int32Value(); + int x_maprequest = x_val_maprequest.As().Int32Value(); if (x_maprequest < 0) { - return utils::CallbackError("'x' value must not be less than zero", callback); + return utils::CallbackError("'x' value must not be less than zero", info, callback); } baton_data->x = x_maprequest; // y value of maprequest object - if (!zxy_maprequest->Has(Nan::New("y").ToLocalChecked())) + if (!zxy_maprequest.Has(Napi::String::New(info.Env(), "y"))) { - return utils::CallbackError("item in 'tiles' array does not include a 'y' value", callback); + return utils::CallbackError("item in 'tiles' array does not include a 'y' value", info, callback); } - v8::Local y_val_maprequest = zxy_maprequest->Get(Nan::New("y").ToLocalChecked()); - if (!y_val_maprequest->IsInt32()) + Napi::Value y_val_maprequest = zxy_maprequest.Get(Napi::String::New(info.Env(), "y")); + if (!y_val_maprequest.IsNumber()) { - return utils::CallbackError("'y' value in 'tiles' array item is not an int32", callback); + return utils::CallbackError("'y' value in 'tiles' array item is not an int32", info, callback); } - int y_maprequest = y_val_maprequest->Int32Value(); + int y_maprequest = y_val_maprequest.As().Int32Value(); if (y_maprequest < 0) { - return utils::CallbackError("'y' value must not be less than zero", callback); + return utils::CallbackError("'y' value must not be less than zero", info, callback); } baton_data->y = y_maprequest; if (info.Length() > 3) // options { - if (!info[2]->IsObject()) + if (!info[2].IsObject()) { - return utils::CallbackError("'options' arg must be an object", callback); + return utils::CallbackError("'options' arg must be an object", info, callback); } - v8::Local options = info[2]->ToObject(); - if (options->Has(Nan::New("buffer_size").ToLocalChecked())) + Napi::Object options = info[2].ToObject(); + if (options.Has(Napi::String::New(info.Env(), "buffer_size"))) { - v8::Local bs_value = options->Get(Nan::New("buffer_size").ToLocalChecked()); - if (!bs_value->IsInt32()) + Napi::Value bs_value = options.Get(Napi::String::New(info.Env(), "buffer_size")); + if (!bs_value.IsNumber()) { - return utils::CallbackError("'buffer_size' must be an int32", callback); + return utils::CallbackError("'buffer_size' must be an int32", info, callback); } - int buffer_size = bs_value->Int32Value(); + int buffer_size = bs_value.As().Int32Value(); if (buffer_size < 0) { - return utils::CallbackError("'buffer_size' must be a positive int32", callback); + return utils::CallbackError("'buffer_size' must be a positive int32", info, callback); } baton_data->buffer_size = buffer_size; } - if (options->Has(Nan::New("compress").ToLocalChecked())) + if (options.Has(Napi::String::New(info.Env(), "compress"))) { - v8::Local comp_value = options->Get(Nan::New("compress").ToLocalChecked()); - if (!comp_value->IsBoolean()) + Napi::Value comp_value = options.Get(Napi::String::New(info.Env(), "compress")); + if (!comp_value.IsBoolean()) { - return utils::CallbackError("'compress' must be a boolean", callback); + return utils::CallbackError("'compress' must be a boolean", info, callback); } - baton_data->compress = comp_value->BooleanValue(); + baton_data->compress = comp_value.As().Value(); } } - // enter the threadpool, then done in the callback function call the threadpool - auto* worker = new CompositeWorker{std::move(baton_data), new Nan::Callback{callback}}; - Nan::AsyncQueueWorker(worker); + auto* worker = new CompositeWorker{std::move(baton_data), callback}; + worker->Queue(); + return info.Env().Undefined(); } - } // namespace vtile diff --git a/src/vtcomposite.hpp b/src/vtcomposite.hpp index a767be0..0e11747 100644 --- a/src/vtcomposite.hpp +++ b/src/vtcomposite.hpp @@ -1,8 +1,9 @@ #pragma once -#include +#include +#include namespace vtile { -NAN_METHOD(composite); +Napi::Value composite(const Napi::CallbackInfo& info); } // namespace vtile diff --git a/test/vtcomposite-param-validation.test.js b/test/vtcomposite-param-validation.test.js index 841cf4e..9cdefea 100644 --- a/test/vtcomposite-param-validation.test.js +++ b/test/vtcomposite-param-validation.test.js @@ -7,8 +7,7 @@ var mvtFixtures = require('@mapbox/mvt-fixtures'); test('failure: fails without callback function', assert => { try { - -composite(); + composite(); } catch(err) { assert.ok(/last argument must be a callback function/.test(err.message), 'expected error message'); assert.end(); From f2c836027c83fd66027cd6b45486b5baed5797d1 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Thu, 25 Oct 2018 14:47:47 +0100 Subject: [PATCH 28/39] check CallbackInfo index is valid + format/tidy --- src/vtcomposite.cpp | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/vtcomposite.cpp b/src/vtcomposite.cpp index 51f554a..335aa14 100644 --- a/src/vtcomposite.cpp +++ b/src/vtcomposite.cpp @@ -1,8 +1,8 @@ // vtcomposite #include "vtcomposite.hpp" +#include "feature_builder.hpp" #include "module_utils.hpp" #include "zxy_math.hpp" -#include "feature_builder.hpp" // gzip-hpp #include #include @@ -11,9 +11,9 @@ #include #include // geometry.hpp +#include #include #include -#include // stl #include @@ -76,7 +76,7 @@ namespace { template struct build_feature_from_v1 { - build_feature_from_v1(FeatureBuilder& builder) + explicit build_feature_from_v1(FeatureBuilder& builder) : builder_(builder) {} bool operator()(vtzero::feature const& feature) @@ -97,7 +97,7 @@ struct build_feature_from_v1 template struct build_feature_from_v2 { - build_feature_from_v2(FeatureBuilder& builder) + explicit build_feature_from_v2(FeatureBuilder& builder) : builder_(builder) {} bool operator()(vtzero::feature const& feature) @@ -197,7 +197,7 @@ struct CompositeWorker : Napi::AsyncWorker throw std::invalid_argument(os.str()); } } - std::string& tile_buffer = *output_buffer_.get(); + std::string& tile_buffer = *output_buffer_; if (baton_data_->compress) { std::string temp; @@ -218,7 +218,7 @@ struct CompositeWorker : Napi::AsyncWorker } void OnOK() override { - std::string& tile_buffer = *output_buffer_.get(); + std::string& tile_buffer = *output_buffer_; Napi::HandleScope scope(Env()); Napi::Value argv = Napi::Buffer::New(Env(), const_cast(tile_buffer.data()), @@ -238,7 +238,13 @@ struct CompositeWorker : Napi::AsyncWorker Napi::Value composite(Napi::CallbackInfo const& info) { // validate callback function - Napi::Value callback_val = info[info.Length() - 1]; + std::size_t length = info.Length(); + if (length == 0) + { + Napi::Error::New(info.Env(), "last argument must be a callback function").ThrowAsJavaScriptException(); + return info.Env().Null(); + } + Napi::Value callback_val = info[length - 1]; if (!callback_val.IsFunction()) { Napi::Error::New(info.Env(), "last argument must be a callback function").ThrowAsJavaScriptException(); From f1765fdce6f65bab49e60d5193f18a3891872399 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Thu, 18 Oct 2018 11:58:27 +0100 Subject: [PATCH 29/39] attempting to fix .travis.yml (/home/travis/.travis/job_stages: line 373: exit: : numeric argument required) --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 35a2f7b..206f01d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -120,7 +120,7 @@ matrix: install: - make ${BUILDTYPE} # Overrides `script` to disable publishing - script: [] + script: # Coverage build - os: linux env: BUILDTYPE=debug CXXFLAGS="--coverage" LDFLAGS="--coverage" From 7aabb89d9549bb9e9beca199464fa1d1934f1643 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Thu, 25 Oct 2018 16:22:35 +0100 Subject: [PATCH 30/39] Perhaps more generic approach to obtaining 'node-addon-api' include path --- binding.gyp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/binding.gyp b/binding.gyp index 4ec0037..d85699c 100644 --- a/binding.gyp +++ b/binding.gyp @@ -15,7 +15,7 @@ # It's a variable to make easy to pass to # cflags (linux) and xcode (mac) 'system_includes': [ - "-isystem <(module_root_dir)/node_modules/node-addon-api/", + "-isystem Date: Tue, 30 Oct 2018 15:18:02 +0000 Subject: [PATCH 31/39] remove `` --- src/module.cpp | 1 - src/module_utils.hpp | 1 - src/vtcomposite.hpp | 1 - 3 files changed, 3 deletions(-) diff --git a/src/module.cpp b/src/module.cpp index d86ccc1..72b9e37 100644 --- a/src/module.cpp +++ b/src/module.cpp @@ -1,6 +1,5 @@ #include "vtcomposite.hpp" #include -#include Napi::Object init(Napi::Env env, Napi::Object exports) { diff --git a/src/module_utils.hpp b/src/module_utils.hpp index cb363ad..ad1c3be 100644 --- a/src/module_utils.hpp +++ b/src/module_utils.hpp @@ -1,6 +1,5 @@ #pragma once #include -#include namespace utils { diff --git a/src/vtcomposite.hpp b/src/vtcomposite.hpp index 0e11747..7420f7e 100644 --- a/src/vtcomposite.hpp +++ b/src/vtcomposite.hpp @@ -1,6 +1,5 @@ #pragma once #include -#include namespace vtile { From 4efb3561fb5e2e69b5b6766b490b6af00acf5a53 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Wed, 31 Oct 2018 13:12:48 +0000 Subject: [PATCH 32/39] Simplify `CallbackError` signature --- src/module_utils.hpp | 12 ++------- src/vtcomposite.cpp | 58 ++++++++++++++++++++++---------------------- 2 files changed, 31 insertions(+), 39 deletions(-) diff --git a/src/module_utils.hpp b/src/module_utils.hpp index ad1c3be..d0a8f73 100644 --- a/src/module_utils.hpp +++ b/src/module_utils.hpp @@ -3,19 +3,11 @@ namespace utils { -/* -* This is an internal function used to return callback error messages instead of -* throwing errors. -* Usage: -* -* Napi::Function callback; -* return CallbackError("error message", callback); -*/ - -inline Napi::Value CallbackError(std::string const& message, Napi::CallbackInfo const& info, Napi::Function const& func) +inline Napi::Value CallbackError(std::string const& message, Napi::CallbackInfo const& info) { Napi::Object obj = Napi::Object::New(info.Env()); obj.Set("message", message); + auto func = info[info.Length() - 1].As(); return func.Call({obj}); } } // namespace utils diff --git a/src/vtcomposite.cpp b/src/vtcomposite.cpp index 335aa14..b6f68d8 100644 --- a/src/vtcomposite.cpp +++ b/src/vtcomposite.cpp @@ -256,7 +256,7 @@ Napi::Value composite(Napi::CallbackInfo const& info) // validate tiles if (!info[0].IsArray()) { - return utils::CallbackError("first arg 'tiles' must be an array of tile objects", info, callback); + return utils::CallbackError("first arg 'tiles' must be an array of tile objects", info); } Napi::Array tiles = info[0].As(); @@ -264,7 +264,7 @@ Napi::Value composite(Napi::CallbackInfo const& info) if (num_tiles <= 0) { - return utils::CallbackError("'tiles' array must be of length greater than 0", info, callback); + return utils::CallbackError("'tiles' array must be of length greater than 0", info); } std::unique_ptr baton_data = std::make_unique(num_tiles); @@ -274,72 +274,72 @@ Napi::Value composite(Napi::CallbackInfo const& info) Napi::Value tile_val = tiles.Get(t); if (!tile_val.IsObject()) { - return utils::CallbackError("items in 'tiles' array must be objects", info, callback); + return utils::CallbackError("items in 'tiles' array must be objects", info); } Napi::Object tile_obj = tile_val.ToObject(); // check buffer value if (!tile_obj.Has(Napi::String::New(info.Env(), "buffer"))) { - return utils::CallbackError("item in 'tiles' array does not include a buffer value", info, callback); + return utils::CallbackError("item in 'tiles' array does not include a buffer value", info); } Napi::Value buf_val = tile_obj.Get(Napi::String::New(info.Env(), "buffer")); if (buf_val.IsNull() || buf_val.IsUndefined()) { - return utils::CallbackError("buffer value in 'tiles' array item is null or undefined", info, callback); + return utils::CallbackError("buffer value in 'tiles' array item is null or undefined", info); } Napi::Object buffer = buf_val.ToObject(); if (!buffer.IsBuffer()) { - return utils::CallbackError("buffer value in 'tiles' array item is not a true buffer", info, callback); + return utils::CallbackError("buffer value in 'tiles' array item is not a true buffer", info); } // z value if (!tile_obj.Has(Napi::String::New(info.Env(), "z"))) { - return utils::CallbackError("item in 'tiles' array does not include a 'z' value", info, callback); + return utils::CallbackError("item in 'tiles' array does not include a 'z' value", info); } Napi::Value z_val = tile_obj.Get(Napi::String::New(info.Env(), "z")); if (!z_val.IsNumber()) { - return utils::CallbackError("'z' value in 'tiles' array item is not an int32", info, callback); + return utils::CallbackError("'z' value in 'tiles' array item is not an int32", info); } int z = z_val.As().Int32Value(); if (z < 0) { - return utils::CallbackError("'z' value must not be less than zero", info, callback); + return utils::CallbackError("'z' value must not be less than zero", info); } // x value if (!tile_obj.Has(Napi::String::New(info.Env(), "x"))) { - return utils::CallbackError("item in 'tiles' array does not include a 'x' value", info, callback); + return utils::CallbackError("item in 'tiles' array does not include a 'x' value", info); } Napi::Value x_val = tile_obj.Get(Napi::String::New(info.Env(), "x")); if (!x_val.IsNumber()) { - return utils::CallbackError("'x' value in 'tiles' array item is not an int32", info, callback); + return utils::CallbackError("'x' value in 'tiles' array item is not an int32", info); } int x = x_val.As().Int32Value(); if (x < 0) { - return utils::CallbackError("'x' value must not be less than zero", info, callback); + return utils::CallbackError("'x' value must not be less than zero", info); } // y value if (!tile_obj.Has(Napi::String::New(info.Env(), "y"))) { - return utils::CallbackError("item in 'tiles' array does not include a 'y' value", info, callback); + return utils::CallbackError("item in 'tiles' array does not include a 'y' value", info); } Napi::Value y_val = tile_obj.Get(Napi::String::New(info.Env(), "y")); if (!y_val.IsNumber()) { - return utils::CallbackError("'y' value in 'tiles' array item is not an int32", info, callback); + return utils::CallbackError("'y' value in 'tiles' array item is not an int32", info); } int y = y_val.As().Int32Value(); if (y < 0) { - return utils::CallbackError("'y' value must not be less than zero", info, callback); + return utils::CallbackError("'y' value must not be less than zero", info); } baton_data->tiles.push_back(std::make_unique(z, x, y, buffer)); } @@ -347,41 +347,41 @@ Napi::Value composite(Napi::CallbackInfo const& info) //validate zxy maprequest object if (!info[1].IsObject()) { - return utils::CallbackError("'zxy_maprequest' must be an object", info, callback); + return utils::CallbackError("'zxy_maprequest' must be an object", info); } Napi::Object zxy_maprequest = info[1].As(); // z value of map request object if (!zxy_maprequest.Has(Napi::String::New(info.Env(), "z"))) { - return utils::CallbackError("item in 'tiles' array does not include a 'z' value", info, callback); + return utils::CallbackError("item in 'tiles' array does not include a 'z' value", info); } Napi::Value z_val_maprequest = zxy_maprequest.Get(Napi::String::New(info.Env(), "z")); if (!z_val_maprequest.IsNumber()) { - return utils::CallbackError("'z' value in 'tiles' array item is not an int32", info, callback); + return utils::CallbackError("'z' value in 'tiles' array item is not an int32", info); } int z_maprequest = z_val_maprequest.As().Int32Value(); if (z_maprequest < 0) { - return utils::CallbackError("'z' value must not be less than zero", info, callback); + return utils::CallbackError("'z' value must not be less than zero", info); } baton_data->z = z_maprequest; // x value of map request object if (!zxy_maprequest.Has(Napi::String::New(info.Env(), "x"))) { - return utils::CallbackError("item in 'tiles' array does not include a 'x' value", info, callback); + return utils::CallbackError("item in 'tiles' array does not include a 'x' value", info); } Napi::Value x_val_maprequest = zxy_maprequest.Get(Napi::String::New(info.Env(), "x")); if (!x_val_maprequest.IsNumber()) { - return utils::CallbackError("'x' value in 'tiles' array item is not an int32", info, callback); + return utils::CallbackError("'x' value in 'tiles' array item is not an int32", info); } int x_maprequest = x_val_maprequest.As().Int32Value(); if (x_maprequest < 0) { - return utils::CallbackError("'x' value must not be less than zero", info, callback); + return utils::CallbackError("'x' value must not be less than zero", info); } baton_data->x = x_maprequest; @@ -389,17 +389,17 @@ Napi::Value composite(Napi::CallbackInfo const& info) // y value of maprequest object if (!zxy_maprequest.Has(Napi::String::New(info.Env(), "y"))) { - return utils::CallbackError("item in 'tiles' array does not include a 'y' value", info, callback); + return utils::CallbackError("item in 'tiles' array does not include a 'y' value", info); } Napi::Value y_val_maprequest = zxy_maprequest.Get(Napi::String::New(info.Env(), "y")); if (!y_val_maprequest.IsNumber()) { - return utils::CallbackError("'y' value in 'tiles' array item is not an int32", info, callback); + return utils::CallbackError("'y' value in 'tiles' array item is not an int32", info); } int y_maprequest = y_val_maprequest.As().Int32Value(); if (y_maprequest < 0) { - return utils::CallbackError("'y' value must not be less than zero", info, callback); + return utils::CallbackError("'y' value must not be less than zero", info); } baton_data->y = y_maprequest; @@ -408,7 +408,7 @@ Napi::Value composite(Napi::CallbackInfo const& info) { if (!info[2].IsObject()) { - return utils::CallbackError("'options' arg must be an object", info, callback); + return utils::CallbackError("'options' arg must be an object", info); } Napi::Object options = info[2].ToObject(); if (options.Has(Napi::String::New(info.Env(), "buffer_size"))) @@ -416,13 +416,13 @@ Napi::Value composite(Napi::CallbackInfo const& info) Napi::Value bs_value = options.Get(Napi::String::New(info.Env(), "buffer_size")); if (!bs_value.IsNumber()) { - return utils::CallbackError("'buffer_size' must be an int32", info, callback); + return utils::CallbackError("'buffer_size' must be an int32", info); } int buffer_size = bs_value.As().Int32Value(); if (buffer_size < 0) { - return utils::CallbackError("'buffer_size' must be a positive int32", info, callback); + return utils::CallbackError("'buffer_size' must be a positive int32", info); } baton_data->buffer_size = buffer_size; } @@ -431,7 +431,7 @@ Napi::Value composite(Napi::CallbackInfo const& info) Napi::Value comp_value = options.Get(Napi::String::New(info.Env(), "compress")); if (!comp_value.IsBoolean()) { - return utils::CallbackError("'compress' must be a boolean", info, callback); + return utils::CallbackError("'compress' must be a boolean", info); } baton_data->compress = comp_value.As().Value(); } From ef12486c027242157c3d13898741b58376575616 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Mon, 5 Nov 2018 17:50:58 +0000 Subject: [PATCH 33/39] clang-tidy --- src/vtcomposite.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/vtcomposite.cpp b/src/vtcomposite.cpp index 4511813..bf40800 100644 --- a/src/vtcomposite.cpp +++ b/src/vtcomposite.cpp @@ -1,10 +1,9 @@ // vtcomposite #include "vtcomposite.hpp" +#include "deflate.hpp" #include "feature_builder.hpp" #include "module_utils.hpp" #include "zxy_math.hpp" -#include "feature_builder.hpp" -#include "deflate.hpp" // vtzero #include #include From b39a4418b13fd25c699be89a3fde8095893abd59 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Fri, 9 Nov 2018 16:27:58 +0000 Subject: [PATCH 34/39] bump version to v0.2.0 [publish binary] --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 10b173f..bc18c8e 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@mapbox/vtcomposite", - "version": "0.1.1", + "version": "0.2.0", "description": "Compositing operations on Vector Tiles (c++ bindings using N-API)", "url": "http://github.com/mapbox/vtcomposite", "main": "./lib/index.js", From f0016e34c63fcaa4207018cd48a55b6cbbab591b Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Mon, 12 Nov 2018 10:44:37 +0000 Subject: [PATCH 35/39] refactor decompression logic to use output buffer size (not capacity) + 2x output size on LIBDEFLATE_INSUFFICIENT_SPACE + removed LIBDEFLATE_SHORT_OUTPUT case as we're always passing non NULL actual_size [publish binary] --- src/deflate.hpp | 32 +++++++++++++------------------- src/vtcomposite.hpp | 2 +- 2 files changed, 14 insertions(+), 20 deletions(-) diff --git a/src/deflate.hpp b/src/deflate.hpp index 694dec5..b9193e1 100644 --- a/src/deflate.hpp +++ b/src/deflate.hpp @@ -141,31 +141,25 @@ class Decompressor size, const_cast(output.data()), output.size(), &actual_size); - if (result != LIBDEFLATE_INSUFFICIENT_SPACE) + if (result == LIBDEFLATE_SUCCESS) { + output.resize(actual_size); break; } - if (output.size() == max_) + else if (result == LIBDEFLATE_INSUFFICIENT_SPACE) { - throw std::runtime_error("request to resize output buffer can't exceed maximum limit"); + if (output.size() == max_) + { + throw std::runtime_error("request to resize output buffer can't exceed maximum limit"); + } + std::size_t new_size = std::min(output.size() << 1, max_); + output.resize(new_size); + } + else //LIBDEFLATE_BAD_DATA + { + throw std::runtime_error("bad data: did not succeed"); } - std::size_t new_size = std::min((output.capacity() << 1) - output.size(), max_); - output.resize(new_size); - } - - if (result == LIBDEFLATE_SHORT_OUTPUT) - { - throw std::runtime_error("short output: did not succeed"); - } - else if (result == LIBDEFLATE_BAD_DATA) - { - throw std::runtime_error("bad data: did not succeed"); - } - else if (result != LIBDEFLATE_SUCCESS) - { - throw std::runtime_error("did not succeed"); } - output.resize(actual_size); } }; diff --git a/src/vtcomposite.hpp b/src/vtcomposite.hpp index 7420f7e..683b2c7 100644 --- a/src/vtcomposite.hpp +++ b/src/vtcomposite.hpp @@ -3,6 +3,6 @@ namespace vtile { -Napi::Value composite(const Napi::CallbackInfo& info); +Napi::Value composite(Napi::CallbackInfo const& info); } // namespace vtile From 3f8025861a28ef4ea69e0ecca59d80dee8cc6fc1 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Mon, 12 Nov 2018 10:59:07 +0000 Subject: [PATCH 36/39] bump version to v0.2.1 [publish binary] --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index bc18c8e..bc15aaa 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@mapbox/vtcomposite", - "version": "0.2.0", + "version": "0.2.1", "description": "Compositing operations on Vector Tiles (c++ bindings using N-API)", "url": "http://github.com/mapbox/vtcomposite", "main": "./lib/index.js", From ae290da8caa4a35acedd83eecd2f986bea8870b1 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Mon, 19 Nov 2018 13:01:32 +0000 Subject: [PATCH 37/39] use `As` consistently --- src/vtcomposite.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/vtcomposite.cpp b/src/vtcomposite.cpp index b6f68d8..04ead37 100644 --- a/src/vtcomposite.cpp +++ b/src/vtcomposite.cpp @@ -276,8 +276,8 @@ Napi::Value composite(Napi::CallbackInfo const& info) { return utils::CallbackError("items in 'tiles' array must be objects", info); } - Napi::Object tile_obj = tile_val.ToObject(); + Napi::Object tile_obj = tile_val.As(); // check buffer value if (!tile_obj.Has(Napi::String::New(info.Env(), "buffer"))) { @@ -288,7 +288,7 @@ Napi::Value composite(Napi::CallbackInfo const& info) { return utils::CallbackError("buffer value in 'tiles' array item is null or undefined", info); } - Napi::Object buffer = buf_val.ToObject(); + Napi::Object buffer = buf_val.As(); if (!buffer.IsBuffer()) { return utils::CallbackError("buffer value in 'tiles' array item is not a true buffer", info); @@ -410,7 +410,7 @@ Napi::Value composite(Napi::CallbackInfo const& info) { return utils::CallbackError("'options' arg must be an object", info); } - Napi::Object options = info[2].ToObject(); + Napi::Object options = info[2].As(); if (options.Has(Napi::String::New(info.Env(), "buffer_size"))) { Napi::Value bs_value = options.Get(Napi::String::New(info.Env(), "buffer_size")); From 93806a9dcff7ec26a245078bc1dda7db20f8b4d5 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Mon, 19 Nov 2018 16:07:55 +0000 Subject: [PATCH 38/39] remove redundant conversion --- src/vtcomposite.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vtcomposite.cpp b/src/vtcomposite.cpp index 04ead37..10dd584 100644 --- a/src/vtcomposite.cpp +++ b/src/vtcomposite.cpp @@ -31,7 +31,7 @@ struct TileObject x{x0}, y{y0}, data{buffer.As>().Data(), buffer.As>().Length()}, - buffer_ref{Napi::Persistent(buffer.As())} + buffer_ref{Napi::Persistent(buffer)} { } From 4f508969bd085ed50813b54360efbe20e52a53cf Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Mon, 19 Nov 2018 16:38:21 +0000 Subject: [PATCH 39/39] Minimise internal type conversions by passing `Napi::Buffer` and storing `Napi::Reference>` in TileObject. --- src/vtcomposite.cpp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/vtcomposite.cpp b/src/vtcomposite.cpp index 10dd584..476efd3 100644 --- a/src/vtcomposite.cpp +++ b/src/vtcomposite.cpp @@ -26,11 +26,11 @@ struct TileObject TileObject(int z0, int x0, int y0, - Napi::Object& buffer) + Napi::Buffer const& buffer) : z{z0}, x{x0}, y{y0}, - data{buffer.As>().Data(), buffer.As>().Length()}, + data{buffer.Data(), buffer.Length()}, buffer_ref{Napi::Persistent(buffer)} { } @@ -48,7 +48,7 @@ struct TileObject int x; int y; vtzero::data_view data; - Napi::ObjectReference buffer_ref; + Napi::Reference> buffer_ref; }; struct BatonType @@ -288,12 +288,13 @@ Napi::Value composite(Napi::CallbackInfo const& info) { return utils::CallbackError("buffer value in 'tiles' array item is null or undefined", info); } - Napi::Object buffer = buf_val.As(); - if (!buffer.IsBuffer()) + Napi::Object buffer_obj = buf_val.As(); + if (!buffer_obj.IsBuffer()) { return utils::CallbackError("buffer value in 'tiles' array item is not a true buffer", info); } + Napi::Buffer buffer = buffer_obj.As>(); // z value if (!tile_obj.Has(Napi::String::New(info.Env(), "z"))) { @@ -341,6 +342,7 @@ Napi::Value composite(Napi::CallbackInfo const& info) { return utils::CallbackError("'y' value must not be less than zero", info); } + baton_data->tiles.push_back(std::make_unique(z, x, y, buffer)); }