Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move on to Mapnik 3.x #9

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions build.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@
<target name="compile">
<mkdir dir="build/classes"/>
<mkdir dir="csrc"/>
<javac srcdir="src" destdir="build/classes" debug="on" source="1.5" target="1.5" includeantruntime="false"/>
<javac srcdir="src" destdir="build/classes" debug="on" source="1.6" target="1.6" includeantruntime="false"/>
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've only actually tested this with Oracle's Java 8 VM on both OS X and Windows, which complained about source="1.5" (warnings). Maybe it would be better to update this to 1.8 or drop the source parameter altogether? I am not an Ant or JVM expert.

<mkdir dir="build/classes-tools"/>
<javac srcdir="tools" destdir="build/classes-tools" debug="on" source="1.5" target="1.5" includeantruntime="false">
<javac srcdir="tools" destdir="build/classes-tools" debug="on" source="1.6" target="1.6" includeantruntime="false">
<classpath>
<pathelement location="build/classes"/>
</classpath>
Expand All @@ -26,7 +26,7 @@
<target name="test" depends="jar,native">
<mkdir dir="build/classes-test"/>
<mkdir dir="build/testresults"/>
<javac srcdir="test" destdir="build/classes-test" debug="on" source="1.5" target="1.5" includeantruntime="false">
<javac srcdir="test" destdir="build/classes-test" debug="on" source="1.6" target="1.6" includeantruntime="false">
<classpath>
<pathelement location="${junit.jar}"/>
<pathelement location="build/dist/mapnik-jni.jar"/>
Expand All @@ -40,6 +40,8 @@
<pathelement location="build/classes-test"/>
</classpath>

<sysproperty key="java.library.path" value="build/dist/"/>

<formatter type="plain"/>

<batchtest todir="build/testresults">
Expand All @@ -61,7 +63,7 @@
access="public"
encoding="UTF-8"
windowtitle="mapnik-jni API documentation"
source="1.5"
source="1.6"
/>
</target>

Expand Down
3 changes: 2 additions & 1 deletion csrc/Makefile.osx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ libmapnik-jni.jnilib: $(SOURCE_DEPENDS)
$(JAVA_CFLAGS) \
mapnikjni.cpp \
$(LDFLAGS) \
$(MAPNIK_LIBS) -framework JavaVM
$(JAVA_LDFLAGS) \
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The JavaVM.framework contains symlinks to a non-existent JDK 1.6 on my Mac; I'm not sure if that is the default state of Macs nowadays, or if I broke things by installing and uninstalling JDKs.

$(MAPNIK_LIBS)

clean:
rm libmapnik-jni.jnilib
Expand Down
25 changes: 24 additions & 1 deletion csrc/class_datasource.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,30 @@ JNIEXPORT jobject JNICALL Java_mapnik_Datasource_getParameters

for (mapnik::param_map::const_iterator iter=params.begin(); iter!=params.end(); iter++) {
jstring key=env->NewStringUTF(iter->first.c_str());
boost::apply_visitor(translate_parameter_visitor(env, paramobject, key), iter->second);
translate_parameter_visitor visitor(env, paramobject, key);
// TODO - The call to visit() does not compile on MSVC 2015 (error C2783).
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't compile on OS X, either. I'm not sure if I am even supposed to call mapnik::value_type::visit directly? It worked fine in another place.

// The compiler cannot deduce the __formal type(?) in:
// const T &mapnik::util::variant<mapnik::value_null,
// mapnik::value_integer,
// mapnik::value_double,
// std::string,
// mapnik::value_bool>::get(void) const
// (ditto for the non-const version)
// mapnik::value_type::visit(iter->second, visitor);
// So, the variant<> types are temporarily unrolled here...
if (iter->second.is<mapnik::value_integer>()) {
visitor(iter->second.get<mapnik::value_integer>());
}
else if (iter->second.is<mapnik::value_double>()) {
visitor(iter->second.get<mapnik::value_double>());
}
else if (iter->second.is<std::string>()) {
visitor(iter->second.get<std::string>());
}
else if (iter->second.is<mapnik::value_bool>()) {
visitor(iter->second.get<mapnik::value_bool>());
}
// else: value_null or unexpected value - ignore
}

return paramobject;
Expand Down
27 changes: 11 additions & 16 deletions csrc/class_featureset.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,12 @@ JNIEXPORT jobjectArray JNICALL Java_mapnik_FeatureSet__1loadGeometries
return 0;
}

unsigned count=(*fp)->num_geometries();
// Mapnik 3.x only holds one geometry<> per feature_impl.
// (In Mapnik 2.x this used to be a variable-length vector of geometries.)
unsigned count=1;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I'm not misunderstanding things here, the JNI interface should be refactored to match the new C++ interface – but I didn't want to expand the scope of this PR for something that I'm not really sure about (I only want to render maps 😅).

jobjectArray ret=env->NewObjectArray(count, CLASS_GEOMETRY, (jobject)0);
for (unsigned index=0; index<count; index++) {
mapnik::geometry_type &g((*fp)->get_geometry(index));
mapnik::geometry::geometry<double> const &g((*fp)->get_geometry());
jobject gobject=env->NewObject(CLASS_GEOMETRY, CTOR_NATIVEOBJECT);
env->SetLongField(gobject, FIELD_PTR, FROM_POINTER(&g));
env->SetObjectArrayElement(ret, index, gobject);
Expand Down Expand Up @@ -128,50 +130,43 @@ JNIEXPORT jobject JNICALL Java_mapnik_FeatureSet_getPropertyNames
mapnik::feature_impl::iterator end = (*fp)->end();
for ( ;itr!=end; ++itr)
{
std::string const& name(boost::get<0>(*itr));
std::string const& name(std::get<0>(*itr));
env->CallBooleanMethod(ret, METHOD_HASHSET_ADD, env->NewStringUTF(name.c_str()));
}

return ret;
TRAILER(0);
}

// http://en.wikipedia.org/wiki/Java_Native_Interface#Mapping_types
#if MAPNIK_VERSION >= 200200
typedef mapnik::value_integer value_integer;
#else
typedef int value_integer;
#endif

class value_to_java: public boost::static_visitor<jobject> {
JNIEnv* env;
public:
value_to_java(JNIEnv* aenv): env(aenv) {
}


jobject operator()(value_integer value) const {
jobject operator()(mapnik::value_integer value) const {
#ifdef BIGINT
return env->CallStaticObjectMethod(CLASS_LONG, METHOD_LONG_VALUEOF, value);
#else
return env->CallStaticObjectMethod(CLASS_INTEGER, METHOD_INTEGER_VALUEOF, value);
#endif
}

jobject operator()(bool value) const {
jobject operator()(mapnik::value_bool value) const {
return env->CallStaticObjectMethod(CLASS_BOOLEAN, METHOD_BOOLEAN_VALUEOF, value);
}

jobject operator()(double value) const {
jobject operator()(mapnik::value_double value) const {
return env->CallStaticObjectMethod(CLASS_DOUBLE, METHOD_DOUBLE_VALUEOF, value);
}

jobject operator()(std::string const& value) const {
return env->NewStringUTF(value.c_str());
}

jobject operator()(icu::UnicodeString const& value) const {
return env->NewString(value.getBuffer(), value.length());
jobject operator()(mapnik::value_unicode_string const& value) const {
return env->NewString(reinterpret_cast<const jchar*>(value.getBuffer()), value.length());
}

jobject operator()(mapnik::value_null const& value) const {
Expand All @@ -198,7 +193,7 @@ JNIEXPORT jobject JNICALL Java_mapnik_FeatureSet_getProperty

// Convert the value
mapnik::value_type const& value= (*fp)->get(name.stringz);
return boost::apply_visitor(value_to_java(env), value.base());
return mapnik::value_type::visit(value, value_to_java(env));
TRAILER(0);
}

2 changes: 1 addition & 1 deletion csrc/class_featuretypestyle.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ JNIEXPORT jobject JNICALL Java_mapnik_FeatureTypeStyle_collectAttributes

std::set<std::string> attrs;
mapnik::attribute_collector collector(attrs);
BOOST_FOREACH(mapnik::rule const& r, rules) {
for (mapnik::rule const& r : rules) {
collector(r);
}

Expand Down
56 changes: 36 additions & 20 deletions csrc/class_geometry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,27 @@ JNIEXPORT jint JNICALL Java_mapnik_Geometry_getType
(JNIEnv *env, jobject gobj)
{
PREAMBLE;
mapnik::geometry_type* g=LOAD_GEOMETRY_POINTER(gobj);
return g->type();
mapnik::geometry::geometry<double>* g=LOAD_GEOMETRY_POINTER(gobj);
return mapnik::geometry::geometry_type(*g);
TRAILER(0);
}


struct VertexCollector
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I'm trying to do here is to collect all the vertices in an arbitrary geometry<double> into an std::vector, using geometry::vertex_processor. I think it should work, but it is certainly inefficient to do this on every call to getNumPoints or getVertex. => The (internal) Java interface should be refactored to do this more efficiently.

{
std::vector<std::tuple<unsigned, double, double>> vertices;

template<typename T>
void operator()(const T &vertex_adapter)
{
unsigned command;
double x, y;
while ((command = vertex_adapter.vertex(&x, &y)) != mapnik::SEG_END)
vertices.push_back(std::make_tuple(command, x, y));
}
};


/*
* Class: mapnik_Geometry
* Method: getNumPoints
Expand All @@ -22,12 +38,13 @@ JNIEXPORT jint JNICALL Java_mapnik_Geometry_getNumPoints
(JNIEnv *env, jobject gobj)
{
PREAMBLE;
mapnik::geometry_type* g=LOAD_GEOMETRY_POINTER(gobj);
#if MAPNIK_VERSION >= 200100
return g->size();
#else
return g->num_points();
#endif
mapnik::geometry::geometry<double>* g = LOAD_GEOMETRY_POINTER(gobj);

VertexCollector vertex_collector;
mapnik::geometry::vertex_processor<VertexCollector> processor(vertex_collector);
processor(*g);

return vertex_collector.vertices.size();
TRAILER(0);
}

Expand All @@ -40,20 +57,19 @@ JNIEXPORT jint JNICALL Java_mapnik_Geometry_getVertex
(JNIEnv *env, jobject gobj, jint pos, jobject coord)
{
PREAMBLE;
mapnik::geometry_type* g=LOAD_GEOMETRY_POINTER(gobj);
double x=0, y=0;
#if MAPNIK_VERSION >= 200100
unsigned ret=g->vertex(pos, &x, &y);
#else
unsigned ret=g->get_vertex(pos, &x, &y);
#endif
mapnik::geometry::geometry<double>* g=LOAD_GEOMETRY_POINTER(gobj);

VertexCollector vertex_collector;
mapnik::geometry::vertex_processor<VertexCollector> processor(vertex_collector);
processor(*g);
const std::tuple<unsigned, double, double> &command_and_coords = vertex_collector.vertices.at(pos);

if (coord) {
env->SetDoubleField(coord, FIELD_COORD_X, x);
env->SetDoubleField(coord, FIELD_COORD_Y, y);
env->SetDoubleField(coord, FIELD_COORD_X, std::get<1>(command_and_coords));
env->SetDoubleField(coord, FIELD_COORD_Y, std::get<2>(command_and_coords));
}

return ret;
return std::get<0>(command_and_coords);
TRAILER(0);
}

Expand All @@ -66,8 +82,8 @@ JNIEXPORT jobject JNICALL Java_mapnik_Geometry_getEnvelope
(JNIEnv *env, jobject gobj)
{
PREAMBLE;
mapnik::geometry_type* g=LOAD_GEOMETRY_POINTER(gobj);
return box2dFromNative(env, g->envelope());
mapnik::geometry::geometry<double>* g=LOAD_GEOMETRY_POINTER(gobj);
return box2dFromNative(env, mapnik::geometry::envelope(*g));
TRAILER(0);
}

16 changes: 8 additions & 8 deletions csrc/class_image.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ JNIEXPORT jlong JNICALL Java_mapnik_Image_alloc__II
(JNIEnv *env, jclass c, jint width, jint height)
{
PREAMBLE;
mapnik::image_32* im=new mapnik::image_32(width, height);
mapnik::image_rgba8* im=new mapnik::image_rgba8(width, height);
return FROM_POINTER(im);
TRAILER(0);
}
Expand All @@ -25,9 +25,9 @@ JNIEXPORT jlong JNICALL Java_mapnik_Image_alloc__Lmapnik_Image_2
throw_runtime_exception(env, "Image cannot be null in call to constructor");
return 0;
}
mapnik::image_32* other=LOAD_IMAGE_POINTER(iobjother);
mapnik::image_rgba8* other=LOAD_IMAGE_POINTER(iobjother);

mapnik::image_32* im=new mapnik::image_32(*other);
mapnik::image_rgba8* im=new mapnik::image_rgba8(*other);
return FROM_POINTER(im);

TRAILER(0);
Expand All @@ -42,7 +42,7 @@ JNIEXPORT void JNICALL Java_mapnik_Image_dealloc
(JNIEnv * env, jobject, jlong ptr)
{
PREAMBLE;
mapnik::image_32* im=static_cast<mapnik::image_32*>(TO_POINTER(ptr));
mapnik::image_rgba8* im=static_cast<mapnik::image_rgba8*>(TO_POINTER(ptr));
if (im) {
delete im;
}
Expand All @@ -58,7 +58,7 @@ JNIEXPORT jint JNICALL Java_mapnik_Image_getWidth
(JNIEnv *env, jobject imobj)
{
PREAMBLE;
mapnik::image_32* im=LOAD_IMAGE_POINTER(imobj);
mapnik::image_rgba8* im=LOAD_IMAGE_POINTER(imobj);
return im->width();
TRAILER(0);
}
Expand All @@ -72,7 +72,7 @@ JNIEXPORT jint JNICALL Java_mapnik_Image_getHeight
(JNIEnv *env, jobject imobj)
{
PREAMBLE;
mapnik::image_32* im=LOAD_IMAGE_POINTER(imobj);
mapnik::image_rgba8* im=LOAD_IMAGE_POINTER(imobj);
return im->height();
TRAILER(0);
}
Expand All @@ -86,7 +86,7 @@ JNIEXPORT void JNICALL Java_mapnik_Image_saveToFile
(JNIEnv *env, jobject imobj, jstring filenamej, jstring typej)
{
PREAMBLE;
mapnik::image_32* im=LOAD_IMAGE_POINTER(imobj);
mapnik::image_rgba8* im=LOAD_IMAGE_POINTER(imobj);
refjavastring filename(env, filenamej);
refjavastring type(env, typej);

Expand All @@ -103,7 +103,7 @@ JNIEXPORT jbyteArray JNICALL Java_mapnik_Image_saveToMemory
(JNIEnv *env, jobject imobj, jstring typej)
{
PREAMBLE;
mapnik::image_32* im=LOAD_IMAGE_POINTER(imobj);
mapnik::image_rgba8* im=LOAD_IMAGE_POINTER(imobj);
refjavastring type(env, typej);

std::string datastring=mapnik::save_to_string(*im, type.stringz);
Expand Down
8 changes: 4 additions & 4 deletions csrc/class_layer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ JNIEXPORT jdouble JNICALL Java_mapnik_Layer_getMinZoom
{
PREAMBLE;
mapnik::layer* layer=LOAD_LAYER_POINTER(layerobj);
return layer->min_zoom();
return layer->minimum_scale_denominator();
TRAILER(0);
}

Expand All @@ -163,7 +163,7 @@ JNIEXPORT void JNICALL Java_mapnik_Layer_setMinZoom
{
PREAMBLE;
mapnik::layer* layer=LOAD_LAYER_POINTER(layerobj);
layer->set_min_zoom(z);
layer->set_minimum_scale_denominator(z);
TRAILER_VOID;
}

Expand All @@ -177,7 +177,7 @@ JNIEXPORT jdouble JNICALL Java_mapnik_Layer_getMaxZoom
{
PREAMBLE;
mapnik::layer* layer=LOAD_LAYER_POINTER(layerobj);
return layer->max_zoom();
return layer->maximum_scale_denominator();
TRAILER(0);
}

Expand All @@ -191,7 +191,7 @@ JNIEXPORT void JNICALL Java_mapnik_Layer_setMaxZoom
{
PREAMBLE;
mapnik::layer* layer=LOAD_LAYER_POINTER(layerobj);
layer->set_max_zoom(z);
layer->set_maximum_scale_denominator(z);
TRAILER_VOID;
}

Expand Down
4 changes: 2 additions & 2 deletions csrc/class_map.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ JNIEXPORT void JNICALL Java_mapnik_MapDefinition_removeLayer
{
PREAMBLE;
mapnik::Map* map=LOAD_MAP_POINTER(mapobject);
map->removeLayer(index);
map->remove_layer(index);
TRAILER_VOID;
}

Expand All @@ -179,7 +179,7 @@ JNIEXPORT void JNICALL Java_mapnik_MapDefinition_addLayer
mapnik::layer* layer=
static_cast<mapnik::layer*>(TO_POINTER(env->GetLongField(layerobject, FIELD_PTR)));

map->addLayer(*layer);
map->add_layer(*layer);
TRAILER_VOID;
}

Expand Down
Loading