mapbox / vtshaver Goto Github PK

Remove unneeded vector tile layers, features, and properties based on particular style

License: BSD 2-Clause "Simplified" License

Makefile 2.66% JavaScript 58.54% Python 7.69% Shell 8.12% C++ 23.00%

style-optimized vector-tiles node cpp filter mapbox-gl

vtshaver's Introduction

Vector Tile Shaver

Style-optimized vector tiles. The shaver takes a Mapbox Vector Tile and a Mapbox GL Style and removes layers, features, and properties in the tile that are not used by the style to reduce the size of the tile. Read the feature release blog post and the api-documentation for more info.

Installation

npm install @mapbox/vtshaver

If you want to install locally you can also do:

git clone https://github.com/mapbox/vtshaver
cd vtshaver
npm install

API Usage

CLI

Shaver provides 2 command line tools:

vtshave

vtshave [args]

  --tile:    required: path to the input vector tile
  --style:   required: path to a gl style to use to shave
  --zoom:    required: the zoom level
  --maxzoom: optional: the maxzoom of a tileset relevant to the tile buffer being shaved
  --out:     optional: pass a path if you want the shaved tile to be saved

Will output a size comparison of how many bytes were shaved off the tile.

Example:

  vtshave --tile tile.mvt --zoom 0 --maxzoom 16 --style style.json

vtshaver-filters

vtshaver-filters [args]

  --style:   required: path to a gl style to parse
  --sources: optional: list of one or more sources (comma separated) to display in the output (default is all sources)
  --pretty:  optional: whether to pretty print the output (default false). Pass '--pretty' to indent the JSON.

Will output a json object describing each of the source-layers and their parsed metadata to be used for shaving.

Example:

  vtshaver-filters --style style.json > meta.json

Develop

Build binaries

make

For Mac M1 users, there are a couple of extra steps before building

Comment out linking instructions in your local binding.gyp as follows

#  'make_global_settings': [
#    ['CXX', '<(module_root_dir)/mason_packages/.link/bin/clang++'],
#    ['CC', '<(module_root_dir)/mason_packages/.link/bin/clang'],
#    ['LINK', '<(module_root_dir)/mason_packages/.link/bin/clang++'],
#    ['AR', '<(module_root_dir)/mason_packages/.link/bin/llvm-ar'],
#    ['NM', '<(module_root_dir)/mason_packages/.link/bin/llvm-nm']
#  ],

Switch to x86_64 processor since arm64 has unresolved issues with the latest mbgl-core library

$ arch --x86_64 zsh

Test

make test

Run bench test

node bench/bench-batch.js --iterations 50 --concurrency 10

Optionally combine with the time command

Docs

Documentation is generated using Documentation.js --polyglot mode. Generate docs in API.md by running:

make docs

NOTE: we are pinned to [email protected] because 5.x removed C++ support: https://github.com/documentationjs/documentation/blob/master/CHANGELOG.md#500-2017-07-27

vtshaver's People

Contributors

Stargazers

Watchers

Forkers

lukaswelte rdmurphy tompohys isabella232 cuulee mapclone zelonewolf ryanmjacobs

vtshaver's Issues

Move to using `isExpressionFilter`

We can likely use isExpressionFilter at

vtshaver/lib/styleToFilters.js

Lines 82 to 112 in b0d8eb4

 function getPropertyFromFilter(filter, properties) { 

 if (styleSpec.expression.isExpression(filter)) { 

 getPropertyFromExpression(filter, properties); 

 } 

 // Warning: Below code should put in to an else conditions, 

 // but since the `isExpression` can not tell it is a expression or filter syntax I put it outsied the else 

 // this could reduce the performance or cause some potential bugs, we must keep an eye on this. 

 // else { 

 let subFilter = []; 

 for (let i = 0; i < filter.length; i++) { 

 if (typeof filter[i] === 'object' && filter[i] instanceof Array) { 

 subFilter.push(filter[i]); 

 } 

 } 

 if (subFilter.length > 0) { 

 subFilter.forEach(sfilter => { 

 getPropertyFromFilter(sfilter, properties); 

 }) 

 } else { 

 if (filter.length >= 3 && typeof filter[1] === 'string') { 

 if (filter[1].indexOf('$') === -1) { 

 properties.push(filter[1]); 

 } 

 } 

 } 

 // } 

 }

to clean up that code and avoid the commented else block.

This is getting exposed at mapbox/mapbox-gl-js#9530

Add CLI tests

The CLI could use some tests

Next Actions

Update bin/vtshave.js to accept a --filter parameter, which will be a new Shaver.Filter object.
Create test/cli.test.js and add unit tests for bin/vtshave.js (See vtvalidate's CLI tests for reference)

Idea: CLI supporting raw filters json

Per @springmeyer 's thoughts:

The CLI currently expects a --style flag that reads a mbgl style that is then run through /lib/styleToFilters.js

I think it would be ideal to also:

Have the CLI accept an option like --filters that would be the JSON schema which new shaver.Filter expects (in place of --style)
Have this "Filters" JSON schema clearly documented in the readme so that it is obvious how to use shaver without needing to form up a full mbgl style.

This could be useful for testing and finding lurking parsing bugs.

refs

vtshaver/src/filters.cpp

Line 42 in da0b5ba

NAN_METHOD(Filters::New) {

Supporting runtime style

Even though vtshaver is not designing for runtime style, we still want to make efforts to make things better.

Now we have two experimental ideas. This ticket is to track the process of support runtime style.

Style specific retain, we want to make some special marks in style specific, to mark which properties in a particular layer we want to use, then the vtshaver will keep the certain properties.
Query properties, vtshaver well accept a param to determine which properties the user want to use, then return the shaved tiles+the certain properties. For the client side, every time user wants to change the style, the client can access tile with the certain properties in query(like vector.pbf?styl=<path to style>&keep={"country_label" : ["name_ru", "name_de", "name_es"]}).

cc w/ @springmeyer

`napi_get_property_names` is buggy in node 10.19

napi_get_property_names(_env, _value, &result);

erroneously returns Number type when property name has only digit literals e.g '123'

This has been fixed in node >=12 (tested 12.16.1, 13.12.0)
/cc @springmeyer

Optimization idea: avoid serialize and compress if we've shaved nothing

Per @springmeyer 's thoughts:

A potential optimization to look into would be:

Track if we don't ever shave any data off a tile
In that case, return early without serializing the newly created tile

The early return would save the call to finalvt.serialize and to compression (if compression is requested). The largest performance gain would likely come from skipping compression (if it is requested) since we've learned from profiling in api-maps[citation needed] that compression of large tiles can be more expensive than the vtile encoding of layers+features.

So, we'd be able to skip these calls in the case that we've not actually been able to shave any data

vtshaver/src/shave.cpp

Lines 405 to 410 in 809ba39

 if (baton->compress) { 

 // Compress final tile before sending back 

 std::string final_data; 

 finalvt.serialize(final_data); 

 gzip::Compressor compressor; 

 compressor.compress(*baton->shaved_tile, final_data.data(), final_data.size());

. In this case we could return nothing for the resultant buffer as an indication that the caller should simply re-use the data passed into the shaver for that tile.

I'm basing this optimization idea on the assumption that, while potentially uncommon or rare, it is possible (and frequent enough to matter for large scale performance) that you might have a vector tileset with some tiles which get a lot shaved off and some tiles that get nothing shaved off.

Before ever moving on this feature however we should do some research to figure out of this "did not shave anything" scenario actually happens in reality and if it does whether it happens with data the is large enough to have measurable cost to serialize and compress. If these things don't happen then this optimization would likely have no benefit.

Port to NAPI

Like mapbox/vtcomposite#90 we should port to NAPI to make upgrades easier in the future.

Fix last clang-tidy warnings

Per @springmeyer and @joto 's thoughts:

We have a few clang-tidy warnings needing fixed.

They should be failing the build, but are not due to mapbox/node-cpp-skel#105.

These are what show up for me with make tidy locally on OS X:

../src/filters.cpp:50:13: error: initializing non-owner 'Filters *const' with a newly created 'gsl::owner<>' [cppcoreguidelines-owning-memory,-warnings-as-errors]
            auto* const self = new Filters();
            ^
../src/shave.cpp:152:9: error: initializing non-owner 'AsyncBaton *' with a newly created 'gsl::owner<>' [cppcoreguidelines-owning-memory,-warnings-as-errors]
        auto* baton = new AsyncBaton();
        ^
../src/shave.cpp:342:60: error: deleting a pointer through a type that is not marked 'gsl::owner<>'; consider using a smart pointer instead [cppcoreguidelines-owning-memory,-warnings-as-errors]
                                                           delete reinterpret_cast<std::string*>(hint);
                                                           ^
../src/shave.cpp:341:66: note: variable declared here
                                                       [](char*, void* hint) {
                                                                 ^
../src/shave.cpp:353:5: error: deleting a pointer through a type that is not marked 'gsl::owner<>'; consider using a smart pointer instead [cppcoreguidelines-owning-memory,-warnings-as-errors]
    delete baton;
    ^
../src/shave.cpp:329:5: note: variable declared here
    auto* baton = static_cast<AsyncBaton*>(req->data);
    ^

Now, I noticed that @joto just disabled this clang-tidy warning (`cppcoreguidelines-owning-memory) today in vtzero which indicates to be we may want to consider the same thing: mapbox/vtzero@6822c59

I think the cppcoreguidelines-owning-memory only makes sense if you buy into the whole gsl library. Each project has to decide whether it makes sense for them, but I think the gsl library makes more sense for higher level applications while lower-level libraries a) need more bit-twiddling and pointer-twiddling etc. anyway so they need to disable lots of these warnings and b) any extra dependency on low-level libraries makes they use that much harder.

CLI to include compress option

Add the compress option to the CLI script. This option currently exists in the API, but is not yet enabled in the vtshave CLI

Install error with Node 12

I ended up getting the install from NPM to work with Node 10. If you'd like, you can close this issue.

Error when installing from NPM:

> npm install @mapbox/vtshaver

> @mapbox/[email protected] install /Users/kyle/tmp/node_modules/@mapbox/vtshaver
> node-pre-gyp install --fallback-to-build

node-pre-gyp WARN Using request for node-pre-gyp https download
node-pre-gyp WARN Tried to download(403): https://mapbox-node-binary.s3.amazonaws.com/@mapbox/vtshaver/v0.2.1/Release/node-v72-darwin-x64.tar.gz
node-pre-gyp WARN Pre-built binaries not found for @mapbox/[email protected] and [email protected] (node-v72 ABI, unknown) (falling back to source compile with node-gyp)
make: *** No rule to make target `../node_modules/.bin/mason-js', needed by `mason_packages'.  Stop.
gyp ERR! build error
gyp ERR! stack Error: `make` failed with exit code: 2
gyp ERR! stack     at ChildProcess.onExit (/Users/kyle/.nvm/versions/node/v12.13.1/lib/node_modules/npm/node_modules/npm-lifecycle/node_modules/node-gyp/lib/build.js:194:23)
gyp ERR! stack     at ChildProcess.emit (events.js:210:5)
gyp ERR! stack     at Process.ChildProcess._handle.onexit (internal/child_process.js:272:12)
gyp ERR! System Darwin 19.0.0
gyp ERR! command "/Users/kyle/.nvm/versions/node/v12.13.1/bin/node" "/Users/kyle/.nvm/versions/node/v12.13.1/lib/node_modules/npm/node_modules/npm-lifecycle/node_modules/node-gyp/bin/node-gyp.js" "build" "--fallback-to-build" "--module=/Users/kyle/tmp/node_modules/@mapbox/vtshaver/lib/binding/vtshaver.node" "--module_name=vtshaver" "--module_path=/Users/kyle/tmp/node_modules/@mapbox/vtshaver/lib/binding" "--napi_version=5" "--node_abi_napi=napi" "--napi_build_version=0" "--node_napi_label=node-v72"
gyp ERR! cwd /Users/kyle/tmp/node_modules/@mapbox/vtshaver
gyp ERR! node -v v12.13.1
gyp ERR! node-gyp -v v6.0.1
gyp ERR! not ok
node-pre-gyp ERR! build error
node-pre-gyp ERR! stack Error: Failed to execute '/Users/kyle/.nvm/versions/node/v12.13.1/bin/node /Users/kyle/.nvm/versions/node/v12.13.1/lib/node_modules/npm/node_modules/npm-lifecycle/node_modules/node-gyp/bin/node-gyp.js build --fallback-to-build --module=/Users/kyle/tmp/node_modules/@mapbox/vtshaver/lib/binding/vtshaver.node --module_name=vtshaver --module_path=/Users/kyle/tmp/node_modules/@mapbox/vtshaver/lib/binding --napi_version=5 --node_abi_napi=napi --napi_build_version=0 --node_napi_label=node-v72' (1)
node-pre-gyp ERR! stack     at ChildProcess.<anonymous> (/Users/kyle/tmp/node_modules/node-pre-gyp/lib/util/compile.js:83:29)
node-pre-gyp ERR! stack     at ChildProcess.emit (events.js:210:5)
node-pre-gyp ERR! stack     at maybeClose (internal/child_process.js:1021:16)
node-pre-gyp ERR! stack     at Process.ChildProcess._handle.onexit (internal/child_process.js:283:5)
node-pre-gyp ERR! System Darwin 19.0.0
node-pre-gyp ERR! command "/Users/kyle/.nvm/versions/node/v12.13.1/bin/node" "/Users/kyle/tmp/node_modules/.bin/node-pre-gyp" "install" "--fallback-to-build"
node-pre-gyp ERR! cwd /Users/kyle/tmp/node_modules/@mapbox/vtshaver
node-pre-gyp ERR! node -v v12.13.1
node-pre-gyp ERR! node-pre-gyp -v v0.12.0
node-pre-gyp ERR! not ok
Failed to execute '/Users/kyle/.nvm/versions/node/v12.13.1/bin/node /Users/kyle/.nvm/versions/node/v12.13.1/lib/node_modules/npm/node_modules/npm-lifecycle/node_modules/node-gyp/bin/node-gyp.js build --fallback-to-build --module=/Users/kyle/tmp/node_modules/@mapbox/vtshaver/lib/binding/vtshaver.node --module_name=vtshaver --module_path=/Users/kyle/tmp/node_modules/@mapbox/vtshaver/lib/binding --napi_version=5 --node_abi_napi=napi --napi_build_version=0 --node_napi_label=node-v72' (1)
npm WARN tmp No description
npm WARN tmp No repository field.
npm WARN tmp No license field.

npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! @mapbox/[email protected] install: `node-pre-gyp install --fallback-to-build`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the @mapbox/[email protected] install script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/kyle/.npm/_logs/2019-12-10T01_06_37_696Z-debug.log

Error when installing from git, tag v0.2.1

> npm install

> @mapbox/[email protected] install /Users/kyle/github/mapping/vtshaver
> node-pre-gyp install --fallback-to-build

node-pre-gyp WARN Using needle for node-pre-gyp https download
node-pre-gyp WARN Tried to download(403): https://mapbox-node-binary.s3.amazonaws.com/@mapbox/vtshaver/v0.2.1/Release/node-v72-darwin-x64.tar.gz
node-pre-gyp WARN Pre-built binaries not found for @mapbox/[email protected] and [email protected] (node-v72 ABI, unknown) (falling back to source compile with node-gyp)
  ACTION binding_gyp_action_before_build_target_install_deps mason_packages
info Mason Package Install Starting
info [/Users/kyle/github/mapping/vtshaver/mason_packages/headers/protozero/1.6.3] Success: protozero already installed
info [/Users/kyle/github/mapping/vtshaver/mason_packages/headers/rapidjson/1.1.0] Success: rapidjson already installed
info [/Users/kyle/github/mapping/vtshaver/mason_packages/headers/geojson/0.4.2] Success: geojson already installed
info [/Users/kyle/github/mapping/vtshaver/mason_packages/headers/geometry/0.9.2] Success: geometry already installed
info [/Users/kyle/github/mapping/vtshaver/mason_packages/headers/vtzero/1.0.0] Success: vtzero already installed
info [/Users/kyle/github/mapping/vtshaver/mason_packages/headers/variant/1.1.4] Success: variant already installed
info [/Users/kyle/github/mapping/vtshaver/mason_packages/headers/gzip-hpp/0.1.0] Success: gzip-hpp already installed
info [/Users/kyle/github/mapping/vtshaver/mason_packages/headers/vector-tile/1.0.1] Success: vector-tile already installed
info [/Users/kyle/github/mapping/vtshaver/mason_packages/headers/wagyu/0.4.3] Success: wagyu already installed
info [/Users/kyle/github/mapping/vtshaver/mason_packages/osx-x86_64/clang++/6.0.1] Success: clang++ already installed
info [/Users/kyle/github/mapping/vtshaver/mason_packages/osx-x86_64/clang-tidy/6.0.1] Success: clang-tidy already installed
info [/Users/kyle/github/mapping/vtshaver/mason_packages/osx-x86_64/clang-format/6.0.1] Success: clang-format already installed
info [/Users/kyle/github/mapping/vtshaver/mason_packages/osx-x86_64/llvm-cov/6.0.1] Success: llvm-cov already installed
info [/Users/kyle/github/mapping/vtshaver/mason_packages/osx-x86_64/binutils/2.31] Success: binutils already installed
info [/Users/kyle/github/mapping/vtshaver/mason_packages/osx-x86_64/mbgl-core/20f880e] Success: mbgl-core already installed
info [/Users/kyle/github/mapping/vtshaver/mason_packages/osx-x86_64/nunicode/1.8] Success: nunicode already installed
info [/Users/kyle/github/mapping/vtshaver/mason_packages/osx-x86_64/libpng/1.6.25] Success: libpng already installed
info [/Users/kyle/github/mapping/vtshaver/mason_packages/osx-x86_64/sqlite/3.14.2] Success: sqlite already installed
info [/Users/kyle/github/mapping/vtshaver/mason_packages/osx-x86_64/libjpeg-turbo/1.5.0] Success: libjpeg-turbo already installed
info [/Users/kyle/github/mapping/vtshaver/mason_packages/osx-x86_64/webp/0.5.1] Success: webp already installed
info [/Users/kyle/github/mapping/vtshaver/mason_packages/osx-x86_64/icu/58.1-min-size] Success: icu already installed
info Finished downloading packages
info Completed
  ACTION binding_gyp_action_before_build_target_link_deps mason_packages/.link
info Symlinked:  /Users/kyle/github/mapping/vtshaver/mason_packages/headers/protozero/1.6.3 to  /Users/kyle/github/mapping/vtshaver/mason_packages/.link
info Symlinked:  /Users/kyle/github/mapping/vtshaver/mason_packages/headers/rapidjson/1.1.0 to  /Users/kyle/github/mapping/vtshaver/mason_packages/.link
info Symlinked:  /Users/kyle/github/mapping/vtshaver/mason_packages/headers/geojson/0.4.2 to  /Users/kyle/github/mapping/vtshaver/mason_packages/.link
info Symlinked:  /Users/kyle/github/mapping/vtshaver/mason_packages/headers/geometry/0.9.2 to  /Users/kyle/github/mapping/vtshaver/mason_packages/.link
info Symlinked:  /Users/kyle/github/mapping/vtshaver/mason_packages/headers/vtzero/1.0.0 to  /Users/kyle/github/mapping/vtshaver/mason_packages/.link
info Symlinked:  /Users/kyle/github/mapping/vtshaver/mason_packages/headers/variant/1.1.4 to  /Users/kyle/github/mapping/vtshaver/mason_packages/.link
info Symlinked:  /Users/kyle/github/mapping/vtshaver/mason_packages/headers/gzip-hpp/0.1.0 to  /Users/kyle/github/mapping/vtshaver/mason_packages/.link
info Symlinked:  /Users/kyle/github/mapping/vtshaver/mason_packages/headers/vector-tile/1.0.1 to  /Users/kyle/github/mapping/vtshaver/mason_packages/.link
info Symlinked:  /Users/kyle/github/mapping/vtshaver/mason_packages/headers/wagyu/0.4.3 to  /Users/kyle/github/mapping/vtshaver/mason_packages/.link
info Symlinked:  /Users/kyle/github/mapping/vtshaver/mason_packages/osx-x86_64/clang++/6.0.1 to  /Users/kyle/github/mapping/vtshaver/mason_packages/.link
info Symlinked:  /Users/kyle/github/mapping/vtshaver/mason_packages/osx-x86_64/clang-tidy/6.0.1 to  /Users/kyle/github/mapping/vtshaver/mason_packages/.link
info Symlinked:  /Users/kyle/github/mapping/vtshaver/mason_packages/osx-x86_64/clang-format/6.0.1 to  /Users/kyle/github/mapping/vtshaver/mason_packages/.link
info Symlinked:  /Users/kyle/github/mapping/vtshaver/mason_packages/osx-x86_64/llvm-cov/6.0.1 to  /Users/kyle/github/mapping/vtshaver/mason_packages/.link
info Symlinked:  /Users/kyle/github/mapping/vtshaver/mason_packages/osx-x86_64/binutils/2.31 to  /Users/kyle/github/mapping/vtshaver/mason_packages/.link
info Symlinked:  /Users/kyle/github/mapping/vtshaver/mason_packages/osx-x86_64/mbgl-core/20f880e to  /Users/kyle/github/mapping/vtshaver/mason_packages/.link
info Symlinked:  /Users/kyle/github/mapping/vtshaver/mason_packages/osx-x86_64/nunicode/1.8 to  /Users/kyle/github/mapping/vtshaver/mason_packages/.link
info Symlinked:  /Users/kyle/github/mapping/vtshaver/mason_packages/osx-x86_64/libpng/1.6.25 to  /Users/kyle/github/mapping/vtshaver/mason_packages/.link
info Symlinked:  /Users/kyle/github/mapping/vtshaver/mason_packages/osx-x86_64/sqlite/3.14.2 to  /Users/kyle/github/mapping/vtshaver/mason_packages/.link
info Symlinked:  /Users/kyle/github/mapping/vtshaver/mason_packages/osx-x86_64/libjpeg-turbo/1.5.0 to  /Users/kyle/github/mapping/vtshaver/mason_packages/.link
info Symlinked:  /Users/kyle/github/mapping/vtshaver/mason_packages/osx-x86_64/webp/0.5.1 to  /Users/kyle/github/mapping/vtshaver/mason_packages/.link
info Symlinked:  /Users/kyle/github/mapping/vtshaver/mason_packages/osx-x86_64/icu/58.1-min-size to  /Users/kyle/github/mapping/vtshaver/mason_packages/.link
info Completed
  TOUCH Release/obj.target/action_before_build.stamp
  CXX(target) Release/obj.target/vtshaver/src/vtshaver.o
In file included from ../src/vtshaver.cpp:1:
In file included from ../src/filters.hpp:5:
In file included from /Users/kyle/github/mapping/vtshaver/node_modules/nan/nan.h:222:
In file included from /Users/kyle/github/mapping/vtshaver/node_modules/nan/nan_new.h:189:
/Users/kyle/github/mapping/vtshaver/node_modules/nan/nan_implementation_12_inl.h:103:42: error: no viable conversion from 'v8::Isolate *' to
      'Local<v8::Context>'
  return scope.Escape(v8::Function::New( isolate
                                         ^~~~~~~
/Users/kyle/Library/Caches/node-gyp/12.13.1/include/node/v8.h:183:7: note: candidate constructor (the implicit copy constructor) not viable: no known
      conversion from 'v8::Isolate *' to 'const v8::Local<v8::Context> &' for 1st argument
class Local {
      ^
/Users/kyle/Library/Caches/node-gyp/12.13.1/include/node/v8.h:183:7: note: candidate constructor (the implicit move constructor) not viable: no known
      conversion from 'v8::Isolate *' to 'v8::Local<v8::Context> &&' for 1st argument
/Users/kyle/Library/Caches/node-gyp/12.13.1/include/node/v8.h:187:13: note: candidate template ignored: could not match 'Local<type-parameter-0-0>' against
      'v8::Isolate *'
  V8_INLINE Local(Local<S> that)
            ^
/Users/kyle/Library/Caches/node-gyp/12.13.1/include/node/v8.h:4171:22: note: passing argument to parameter 'context' here
      Local<Context> context, FunctionCallback callback,
                     ^
1 error generated.
make: *** [Release/obj.target/vtshaver/src/vtshaver.o] Error 1
gyp ERR! build error
gyp ERR! stack Error: `make` failed with exit code: 2
gyp ERR! stack     at ChildProcess.onExit (/Users/kyle/.nvm/versions/node/v12.13.1/lib/node_modules/npm/node_modules/npm-lifecycle/node_modules/node-gyp/lib/build.js:194:23)
gyp ERR! stack     at ChildProcess.emit (events.js:210:5)
gyp ERR! stack     at Process.ChildProcess._handle.onexit (internal/child_process.js:272:12)
gyp ERR! System Darwin 19.0.0
gyp ERR! command "/Users/kyle/.nvm/versions/node/v12.13.1/bin/node" "/Users/kyle/.nvm/versions/node/v12.13.1/lib/node_modules/npm/node_modules/npm-lifecycle/node_modules/node-gyp/bin/node-gyp.js" "build" "--fallback-to-build" "--module=/Users/kyle/github/mapping/vtshaver/lib/binding/vtshaver.node" "--module_name=vtshaver" "--module_path=/Users/kyle/github/mapping/vtshaver/lib/binding" "--napi_version=5" "--node_abi_napi=napi" "--napi_build_version=0" "--node_napi_label=node-v72"
gyp ERR! cwd /Users/kyle/github/mapping/vtshaver
gyp ERR! node -v v12.13.1
gyp ERR! node-gyp -v v6.0.1
gyp ERR! not ok
node-pre-gyp ERR! build error
node-pre-gyp ERR! stack Error: Failed to execute '/Users/kyle/.nvm/versions/node/v12.13.1/bin/node /Users/kyle/.nvm/versions/node/v12.13.1/lib/node_modules/npm/node_modules/npm-lifecycle/node_modules/node-gyp/bin/node-gyp.js build --fallback-to-build --module=/Users/kyle/github/mapping/vtshaver/lib/binding/vtshaver.node --module_name=vtshaver --module_path=/Users/kyle/github/mapping/vtshaver/lib/binding --napi_version=5 --node_abi_napi=napi --napi_build_version=0 --node_napi_label=node-v72' (1)
node-pre-gyp ERR! stack     at ChildProcess.<anonymous> (/Users/kyle/github/mapping/vtshaver/node_modules/node-pre-gyp/lib/util/compile.js:83:29)
node-pre-gyp ERR! stack     at ChildProcess.emit (events.js:210:5)
node-pre-gyp ERR! stack     at maybeClose (internal/child_process.js:1021:16)
node-pre-gyp ERR! stack     at Process.ChildProcess._handle.onexit (internal/child_process.js:283:5)
node-pre-gyp ERR! System Darwin 19.0.0
node-pre-gyp ERR! command "/Users/kyle/.nvm/versions/node/v12.13.1/bin/node" "/Users/kyle/github/mapping/vtshaver/node_modules/.bin/node-pre-gyp" "install" "--fallback-to-build"
node-pre-gyp ERR! cwd /Users/kyle/github/mapping/vtshaver
node-pre-gyp ERR! node -v v12.13.1
node-pre-gyp ERR! node-pre-gyp -v v0.12.0
node-pre-gyp ERR! not ok
Failed to execute '/Users/kyle/.nvm/versions/node/v12.13.1/bin/node /Users/kyle/.nvm/versions/node/v12.13.1/lib/node_modules/npm/node_modules/npm-lifecycle/node_modules/node-gyp/bin/node-gyp.js build --fallback-to-build --module=/Users/kyle/github/mapping/vtshaver/lib/binding/vtshaver.node --module_name=vtshaver --module_path=/Users/kyle/github/mapping/vtshaver/lib/binding --napi_version=5 --node_abi_napi=napi --napi_build_version=0 --node_napi_label=node-v72' (1)
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! @mapbox/[email protected] install: `node-pre-gyp install --fallback-to-build`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the @mapbox/[email protected] install script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/kyle/.npm/_logs/2019-12-10T01_05_10_278Z-debug.log

Versions:

> node --version
v12.13.1
> npm --version
6.12.1

Mac 10.15.1

Upgrading to clang++ 7.x

It would be nice to be able to use the latest version of clang++. However we'll hit:

In file included from ../src/vtshaver.cpp:1:
In file included from ../src/filters.hpp:4:
In file included from /Users/danespringmeyer/projects/vtshaver/mason_packages/.link/include/mbgl/style/filter.hpp:4:
In file included from /Users/danespringmeyer/projects/vtshaver/mason_packages/.link/include/mbgl/util/feature.hpp:3:
In file included from /Users/danespringmeyer/projects/vtshaver/mason_packages/.link/include/mbgl/util/optional.hpp:3:
/Users/danespringmeyer/projects/vtshaver/mason_packages/osx-x86_64/clang++/7.0.0/include/c++/v1/experimental/optional:11:2: error: "<experimental/optional> has been removed. Use
      <optional> instead."
#error "<experimental/optional> has been removed. Use <optional> instead."

So, we'll need to upgrade mbgl to avoid this (#9) to a version that includes mapbox/mapbox-gl-native#13049

Filter keys/values based on filter results

Per @mapsam and @springmeyer 's thoughts:

In the current implementation we shave layers and whole features, but we do not (yet) drop unused properties from features that we keep. So if we keep a feature we currently decode all its properties and re-add all of its properties. If we only re-add'ed properties that are needed by a given style that would likely result in smaller, more efficient tiles for styles that only reference a few properties per style layer using vector tile sources with many properties.

vtshaver/src/shave.cpp

Line 322 in da0b5ba

  * - Filter out keys/values based on filter results below. Currently we need to add them to the new layer to have a complete layer, just like name, version, extent. For each key, add it back (no filtering for now) 

Starting to shave off unused properties could be a particularly good optimization for when vector tiles contain many language fields but a given style only needs to display few or none of them for a given layer.

Overall next steps to test this feature:

Extra parsing of the style by https://github.com/mapbox/vtshaver/blob/master/lib/styleToFilters.js to collect properties we need to keep based on keys referenced in style fields like text-field and any fields referenced in expressions if the styles uses expressions (https://blog.mapbox.com/announcing-expressions-in-gl-js-a72b55d0a6af)
Property whitelisting during shaving: Add the ability to pass down an array of extra "properties to keep" to the C++ shave function so that they are not stripped off.

Upgrade mbgl to avoid expression crash

We should upgrade mbgl to avoid being suceptible to mapbox/mapbox-gl-native#13383

[SPIKE] Revisit vendoring, in favor of adding mason dependency

Context

#58
The above PR added a vendored nunicode to vtshaver build. There may be a simpler way to provide the necessary files to vtshaver, such as adding nunicode as a mason dependency.

Additionally, an environment variable MBGL_USE_BUILTIN_ICU was defined in the build to use a simpler version of number-format that did not require ICU files. This might also be possible to provide as a linked dependency.

Handling Expressions that fail during evaluation

Expression evaluation in gl-native may fail, returning EvaluationError type, like here.

Currently, if this happens, false will be returned from this vtshaver code. The impact of that could be that whole features are dropped even if all of the other conditions of the filter evaluate to true.

Possible solutions are:

Detect expression errors and warn, while continuing to discard the feature
Keep features in this case
Throw an error in this case

It's hard to know which is better.

Releasing v0.2.0

This ticket is to track what is done and what still needs to be done for the v0.2.0 release.

These things are done

✅All features are landed and tickets are closed for the milestone: https://github.com/mapbox/vtshaver/milestone/1?closed=1
✅All tests are passing on travis
✅Test coverage is good: same or increased
✅If anything has been added to .npmignore, when we run make testpacked to ensure tests pass
✅For any major new feature we've made a dev package and tested downstream in staging
✅A developer has bumped the version in the package.json in master
✅A developer has committed with [publish binary] in the commit message
✅We've confirmed that the travis job with [publish binary] was fully 🍏
✅We've tagged a new git tag git tag v0.2.0 -a -m "v0.2.0" and uploaded to github git push --tags

Still needs to be done

@zmofei please go ahead and do all these steps. Let me know if you need a hand.

The binaries should be tested locally. This should work in seconds:

make distclean
npm install --fallback-to-build=false
npm test

Finishing the release:

Close the v0.2.0 milestone since it is done: https://github.com/mapbox/vtshaver/milestones
Update the changelog.md with v0.2.0 details:https://github.com/mapbox/vtshaver/blob/master/CHANGELOG.md
Run npm pack and ensure that it shows only the intended files will be packaged and nothing unintended or accidental
Publish to npm repository: mbx npm publish
Update the https://github.com/mapbox/vtshaver/blob/master/CONTRIBUTING.md by adding details on how to release a new vtshaver version (basically move these docs into the CONTRIBUTING.md for future reference)
Close this ticket!

Option to keep all feature properties

#20 landed the ability to have vtshaver automatically remove feature properties that are not needed for a given style.

However there are use cases (like querying) where some users might want feature properties to be kept.

A sophisticated solution for this would be to have vtshaver respond to metadata in the style that declares which properties to keep on which layers. However a more near-term solution I think would be to open up an option, exposed in the vtshaver CLI tool, that would disable all property shaving.

Consider taking into account opacity:0

If opacity:0 data is not shown. We could consider shaving off data from the tiles in this case.

Add CLI that optimizes an mbtiles of vector tiles

A CLI tool that runs this module over an entire tileset in an mbtiles file would be a useful way of exploring optimizations and optimizing entire outputs from tile generators like tippecanoe.

The CLI could be:

./bin/optimize-mbtiles.js [style.json] [input.mbtiles] [output.mbtiles]

By default it would require writing a new mbtiles. But potentially it could accept an option to overwrite the existing mbtiles with optimized tiles (for the very experienced user).

Add tests for all style expressions

Context

Recently, we had to manually add 3 new c++ symbol implementations from gl-native to the vtshaver build.
#58

In order to prevent other regressions, we should add tests that ensure all style expression symbols are defined.

Acceptance Criteria

The definition of all style expressions should be covered by tests (can be one super-style or multiple style tests).

Remove mason-js dependency

The mason-js dependency is unmaintained and frozen, but still currently used inside of vtshaver. It has been unmaintained since 2018. For a while it seemed like mason-js would again see maintenance (enough that security issues related to out-of-date binaries and mason-js JS dependencies could be mitigated). But, in effect, mason-js not been maintained since 2018 and therefore I think it is critical to acknowledge this and take action downstream (here).

So, my recommendation is to remove the dependence on mason-js in vtshaver.

To do this would involve:

Removing mason-js from the package.json
Removing the mason-versions.ini
Implementing an alternative method for fetching up to date and reliable versions of dependencies that are currently being installed by mason-js

N-API port

Context

vtshaver is used in our APIs, for example in api-vectortiles. Those related APIs can't be upgraded to use more recent node until this is updated.

Notes

vtshaver has old PR containing N-api port for it, which should be reviewed, tested and merged to use. This issue should be followed by other issue updating other dependencies and nodejs to more recent one.
At least vtcomposite has already been ported, as is node-cpp-skel.

Related old PR to be merged:
#44

Acceptance Criteria

vtshaver uses node-addon-api instead of nan as depenency. Publish working version of vtvalidate binary.

AsyncBaton may leak memory if an error occurs in shave setup

Per @springmeyer 's thoughts:

We should use the RAII pattern to ensure that AsyncBaton will not leak if an exception occurs.

Update API.md

The README currently includes API docs, but there's also make docs which will automatically generate them and add them to API.md. It is currently still made up of node-cpp-skel API docs.

Next steps

Update code comments to reflect updated API (see here). You can use the examples in README.md as reference.
Update filter instantiation API docs to properly reflect creating the filter object. They are a little out of date.

Old:

var filters = shaver.styleToFilters(style);

Now:

var filters = new shaver.Filters(shaver.styleToFilters(JSON.parse(style)));

Run make docs to generate new docs using documentation.js
Potentially remove the API examples from the readme and just point to API.md?

cc @mapbox/maps-api

Docs on how to update mbgl dependency

Currently to update mbgl one would need to:

Package two new mbgl packages:
- Release build (like mapbox/mason#651)
- Debug+ASAN build (like mapbox/mason#652)
Update the mbgl version in mason-versions.ini

This should be:

documented in the readme
and @springmeyer should spirit guide another developer through this process to ensure it is robust and we have redundency

Use Nan::AsyncWorker

Per @alliecrevier 's thoughts:

Replace AsyncBaton and AfterShave (the name is so good so try to figure out a way to keep it going) with a Nan::AsyncWorker subclass and Nan::AsyncQueueWorker
Make sure this fixes deprecation warnings related to the use of MakeCallback and Call

Ref: mapbox/vtcomposite#52, in particular

Make code changes to fix the deprecation warnings related to the use of MakeCallback and Call. Example PR to follow: https://github.com/mapbox/node-cpp-skel/pull/127/files

Mascot

Per @mapsam :

We're shaving. Here's a beard that needs some shaving!

Bonus: real world geometry (kinda) as a beard!

Add support to node 14 & 16

Context
vtshaver includes c++ node bindings which should be updated to support more recent node versions

Notes

Acceptance Criteria

vtshaver supports node 14 & 16 and new binaries of vtshaver are published

	function getPropertyFromFilter(filter, properties) {
	if (styleSpec.expression.isExpression(filter)) {
	getPropertyFromExpression(filter, properties);
	}

	// Warning: Below code should put in to an else conditions,
	// but since the `isExpression` can not tell it is a expression or filter syntax I put it outsied the else
	// this could reduce the performance or cause some potential bugs, we must keep an eye on this.

	// else {
	let subFilter = [];
	for (let i = 0; i < filter.length; i++) {
	if (typeof filter[i] === 'object' && filter[i] instanceof Array) {
	subFilter.push(filter[i]);
	}
	}

	if (subFilter.length > 0) {
	subFilter.forEach(sfilter => {
	getPropertyFromFilter(sfilter, properties);
	})
	} else {
	if (filter.length >= 3 && typeof filter[1] === 'string') {
	if (filter[1].indexOf('$') === -1) {
	properties.push(filter[1]);
	}

	}
	}
	// }
	}

	if (baton->compress) {
	// Compress final tile before sending back
	std::string final_data;
	finalvt.serialize(final_data);
	gzip::Compressor compressor;
	compressor.compress(*baton->shaved_tile, final_data.data(), final_data.size());

mapbox / vtshaver Goto Github PK

vtshaver's Introduction

Vector Tile Shaver

Installation

API Usage

CLI

vtshave

vtshaver-filters

Develop

Test

Docs

vtshaver's People

Contributors

Stargazers

Watchers

Forkers

vtshaver's Issues

Next Actions

Context

These things are done

Still needs to be done

Context

Acceptance Criteria

Context

Notes

Acceptance Criteria

Next steps

Notes

Acceptance Criteria

Recommend Projects

Recommend Topics

Recommend Org