Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Remove faulty cast to uint64_t, explict casting for cluster API #13888

Merged
merged 2 commits into from
Feb 7, 2019
Merged
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
@@ -1,13 +1,21 @@
package com.mapbox.mapboxsdk.testapp.geometry;

import com.google.gson.JsonArray;
import com.mapbox.geojson.Feature;
import com.mapbox.geojson.FeatureCollection;
import com.mapbox.geojson.Point;
import com.mapbox.geojson.Polygon;
import com.mapbox.mapboxsdk.geometry.LatLng;
import com.mapbox.mapboxsdk.geometry.LatLngBounds;
import com.mapbox.mapboxsdk.style.expressions.Expression;
import com.mapbox.mapboxsdk.style.layers.PropertyFactory;
import com.mapbox.mapboxsdk.style.layers.SymbolLayer;
import com.mapbox.mapboxsdk.style.sources.CustomGeometrySource;
import com.mapbox.mapboxsdk.style.sources.GeoJsonSource;
import com.mapbox.mapboxsdk.style.sources.GeometryTileProvider;
import com.mapbox.mapboxsdk.testapp.activity.EspressoTest;
import com.mapbox.mapboxsdk.testapp.utils.Utils;

import org.junit.Test;

import static com.mapbox.geojson.Feature.fromGeometry;
Expand All @@ -18,6 +26,7 @@
import static com.mapbox.geojson.MultiPolygon.fromPolygon;
import static java.util.Collections.emptyList;
import static java.util.Collections.singletonList;
import static org.junit.Assert.assertFalse;

/**
* Instrumentation test to validate java geojson conversion to c++
Expand All @@ -44,7 +53,7 @@ public void testPointFeatureCollection() {
onMapView().perform(getMapboxMapAction((uiController, mapboxMap) -> {
mapboxMap.getStyle().addSource(
new CustomGeometrySource("test-id",
new CustomProvider(fromFeatures(singletonList(fromGeometry(Point.fromLngLat(0.0,0.0)))))
new CustomProvider(fromFeatures(singletonList(fromGeometry(Point.fromLngLat(0.0, 0.0)))))
)
);
mapboxMap.getStyle().addLayer(new SymbolLayer("test-id", "test-id"));
Expand Down Expand Up @@ -117,6 +126,34 @@ public void testMultiLineStringFeatureCollection() {
}));
}

@Test
public void testNegativeNumberPropertyConversion() {
validateTestSetup();
onMapView().perform(getMapboxMapAction((uiController, mapboxMap) -> {
LatLng latLng = new LatLng();
Feature feature = Feature.fromGeometry(Point.fromLngLat(latLng.getLongitude(), latLng.getLatitude()));

JsonArray foregroundJsonArray = new JsonArray();
foregroundJsonArray.add(0f);
foregroundJsonArray.add(-3f);
feature.addProperty("property", foregroundJsonArray);

GeoJsonSource source = new GeoJsonSource("source", feature);
mapboxMap.getStyle().addSource(source);

SymbolLayer layer = new SymbolLayer("layer", "source")
.withProperties(
PropertyFactory.iconOffset(Expression.get("property")),
PropertyFactory.iconImage("zoo-15")
);
mapboxMap.getStyle().addLayer(layer);

Utils.waitForLayer(uiController, mapboxMap, latLng, "layer");

assertFalse(mapboxMap.queryRenderedFeatures(mapboxMap.getProjection().toScreenLocation(latLng)).isEmpty());
}));
}

class CustomProvider implements GeometryTileProvider {

private FeatureCollection featureCollection;
Expand All @@ -130,4 +167,4 @@ public FeatureCollection getFeaturesForBounds(LatLngBounds bounds, int zoom) {
return featureCollection;
}
}
}
}
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
package com.mapbox.mapboxsdk.testapp.style;

import android.graphics.Color;
import android.support.test.espresso.UiController;
import android.support.test.runner.AndroidJUnit4;

import com.mapbox.geojson.Feature;
import com.mapbox.geojson.Point;
import com.mapbox.mapboxsdk.geometry.LatLng;
import com.mapbox.mapboxsdk.maps.MapboxMap;
import com.mapbox.mapboxsdk.style.expressions.Expression;
import com.mapbox.mapboxsdk.style.layers.CircleLayer;
import com.mapbox.mapboxsdk.style.layers.FillLayer;
Expand Down Expand Up @@ -55,6 +53,7 @@
import static com.mapbox.mapboxsdk.style.layers.PropertyFactory.fillOutlineColor;
import static com.mapbox.mapboxsdk.style.layers.PropertyFactory.textField;
import static com.mapbox.mapboxsdk.testapp.action.MapboxMapAction.invoke;
import static com.mapbox.mapboxsdk.testapp.utils.Utils.waitForLayer;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNull;
Expand Down Expand Up @@ -291,7 +290,7 @@ public void testConstFormatExpression() {
formatEntry("test")
);
layer.setProperties(textField(expression));
waitForLayer(uiController, mapboxMap, latLng);
waitForLayer(uiController, mapboxMap, latLng, "layer");
assertFalse(mapboxMap.queryRenderedFeatures(mapboxMap.getProjection().toScreenLocation(latLng), "layer")
.isEmpty());

Expand All @@ -314,7 +313,7 @@ public void testConstFormatExpressionFontScaleParam() {
formatEntry("test", formatFontScale(1.75))
);
layer.setProperties(textField(expression));
waitForLayer(uiController, mapboxMap, latLng);
waitForLayer(uiController, mapboxMap, latLng, "layer");
assertFalse(mapboxMap.queryRenderedFeatures(mapboxMap.getProjection().toScreenLocation(latLng), "layer")
.isEmpty());

Expand All @@ -340,7 +339,7 @@ public void testConstFormatExpressionTextFontParam() {
)
);
layer.setProperties(textField(expression));
waitForLayer(uiController, mapboxMap, latLng);
waitForLayer(uiController, mapboxMap, latLng, "layer");
assertFalse(
mapboxMap.queryRenderedFeatures(mapboxMap.getProjection().toScreenLocation(latLng), "layer").isEmpty()
);
Expand Down Expand Up @@ -371,7 +370,7 @@ public void testConstFormatExpressionAllParams() {
)
);
layer.setProperties(textField(expression));
waitForLayer(uiController, mapboxMap, latLng);
waitForLayer(uiController, mapboxMap, latLng, "layer");
assertFalse(
mapboxMap.queryRenderedFeatures(mapboxMap.getProjection().toScreenLocation(latLng), "layer").isEmpty()
);
Expand Down Expand Up @@ -404,7 +403,7 @@ public void testConstFormatExpressionMultipleInputs() {
formatEntry("\ntest2", formatFontScale(2))
);
layer.setProperties(textField(expression));
waitForLayer(uiController, mapboxMap, latLng);
waitForLayer(uiController, mapboxMap, latLng, "layer");
assertFalse(
mapboxMap.queryRenderedFeatures(mapboxMap.getProjection().toScreenLocation(latLng), "layer").isEmpty()
);
Expand Down Expand Up @@ -438,7 +437,7 @@ public void testVariableFormatExpression() {
)
);
layer.setProperties(textField(expression));
waitForLayer(uiController, mapboxMap, latLng);
waitForLayer(uiController, mapboxMap, latLng, "layer");
assertFalse(mapboxMap.queryRenderedFeatures(mapboxMap.getProjection().toScreenLocation(latLng), "layer")
.isEmpty());

Expand Down Expand Up @@ -468,7 +467,7 @@ public void testVariableFormatExpressionMultipleInputs() {
formatEntry("\ntest2", formatFontScale(2))
);
layer.setProperties(textField(expression));
waitForLayer(uiController, mapboxMap, latLng);
waitForLayer(uiController, mapboxMap, latLng, "layer");
assertFalse(mapboxMap.queryRenderedFeatures(mapboxMap.getProjection().toScreenLocation(latLng), "layer")
.isEmpty());

Expand All @@ -488,7 +487,7 @@ public void testFormatExpressionPlainTextCoercion() {
mapboxMap.getStyle().addLayer(layer);

layer.setProperties(textField("test"));
waitForLayer(uiController, mapboxMap, latLng);
waitForLayer(uiController, mapboxMap, latLng, "layer");
assertFalse(mapboxMap.queryRenderedFeatures(mapboxMap.getProjection().toScreenLocation(latLng), "layer")
.isEmpty());

Expand All @@ -513,7 +512,7 @@ public void testTextFieldFormattedArgument() {
new FormattedSection("\ntest", 0.5, new String[] {"Arial Unicode MS Regular", "DIN Offc Pro Regular"})
);
layer.setProperties(textField(formatted));
waitForLayer(uiController, mapboxMap, latLng);
waitForLayer(uiController, mapboxMap, latLng, "layer");
assertFalse(mapboxMap.queryRenderedFeatures(mapboxMap.getProjection().toScreenLocation(latLng), "layer")
.isEmpty());

Expand All @@ -522,18 +521,6 @@ public void testTextFieldFormattedArgument() {
});
}

private static final long WAIT_TIMEOUT = 5000;
private static final long WAIT_DELAY = 150;

private static void waitForLayer(UiController uiController, MapboxMap mapboxMap, LatLng latLng) {
int i = 0;
while (mapboxMap.queryRenderedFeatures(mapboxMap.getProjection().toScreenLocation(latLng), "layer").isEmpty()) {
i++;
assertFalse("Waiting for layer timed out", i * WAIT_DELAY > WAIT_TIMEOUT);
uiController.loopMainThreadForAtLeast(WAIT_DELAY);
}
}

private void setupStyle() {
invoke(mapboxMap, (uiController, mapboxMap) -> {
// Add a source
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package com.mapbox.mapboxsdk.testapp.utils;

import android.support.test.espresso.UiController;

import com.mapbox.mapboxsdk.geometry.LatLng;
import com.mapbox.mapboxsdk.maps.MapboxMap;

import static org.junit.Assert.assertFalse;

public class Utils {

private static final long WAIT_TIMEOUT = 5000;
private static final long WAIT_DELAY = 150;

public static void waitForLayer(UiController uiController, MapboxMap mapboxMap, LatLng latLng, String... layerIds) {
int i = 0;
while (mapboxMap.queryRenderedFeatures(mapboxMap.getProjection().toScreenLocation(latLng), layerIds).isEmpty()) {
i++;
assertFalse("Waiting for layer timed out", i * WAIT_DELAY > WAIT_TIMEOUT);
uiController.loopMainThreadForAtLeast(WAIT_DELAY);
}
}
}
2 changes: 0 additions & 2 deletions platform/android/core-files.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
"platform/android/src/gson/json_primitive.cpp",
"platform/android/src/image.cpp",
"platform/android/src/java/util.cpp",
"platform/android/src/math/math.cpp",
"platform/android/src/java_types.cpp",
"platform/android/src/jni.cpp",
"platform/android/src/logger.cpp",
Expand Down Expand Up @@ -142,7 +141,6 @@
"gson/json_object.hpp": "platform/android/src/gson/json_object.hpp",
"gson/json_primitive.hpp": "platform/android/src/gson/json_primitive.hpp",
"java/util.hpp": "platform/android/src/java/util.hpp",
"math/math.hpp": "platform/android/src/math/math.hpp",
"java_types.hpp": "platform/android/src/java_types.hpp",
"jni.hpp": "platform/android/src/jni.hpp",
"logger.hpp": "platform/android/src/logger.hpp",
Expand Down
11 changes: 1 addition & 10 deletions platform/android/src/gson/json_element.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
#include "json_array.hpp"
#include "json_object.hpp"
#include "json_primitive.hpp"
#include "../math/math.hpp"

namespace mbgl {
namespace android {
Expand Down Expand Up @@ -59,21 +58,13 @@ mbgl::Value JsonElement::convert(jni::JNIEnv &env, const jni::Object<JsonElement
static auto getAsBoolean = primitiveClass.GetMethod<jni::jboolean ()>(env, "getAsBoolean");
static auto getAsString = primitiveClass.GetMethod<jni::String ()>(env, "getAsString");
static auto getAsDouble = primitiveClass.GetMethod<jni::jdouble ()>(env, "getAsDouble");
static auto getAsInteger = primitiveClass.GetMethod<jni::jint ()>(env, "getAsInt");

if (jsonElement.Call(env, isJsonPrimitive)) {
auto primitive = jni::Cast(env, primitiveClass, jsonElement);
if (primitive.Call(env, isBoolean)) {
return bool(primitive.Call(env, getAsBoolean));
} else if (primitive.Call(env, isNumber)) {
auto value = primitive.Call(env, getAsDouble);
if (value == math::Math::rint(env, value)) {
// uint64_t
return static_cast<uint64_t>(primitive.Call(env, getAsInteger));
} else {
// double
return value;
}
return primitive.Call(env, getAsDouble);
} else if (primitive.Call(env, isString)) {
return jni::Make<std::string>(env, primitive.Call(env, getAsString));
} else {
Expand Down
2 changes: 0 additions & 2 deletions platform/android/src/jni.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@
#include "text/collator_jni.hpp"
#include "text/local_glyph_rasterizer_jni.hpp"
#include "logger.hpp"
#include "math/math.hpp"

namespace mbgl {
namespace android {
Expand Down Expand Up @@ -119,7 +118,6 @@ void registerNatives(JavaVM *vm) {
// Basic types
java::registerNatives(env);
java::util::registerNative(env);
math::registerNative(env);
PointF::registerNative(env);
RectF::registerNative(env);

Expand Down
13 changes: 0 additions & 13 deletions platform/android/src/math/math.cpp

This file was deleted.

26 changes: 0 additions & 26 deletions platform/android/src/math/math.hpp

This file was deleted.

3 changes: 3 additions & 0 deletions platform/android/src/style/sources/geojson_source.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ namespace android {

if (rendererFrontend) {
mbgl::Feature _feature = Feature::convert(env, feature);
_feature.properties["cluster_id"] = static_cast<uint64_t>(_feature.properties["cluster_id"].get<double>());
const auto featureExtension = rendererFrontend->queryFeatureExtensions(source.getID(), _feature, "supercluster", "children", {});
if (featureExtension.is<mbgl::FeatureCollection>()) {
return Feature::convert(env, featureExtension.get<mbgl::FeatureCollection>());
Expand All @@ -128,6 +129,7 @@ namespace android {

if (rendererFrontend) {
mbgl::Feature _feature = Feature::convert(env, feature);
_feature.properties["cluster_id"] = static_cast<uint64_t>(_feature.properties["cluster_id"].get<double>());
const std::map<std::string, mbgl::Value> options = { {"limit", static_cast<uint64_t>(limit)},
{"offset", static_cast<uint64_t>(offset)} };
auto featureExtension = rendererFrontend->queryFeatureExtensions(source.getID(), _feature, "supercluster", "leaves", options);
Expand All @@ -144,6 +146,7 @@ namespace android {

if (rendererFrontend) {
mbgl::Feature _feature = Feature::convert(env, feature);
_feature.properties["cluster_id"] = static_cast<uint64_t>(_feature.properties["cluster_id"].get<double>());
auto featureExtension = rendererFrontend->queryFeatureExtensions(source.getID(), _feature, "supercluster", "expansion-zoom", {});
if (featureExtension.is<mbgl::Value>()) {
auto value = featureExtension.get<mbgl::Value>();
Expand Down