N-API call Map::release() in OnWorkComplete(..) + cleanups (WIP)

artemp · artemp · commit 5d9e2bcc1e61 · 2020-06-05T10:50:58.000+01:00
diff --git a/src/blend.cpp b/src/blend.cpp
@@ -490,7 +490,6 @@ struct AsyncBlend : Napi::AsyncWorker
         Base::SetError(err);
     }
 
-//private:
     Images images_;
     int quality_;
     int width_;
@@ -512,7 +511,7 @@ static void Blend_Encode(AsyncBlend* worker, mapnik::image_rgba8 const& image, b
         if (worker->format_ == BLEND_FORMAT_JPEG)
         {
 #if defined(HAVE_JPEG)
-            //if (worker->quality_ == 0) worker->quality_ = 85;
+            if (worker->quality_ == 0) worker->quality_ = 85;
             mapnik::save_as_jpeg(stream, worker->quality_, image);
 #else
             worker->SetError("Mapnik not built with jpeg support");
@@ -914,7 +913,8 @@ Napi::Value blend(Napi::CallbackInfo const& info)
 
         if (image->buffer.IsEmpty() && !image->im_obj)
         {
-            Napi::TypeError::New(env, "All elements must be Buffers or RGBA Mapnik Image objects or objects with a 'buffer' property.").ThrowAsJavaScriptException();
+            Napi::TypeError::New(env, "All elements must be Buffers or RGBA Mapnik Image objects or objects with a 'buffer' property.")
+                .ThrowAsJavaScriptException();
             return env.Undefined();
         }
         if (!image->im_obj)
diff --git a/src/ds_emitter.hpp b/src/ds_emitter.hpp
@@ -25,7 +25,7 @@ static void get_fields(Napi::Env env, Napi::Object & fields, mapnik::datasource_
         else if (field_type == mapnik::Float) type = "Number";
         else if (field_type == mapnik::Double) type = "Number";
         else if (field_type == mapnik::String) type = "String";
-        /* LCOV_EXCL_START */
+        // LCOV_EXCL_START
         // Not currently possible to author these values in mapnik core
         // Should likely be considered for removing in mapnik
         else if (field_type == mapnik::Boolean) type = "Boolean";
@@ -34,8 +34,7 @@ static void get_fields(Napi::Env env, Napi::Object & fields, mapnik::datasource_
         else type = "Unknown";
         std::string const& name = attr_info.get_name();
         fields.Set(name, type);
-        //fields.Set(name, Napi::String::New(env, type)); <== FIXME: duplicate?
-        /* LCOV_EXCL_STOP */
+        // LCOV_EXCL_STOP
     }
 }
 
diff --git a/src/mapnik_cairo_surface.cpp b/src/mapnik_cairo_surface.cpp
@@ -62,6 +62,5 @@ Napi::Value CairoSurface::getData(Napi::CallbackInfo const& info)
     Napi::EscapableHandleScope scope(env);
     if (!data_.empty()) return scope.Escape(Napi::String::New(env, data_));
     std::string str = stream_.str();
-    //return scope.Escape(Napi::Buffer<char>::Copy(env, const_cast<char*>(str.data()), str.size()));
     return scope.Escape(Napi::String::New(env, str));
 }
diff --git a/src/mapnik_cairo_surface.hpp b/src/mapnik_cairo_surface.hpp
@@ -1,8 +1,7 @@
 #pragma once
 
 #include <napi.h>
-
-//#include <string>
+// stl
 #include <sstream>
 // cairo
 #if defined(HAVE_CAIRO)
@@ -15,7 +14,6 @@
 class CairoSurface : public Napi::ObjectWrap<CairoSurface>
 {
 public:
-    //typedef std::stringstream i_stream;
     // initializer
     static Napi::Object Initialize(Napi::Env env, Napi::Object exports);
     // ctor
@@ -24,12 +22,6 @@ class CairoSurface : public Napi::ObjectWrap<CairoSurface>
     Napi::Value getData(Napi::CallbackInfo const& info);
     Napi::Value width(Napi::CallbackInfo const& info);
     Napi::Value height(Napi::CallbackInfo const& info);
-
-    //using Napi::ObjectWrap::Ref;
-    //using Napi::ObjectWrap::Unref;
-
-    //CairoSurface(std::string const& format, unsigned int width, unsigned int height);
-
     static cairo_status_t write_callback(void * stream,
                                          const unsigned char *data,
                                          unsigned int length)
diff --git a/src/mapnik_color.cpp b/src/mapnik_color.cpp
@@ -1,7 +1,4 @@
 #include "mapnik_color.hpp"
-// stl
-#include <exception>
-
 
 Napi::FunctionReference Color::constructor;
 
diff --git a/src/mapnik_datasource.cpp b/src/mapnik_datasource.cpp
@@ -5,12 +5,12 @@
 
 // mapnik
 #include <mapnik/attribute_descriptor.hpp>  // for attribute_descriptor
-#include <mapnik/geometry/box2d.hpp>             // for box2d
-#include <mapnik/datasource.hpp>        // for datasource, datasource_ptr, etc
-#include <mapnik/datasource_cache.hpp>  // for datasource_cache
-#include <mapnik/feature_layer_desc.hpp>  // for layer_descriptor
-#include <mapnik/params.hpp>            // for parameters
-#include <mapnik/query.hpp>             // for query
+#include <mapnik/geometry/box2d.hpp>        // for box2d
+#include <mapnik/datasource.hpp>            // for datasource, datasource_ptr, etc
+#include <mapnik/datasource_cache.hpp>      // for datasource_cache
+#include <mapnik/feature_layer_desc.hpp>    // for layer_descriptor
+#include <mapnik/params.hpp>                // for parameters
+#include <mapnik/query.hpp>                 // for query
 
 // stl
 #include <exception>
@@ -134,10 +134,10 @@ Napi::Value Datasource::extent(Napi::CallbackInfo const& info)
         // where a plugin dynamically calculated extent such as
         // postgis plugin. Therefore this makes this difficult
         // to add to testing. Therefore marking it with exclusion
-        /* LCOV_EXCL_START */
+        // LCOV_EXCL_START
         Napi::Error::New(env, ex.what()).ThrowAsJavaScriptException();
         return env.Undefined();
-        /* LCOV_EXCL_STOP */
+        // LCOV_EXCL_STOP
     }
 
     Napi::Array arr = Napi::Array::New(env, 4);
@@ -172,10 +172,10 @@ Napi::Value Datasource::describe(Napi::CallbackInfo const& info)
         // where a plugin dynamically calculated extent such as
         // postgis plugin. Therefore this makes this difficult
         // to add to testing. Therefore marking it with exclusion
-        /* LCOV_EXCL_START */
+        // LCOV_EXCL_START
         Napi::Error::New(env, ex.what()).ThrowAsJavaScriptException();
         return env.Undefined();
-        /* LCOV_EXCL_STOP */
+        // LCOV_EXCL_STOP
     }
     return scope.Escape(description);
 }
diff --git a/src/mapnik_feature.cpp b/src/mapnik_feature.cpp
@@ -31,16 +31,6 @@ Napi::Object Feature::Initialize(Napi::Env env, Napi::Object exports)
     return exports;
 }
 
-//Feature::Feature(mapnik::feature_ptr f) : Napi::ObjectWrap<Feature>(),
-//    this_(f) {}
-
-//Feature::Feature(int id) : Napi::ObjectWrap<Feature>(),
-//    this_() {
-//    // TODO - accept/require context object to reused
-//    ctx_ = std::make_shared<mapnik::context_type>();
-//    this_ = mapnik::feature_factory::create(ctx_,id);
-//}
-
 /**
  * **`mapnik.Feature`**
  *
diff --git a/src/mapnik_feature.hpp b/src/mapnik_feature.hpp
@@ -10,9 +10,6 @@
 
 class Feature : public Napi::ObjectWrap<Feature>
 {
-    //friend class Featureset;
-    //friend class Expression;
-    //friend class VectorTile;
 public:
     // initialiser
     static Napi::Object Initialize(Napi::Env env, Napi::Object exports);
diff --git a/src/mapnik_image_copy.cpp b/src/mapnik_image_copy.cpp
@@ -37,7 +37,7 @@ struct AsyncCopy : Napi::AsyncWorker
         {
             Napi::Value arg = Napi::External<image_ptr>::New(env, &image_out_);
             Napi::Object obj = Image::constructor.New({arg});
-            return {env.Null(), napi_value(obj)};
+            return {env.Undefined(), napi_value(obj)};
         }
         return Base::GetResult(env);
     }
diff --git a/src/mapnik_image_view.cpp b/src/mapnik_image_view.cpp
@@ -105,18 +105,6 @@ ImageView::ImageView(Napi::CallbackInfo const& info)
     Napi::Error::New(env, "Cannot create this object from Javascript").ThrowAsJavaScriptException();
 }
 
-/*
-typedef struct {
-    uv_work_t request;
-    ImageView* im;
-    Napi::FunctionReference cb;
-    bool error;
-    std::string error_name;
-    bool result;
-} is_solid_image_view_baton_t;
-*/
-
-
 Napi::Value ImageView::isSolid(Napi::CallbackInfo const& info)
 {
     if (info.Length() == 0)
diff --git a/src/mapnik_logger.hpp b/src/mapnik_logger.hpp
@@ -16,7 +16,6 @@ class Logger : public Napi::ObjectWrap<Logger>
     // Are these the only methods available in logger?
     static Napi::Value get_severity(Napi::CallbackInfo const& info);
     static Napi::Value set_severity(Napi::CallbackInfo const& info);
-    //Napi::Value evoke_error(Napi::CallbackInfo const& info);
 private:
     static Napi::FunctionReference constructor;
 };
diff --git a/src/mapnik_map.cpp b/src/mapnik_map.cpp
@@ -1,34 +1,16 @@
 #include "mapnik_map.hpp"
 #include "utils.hpp"
 #include "mapnik_color.hpp"             // for Color, Color::constructor
-//#include "mapnik_featureset.hpp"        // for Featureset
 #include "mapnik_image.hpp"             // for Image, Image::constructor
 #include "mapnik_layer.hpp"             // for Layer, Layer::constructor
 #include "mapnik_palette.hpp"           // for palette_ptr, Palette, etc
-//#include "mapnik_vector_tile.hpp"
-
-// mapnik-vector-tile
-//#include "vector_tile_processor.hpp"
-
 // mapnik
-#include <mapnik/featureset.hpp>        // for featureset_ptr
+#include <mapnik/map.hpp>
 #include <mapnik/layer.hpp>             // for layer
 #include <mapnik/save_map.hpp>          // for save_map, etc
-#include <mapnik/image_scaling.hpp>
-#include <mapnik/request.hpp>
-#if defined(HAVE_CAIRO)
-#include <mapnik/cairo_io.hpp>
-#endif
-
 // stl
-//#include <exception>                    // for exception
-//#include <iosfwd>                       // for ostringstream, ostream
-//#include <ostream>                      // for operator<<, basic_ostream, etc
 #include <sstream>                      // for basic_ostringstream, etc
 
-// boost
-#include <boost/optional/optional.hpp>  // for optional
-
 Napi::FunctionReference Map::constructor;
 
 Napi::Object Map::Initialize(Napi::Env env, Napi::Object exports)
diff --git a/src/mapnik_map_render.cpp b/src/mapnik_map_render.cpp
diff --git a/src/mapnik_vector_tile.hpp b/src/mapnik_vector_tile.hpp
diff --git a/test/map_generation.test.js b/test/map_generation.test.js

Original file line number	Diff line number	Diff line change
`@@ -490,7 +490,6 @@ struct AsyncBlend : Napi::AsyncWorker`
`490`	`490`	`Base::SetError(err);`
`491`	`491`	`}`
`492`	`492`
`493`		`-//private:`
`494`	`493`	`Images images_;`
`495`	`494`	`int quality_;`
`496`	`495`	`int width_;`
`@@ -512,7 +511,7 @@ static void Blend_Encode(AsyncBlend* worker, mapnik::image_rgba8 const& image, b`
`512`	`511`	`if (worker->format_ == BLEND_FORMAT_JPEG)`
`513`	`512`	`{`
`514`	`513`	`#if defined(HAVE_JPEG)`
`515`		`- //if (worker->quality_ == 0) worker->quality_ = 85;`
	`514`	`+ if (worker->quality_ == 0) worker->quality_ = 85;`
`516`	`515`	`mapnik::save_as_jpeg(stream, worker->quality_, image);`
`517`	`516`	`#else`
`518`	`517`	`worker->SetError("Mapnik not built with jpeg support");`
`@@ -914,7 +913,8 @@ Napi::Value blend(Napi::CallbackInfo const& info)`
`914`	`913`
`915`	`914`	`if (image->buffer.IsEmpty() && !image->im_obj)`
`916`	`915`	`{`
`917`		`- Napi::TypeError::New(env, "All elements must be Buffers or RGBA Mapnik Image objects or objects with a 'buffer' property.").ThrowAsJavaScriptException();`
	`916`	`+ Napi::TypeError::New(env, "All elements must be Buffers or RGBA Mapnik Image objects or objects with a 'buffer' property.")`
	`917`	`+ .ThrowAsJavaScriptException();`
`918`	`918`	`return env.Undefined();`
`919`	`919`	`}`
`920`	`920`	`if (!image->im_obj)`
Original file line number	Diff line number	Diff line change
`@@ -62,6 +62,5 @@ Napi::Value CairoSurface::getData(Napi::CallbackInfo const& info)`
`62`	`62`	`Napi::EscapableHandleScope scope(env);`
`63`	`63`	`if (!data_.empty()) return scope.Escape(Napi::String::New(env, data_));`
`64`	`64`	`std::string str = stream_.str();`
`65`		`- //return scope.Escape(Napi::Buffer<char>::Copy(env, const_cast<char*>(str.data()), str.size()));`
`66`	`65`	`return scope.Escape(Napi::String::New(env, str));`
`67`	`66`	`}`
Original file line number	Diff line number	Diff line change
`@@ -10,9 +10,6 @@`
`10`	`10`
`11`	`11`	`class Feature : public Napi::ObjectWrap<Feature>`
`12`	`12`	`{`
`13`		`- //friend class Featureset;`
`14`		`- //friend class Expression;`
`15`		`- //friend class VectorTile;`
`16`	`13`	`public:`
`17`	`14`	`// initialiser`
`18`	`15`	`static Napi::Object Initialize(Napi::Env env, Napi::Object exports);`
Original file line number	Diff line number	Diff line change
`@@ -37,7 +37,7 @@ struct AsyncCopy : Napi::AsyncWorker`
`37`	`37`	`{`
`38`	`38`	`Napi::Value arg = Napi::External<image_ptr>::New(env, &image_out_);`
`39`	`39`	`Napi::Object obj = Image::constructor.New({arg});`
`40`		`- return {env.Null(), napi_value(obj)};`
	`40`	`+ return {env.Undefined(), napi_value(obj)};`
`41`	`41`	`}`
`42`	`42`	`return Base::GetResult(env);`
`43`	`43`	`}`