From 9fea7d0175926f73daebf60972e0d7a49faadd99 Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Wed, 31 Aug 2011 15:52:17 -0700 Subject: [PATCH] move the palette option to be part of the optional second argument to match the map.render() and grid.encode() interfaces - unbreaks tilelive-mapnik --- src/mapnik_image.cpp | 47 +++++++++++++++++++++++++++++--------- src/mapnik_image_view.cpp | 46 ++++++++++++++++++++++++++++--------- src/mapnik_map.cpp | 48 ++++++++++++++++++++++++++------------- tests/palette.test.js | 4 +++- 4 files changed, 106 insertions(+), 39 deletions(-) diff --git a/src/mapnik_image.cpp b/src/mapnik_image.cpp index 796d7abb52..9e30cc19c3 100644 --- a/src/mapnik_image.cpp +++ b/src/mapnik_image.cpp @@ -196,16 +196,29 @@ Handle Image::encodeSync(const Arguments& args) String::New("first arg, 'format' must be a string"))); format = TOSTR(args[0]); } + + + // options hash if (args.Length() >= 2) { if (!args[1]->IsObject()) return ThrowException(Exception::TypeError( - String::New("mapnik.Palette expected as second arg"))); + String::New("optional second arg must be an options object"))); - Local obj = args[1]->ToObject(); - if (obj->IsNull() || obj->IsUndefined() || !Palette::constructor->HasInstance(obj)) - return ThrowException(Exception::TypeError(String::New("mapnik.Palette expected as second arg"))); + Local options = args[1]->ToObject(); - palette = ObjectWrap::Unwrap(obj)->palette(); + if (options->Has(String::New("palette"))) + { + Local format_opt = options->Get(String::New("palette")); + if (!format_opt->IsObject()) + return ThrowException(Exception::TypeError( + String::New("'palette' must be an object"))); + + Local obj = format_opt->ToObject(); + if (obj->IsNull() || obj->IsUndefined() || !Palette::constructor->HasInstance(obj)) + return ThrowException(Exception::TypeError(String::New("mapnik.Palette expected as second arg"))); + + palette = ObjectWrap::Unwrap(obj)->palette(); + } } try { @@ -257,16 +270,28 @@ Handle Image::encode(const Arguments& args) String::New("first arg, 'format' must be a string"))); format = TOSTR(args[0]); } - if (args.Length() > 2) { + + // options hash + if (args.Length() >= 2) { if (!args[1]->IsObject()) return ThrowException(Exception::TypeError( - String::New("mapnik.Palette expected as second arg"))); + String::New("optional second arg must be an options object"))); - Local obj = args[1]->ToObject(); - if (obj->IsNull() || obj->IsUndefined() || !Palette::constructor->HasInstance(obj)) - return ThrowException(Exception::TypeError(String::New("mapnik.Palette expected as second arg"))); + Local options = args[1]->ToObject(); - palette = ObjectWrap::Unwrap(obj)->palette(); + if (options->Has(String::New("palette"))) + { + Local format_opt = options->Get(String::New("palette")); + if (!format_opt->IsObject()) + return ThrowException(Exception::TypeError( + String::New("'palette' must be an object"))); + + Local obj = format_opt->ToObject(); + if (obj->IsNull() || obj->IsUndefined() || !Palette::constructor->HasInstance(obj)) + return ThrowException(Exception::TypeError(String::New("mapnik.Palette expected as second arg"))); + + palette = ObjectWrap::Unwrap(obj)->palette(); + } } // ensure callback is a function diff --git a/src/mapnik_image_view.cpp b/src/mapnik_image_view.cpp index e9e83174da..0fee490fa7 100644 --- a/src/mapnik_image_view.cpp +++ b/src/mapnik_image_view.cpp @@ -116,16 +116,28 @@ Handle ImageView::encodeSync(const Arguments& args) String::New("first arg, 'format' must be a string"))); format = TOSTR(args[0]); } + + // options hash if (args.Length() >= 2) { if (!args[1]->IsObject()) return ThrowException(Exception::TypeError( - String::New("mapnik.Palette expected as second arg"))); + String::New("optional second arg must be an options object"))); - Local obj = args[1]->ToObject(); - if (obj->IsNull() || obj->IsUndefined() || !Palette::constructor->HasInstance(obj)) - return ThrowException(Exception::TypeError(String::New("mapnik.Palette expected as second arg"))); + Local options = args[1]->ToObject(); - palette = ObjectWrap::Unwrap(obj)->palette(); + if (options->Has(String::New("palette"))) + { + Local format_opt = options->Get(String::New("palette")); + if (!format_opt->IsObject()) + return ThrowException(Exception::TypeError( + String::New("'palette' must be an object"))); + + Local obj = format_opt->ToObject(); + if (obj->IsNull() || obj->IsUndefined() || !Palette::constructor->HasInstance(obj)) + return ThrowException(Exception::TypeError(String::New("mapnik.Palette expected as second arg"))); + + palette = ObjectWrap::Unwrap(obj)->palette(); + } } try { @@ -178,16 +190,28 @@ Handle ImageView::encode(const Arguments& args) String::New("first arg, 'format' must be a string"))); format = TOSTR(args[0]); } - if (args.Length() > 2) { + + // options hash + if (args.Length() >= 2) { if (!args[1]->IsObject()) return ThrowException(Exception::TypeError( - String::New("mapnik.Palette expected as second arg"))); + String::New("optional second arg must be an options object"))); - Local obj = args[1]->ToObject(); - if (obj->IsNull() || obj->IsUndefined() || !Palette::constructor->HasInstance(obj)) - return ThrowException(Exception::TypeError(String::New("mapnik.Palette expected as second arg"))); + Local options = args[1]->ToObject(); - palette = ObjectWrap::Unwrap(obj)->palette(); + if (options->Has(String::New("palette"))) + { + Local format_opt = options->Get(String::New("palette")); + if (!format_opt->IsObject()) + return ThrowException(Exception::TypeError( + String::New("'palette' must be an object"))); + + Local obj = format_opt->ToObject(); + if (obj->IsNull() || obj->IsUndefined() || !Palette::constructor->HasInstance(obj)) + return ThrowException(Exception::TypeError(String::New("mapnik.Palette expected as second arg"))); + + palette = ObjectWrap::Unwrap(obj)->palette(); + } } // ensure callback is a function diff --git a/src/mapnik_map.cpp b/src/mapnik_map.cpp index 40f7692700..2e0797a8d4 100644 --- a/src/mapnik_map.cpp +++ b/src/mapnik_map.cpp @@ -1438,16 +1438,28 @@ Handle Map::renderSync(const Arguments& args) std::string format = TOSTR(args[0]); palette_ptr palette; + + // options hash if (args.Length() >= 2) { if (!args[1]->IsObject()) return ThrowException(Exception::TypeError( - String::New("mapnik.Palette expected as second arg"))); + String::New("optional second arg must be an options object"))); + + Local options = args[1]->ToObject(); - Local obj = args[1]->ToObject(); - if (obj->IsNull() || obj->IsUndefined() || !Palette::constructor->HasInstance(obj)) - return ThrowException(Exception::TypeError(String::New("mapnik.Palette expected as second arg"))); + if (options->Has(String::New("palette"))) + { + Local bind_opt = options->Get(String::New("palette")); + if (!bind_opt->IsObject()) + return ThrowException(Exception::TypeError( + String::New("mapnik.Palette expected as second arg"))); - palette = ObjectWrap::Unwrap(obj)->palette(); + Local obj = bind_opt->ToObject(); + if (obj->IsNull() || obj->IsUndefined() || !Palette::constructor->HasInstance(obj)) + return ThrowException(Exception::TypeError(String::New("mapnik.Palette expected as second arg"))); + + palette = ObjectWrap::Unwrap(obj)->palette(); + } } Map* m = ObjectWrap::Unwrap(args.This()); @@ -1488,9 +1500,9 @@ Handle Map::renderFileSync(const Arguments& args) return ThrowException(Exception::TypeError( String::New("first argument must be a path to a file to save"))); - if (args.Length() > 3) + if (args.Length() > 2) return ThrowException(Exception::TypeError( - String::New("accepts three arguments, a required path to a file, an optional options object, eg. {format: 'pdf'}, and an optional Palette object"))); + String::New("accepts two arguments, a required path to a file, an optional options object, eg. {format: 'pdf'}"))); std::string format = "png8"; palette_ptr palette; @@ -1510,17 +1522,21 @@ Handle Map::renderFileSync(const Arguments& args) format = TOSTR(format_opt); } - } - if (args.Length() >= 3) { - if (!args[2]->IsObject()) - return ThrowException(Exception::TypeError( - String::New("mapnik.Palette expected as second arg"))); - Local obj = args[2]->ToObject(); - if (obj->IsNull() || obj->IsUndefined() || !Palette::constructor->HasInstance(obj)) - return ThrowException(Exception::TypeError(String::New("mapnik.Palette expected as second arg"))); + if (options->Has(String::New("palette"))) + { + Local format_opt = options->Get(String::New("palette")); + if (!format_opt->IsObject()) + return ThrowException(Exception::TypeError( + String::New("'palette' must be an object"))); + + Local obj = format_opt->ToObject(); + if (obj->IsNull() || obj->IsUndefined() || !Palette::constructor->HasInstance(obj)) + return ThrowException(Exception::TypeError(String::New("mapnik.Palette expected as second arg"))); + + palette = ObjectWrap::Unwrap(obj)->palette(); + } - palette = ObjectWrap::Unwrap(obj)->palette(); } Map* m = ObjectWrap::Unwrap(args.This()); diff --git a/tests/palette.test.js b/tests/palette.test.js index c83c541ca7..0d00f56e58 100644 --- a/tests/palette.test.js +++ b/tests/palette.test.js @@ -37,7 +37,9 @@ exports['test palette rendering'] = function(beforeExit) { // Test rendering a blank image var filename = helper.filename(); - map.renderFileSync(filename, {}, pal); + var buffer = map.renderSync("png", {palette:pal}); + assert.ok(buffer.length < 7000); + map.renderFileSync(filename, {palette:pal}); var stat = fs.statSync(filename); assert.ok(stat.size < 7000); };