Initial Android collator implementation.

Move Darwin collator implementation internals into an Impl class.

6 files changed, 235 insertions, 45 deletions

diff --git a/include/mbgl/style/expression/collator.hpp b/include/mbgl/style/expression/collator.hpp
index 8d14f536f4..499e205320 100644
--- a/include/mbgl/style/expression/collator.hpp
+++ b/include/mbgl/style/expression/collator.hpp
@@ -15,19 +15,21 @@ public:
     Collator(bool caseSensitive, bool diacriticSensitive, optional<std::string> locale = {});
 
     bool operator==(const Collator& other) const;
-    
+
     int compare(const std::string& lhs, const std::string& rhs) const;
-    
+
     std::string resolvedLocale() const;
-    
+
     // TODO: This serialization shouldn't ever be used, but since we're part of
     // mbgl::style::expression::Value we're expected to have a serialize()
     mbgl::Value serialize() const;
-    
+
 private:
-    bool caseSensitive;
-    bool diacriticSensitive;
-    optional<std::string> locale;
+    class Impl;
+    // TODO: Figure out right copy semantics for Collator -- sharing an underlying implementation
+    // should be fine within one thread, but it might be more idiomatic to explicitly copy the
+    // implementation?
+    std::shared_ptr<Impl> impl;
 };
 
 } // namespace expression
diff --git a/platform/android/config.cmake b/platform/android/config.cmake
index e92f5d2c78..40be54e934 100644
--- a/platform/android/config.cmake
+++ b/platform/android/config.cmake
@@ -38,8 +38,9 @@ macro(mbgl_platform_core)
 
         # Misc
         PRIVATE platform/android/src/text/collator.cpp
-        PRIVATE platform/android/src/text/local_glyph_rasterizer_jni.hpp
+        PRIVATE platform/android/src/text/collator_jni.hpp
         PRIVATE platform/android/src/text/local_glyph_rasterizer.cpp
+        PRIVATE platform/android/src/text/local_glyph_rasterizer_jni.hpp
         PRIVATE platform/android/src/logging_android.cpp
         PRIVATE platform/android/src/thread.cpp
         PRIVATE platform/default/string_stdlib.cpp
diff --git a/platform/android/src/jni.cpp b/platform/android/src/jni.cpp
index 2f6ed96ab0..6815276614 100755
--- a/platform/android/src/jni.cpp
+++ b/platform/android/src/jni.cpp
@@ -46,6 +46,7 @@
 #include "style/light.hpp"
 #include "snapshotter/map_snapshotter.hpp"
 #include "snapshotter/map_snapshot.hpp"
+#include "text/collator_jni.hpp"
 #include "text/local_glyph_rasterizer_jni.hpp"
 #include "java/lang.hpp"
 
diff --git a/platform/android/src/text/collator.cpp b/platform/android/src/text/collator.cpp
index f4d71b8de9..5e2d84d6f4 100644
--- a/platform/android/src/text/collator.cpp
+++ b/platform/android/src/text/collator.cpp
@@ -1,33 +1,155 @@
 #include <mbgl/style/expression/collator.hpp>
+#include <mbgl/util/platform.hpp>
+
+#include <jni/jni.hpp>
+
+#include "../attach_env.hpp"
+#include "collator_jni.hpp"
 
 // TODO: This Android collator stub should hook up to
 // https://developer.android.com/reference/java/text/Collator
 // via JNI
 
 namespace mbgl {
+namespace android {
+
+void Collator::registerNative(jni::JNIEnv& env) {
+    javaClass = *jni::Class<Collator>::Find(env).NewGlobalRef(env).release();
+}
+
+jni::Class<Collator> Collator::javaClass;
+
+jni::Object<Collator> Collator::getInstance(jni::JNIEnv& env, jni::Object<Locale> locale) {
+    using Signature = jni::Object<Collator>(jni::Object<Locale>);
+    auto method = javaClass.GetStaticMethod<Signature>(env, "getInstance");
+    return javaClass.Call(env, method, locale);
+}
+
+void Collator::setStrength(jni::JNIEnv& env, jni::Object<Collator> collator, jni::jint strength) {
+    using Signature = void(jni::jint);
+    auto static method = javaClass.GetMethod<Signature>(env, "setStrength");
+    collator.Call(env, method, strength);
+}
+
+jni::jint Collator::compare(jni::JNIEnv& env, jni::Object<Collator> collator, jni::String lhs, jni::String rhs) {
+    using Signature = jni::jint(jni::String, jni::String);
+    auto static method = javaClass.GetMethod<Signature>(env, "compare");
+    return collator.Call(env, method, lhs, rhs);
+}
+
+void Locale::registerNative(jni::JNIEnv& env) {
+    javaClass = *jni::Class<Locale>::Find(env).NewGlobalRef(env).release();
+}
+
+jni::Object<Locale> Locale::forLanguageTag(jni::JNIEnv& env, jni::String languageTag) {
+    using Signature = jni::Object<Locale>(jni::String);
+    auto method = javaClass.GetStaticMethod<Signature>(env, "forLanguageTag");
+    return javaClass.Call(env, method, languageTag);
+}
+
+jni::String Locale::toLanguageTag(jni::JNIEnv& env, jni::Object<Locale> locale) {
+    using Signature = jni::String();
+    auto static method = javaClass.GetMethod<Signature>(env, "toLanguageTag");
+    return locale.Call(env, method);
+}
+
+jni::Class<Locale> Locale::javaClass;
+
+} // namespace android
+
 namespace style {
 namespace expression {
 
-Collator::Collator(bool caseSensitive_, bool diacriticSensitive_, optional<std::string> locale_)
-    : caseSensitive(caseSensitive_)
-    , diacriticSensitive(diacriticSensitive_)
-    , locale(std::move(locale_))
+class Collator::Impl {
+public:
+    Impl(bool caseSensitive_, bool diacriticSensitive_, optional<std::string> locale_)
+        : caseSensitive(caseSensitive_)
+        , diacriticSensitive(diacriticSensitive_)
+    {
+        android::UniqueEnv env { android::AttachEnv() }; // TODO: How does it work to hold onto jni Objects long term?
+        jni::String languageTag = jni::Make<jni::String>(*env, locale_ ? *locale_ : "");
+        locale = android::Locale::forLanguageTag(*env, languageTag);
+        collator = android::Collator::getInstance(*env, locale);
+        if (!diacriticSensitive) {
+            // Only look for "base letter" differences, we'll look at case independently
+            android::Collator::setStrength(*env, collator, 0 /*PRIMARY*/);
+        } else if (diacriticSensitive && !caseSensitive) {
+            android::Collator::setStrength(*env, collator, 1 /*PRIMARY*/);
+        } else if (diacriticSensitive && caseSensitive) {
+            android::Collator::setStrength(*env, collator, 2 /*TERTIARY*/);
+        }
+        //  is tricky, no native support
+    }
+
+    bool operator==(const Impl& other) const {
+        return caseSensitive == other.caseSensitive &&
+                diacriticSensitive == other.diacriticSensitive &&
+                resolvedLocale() == other.resolvedLocale();
+    }
+
+    int compare(const std::string& lhs, const std::string& rhs) const {
+        android::UniqueEnv env { android::AttachEnv() };
+
+        jni::String jlhs = jni::Make<jni::String>(*env, lhs);
+        jni::String jrhs = jni::Make<jni::String>(*env, rhs);
+
+        jni::jint result = android::Collator::compare(*env, collator, jlhs, jrhs);;
+        if (!diacriticSensitive && caseSensitive) {
+            // java.text.Collator doesn't support a diacritic-insensitive/case-sensitive collation
+            // order, so we have to compromise a little here.
+            // (1) We use platform::lowercase to isolate case differences from other differences,
+            //     but it's not locale aware.
+            // (2) If we detect a case-only difference, we know the result is non-zero, but we
+            //     have to fall back to the base sort order, which _might_ pick up a diacritic
+            //     difference that ideally we'd ignore.
+            if (!result) {
+                // We compared at PRIMARY so we know there's no base letter difference
+                auto lowerLhs = platform::lowercase(lhs);
+                auto lowerRhs = platform::lowercase(rhs);
+                if (lowerLhs != lowerRhs) {
+                    // Case-only difference, fall back to base sort order
+                    result = lowerLhs < lowerRhs ? -1 : 1;
+                }
+            }
+
+        }
+        jni::DeleteLocalRef(*env, jlhs);
+        jni::DeleteLocalRef(*env, jrhs);
+
+        return result;
+    }
+
+    std::string resolvedLocale() const {
+        android::UniqueEnv env { android::AttachEnv() };
+        jni::String languageTag = android::Locale::toLanguageTag(*env, locale);
+        std::string result = jni::Make<std::string>(*env, languageTag);
+        jni::DeleteLocalRef(*env, languageTag);
+        return result;
+    }
+private:
+    jni::Object<android::Collator> collator;
+    jni::Object<android::Locale> locale;
+    bool caseSensitive;
+    bool diacriticSensitive;
+};
+
+
+Collator::Collator(bool caseSensitive, bool diacriticSensitive, optional<std::string> locale_)
+    : impl(std::make_unique<Impl>(caseSensitive, diacriticSensitive, std::move(locale_)))
 {}
 
 bool Collator::operator==(const Collator& other) const {
-    return caseSensitive == other.caseSensitive &&
-        diacriticSensitive == other.diacriticSensitive &&
-        locale == other.locale;
+    return *impl == *(other.impl);
 }
 
-int Collator::compare(const std::string&, const std::string&) const {
-    return 0;
+int Collator::compare(const std::string& lhs, const std::string& rhs) const {
+    return impl->compare(lhs, rhs);
 }
 
 std::string Collator::resolvedLocale() const {
-    static std::string placeholder;
-    return placeholder;
+    return impl->resolvedLocale();
 }
+
 mbgl::Value Collator::serialize() const {
     return mbgl::Value(true);
 }
diff --git a/platform/android/src/text/collator_jni.hpp b/platform/android/src/text/collator_jni.hpp
new file mode 100644
index 0000000000..e343fd3cdb
--- /dev/null
+++ b/platform/android/src/text/collator_jni.hpp
@@ -0,0 +1,53 @@
+#pragma once
+
+#include <mbgl/util/image.hpp>
+
+#include <jni/jni.hpp>
+
+/*
+    android::Collator is the JNI wrapper of
+    java/text/LocalGlyphRasterizer and java/text/Locale
+
+    // TODO: Since these aren't mapbox specific, should
+    // Collator/Locale wrappers be promoted to stand alone in
+    // android/src similar to bitmap.hpp?
+
+    mbgl::Collator is the portable interface
+    Both implementations are in collator.cpp
+ */
+
+namespace mbgl {
+namespace android {
+
+class Locale {
+public:
+    static constexpr auto Name() { return "java/text/Locale"; };
+
+    static jni::Object<Locale> forLanguageTag(jni::JNIEnv&, jni::String);
+
+    static jni::String toLanguageTag(jni::JNIEnv&, jni::Object<Locale>);
+
+    static jni::Class<Locale> javaClass;
+
+    static void registerNative(jni::JNIEnv&);
+
+};
+
+class Collator {
+public:
+    static constexpr auto Name() { return "java/text/Collator"; };
+
+    static jni::Object<Collator> getInstance(jni::JNIEnv&, jni::Object<Locale>);
+
+    static void setStrength(jni::JNIEnv&, jni::Object<Collator>, jni::jint);
+
+    static jni::jint compare(jni::JNIEnv&, jni::Object<Collator>, jni::String, jni::String);
+
+    static jni::Class<Collator> javaClass;
+
+    static void registerNative(jni::JNIEnv&);
+
+};
+
+} // namespace android
+} // namespace mbgl
diff --git a/platform/darwin/src/collator.mm b/platform/darwin/src/collator.mm
index c33e3f9473..bcb9dbe6e8 100644
--- a/platform/darwin/src/collator.mm
+++ b/platform/darwin/src/collator.mm
@@ -5,42 +5,53 @@
 namespace mbgl {
 namespace style {
 namespace expression {
+    
+class Collator::Impl {
+public:
+    Impl(bool caseSensitive, bool diacriticSensitive, optional<std::string> locale_)
+    : options(caseSensitive ? NSCaseInsensitiveSearch : 0 |
+              diacriticSensitive ? NSDiacriticInsensitiveSearch : 0)
+    , locale(locale_ ?
+                [[NSLocale alloc] initWithLocaleIdentifier:[NSString stringWithUTF8String:locale_->c_str()]] :
+                [NSLocale currentLocale])
+    {}
+    
+    bool operator==(const Impl& other) const {
+        return options == other.options &&
+            [[locale localeIdentifier] isEqualToString:[other.locale localeIdentifier]];
+    }
+    
+    int compare(const std::string& lhs, const std::string& rhs) const {
+        NSString* nsLhs = [NSString stringWithUTF8String:lhs.c_str()];
+        // TODO: verify "abc" != "abcde" -- the "range" argument seems strange to me
+        // https://developer.apple.com/documentation/foundation/nsstring/1414561-compare
+        
+        return [nsLhs compare:[NSString stringWithUTF8String:rhs.c_str()] options:options range:NSMakeRange(0, nsLhs.length) locale:locale];
+    }
+    
+    std::string resolvedLocale() const {
+        return [locale localeIdentifier].UTF8String;
+    }
+private:
+    NSStringCompareOptions options;
+    NSLocale* locale;
+};
 
-Collator::Collator(bool caseSensitive_, bool diacriticSensitive_, optional<std::string> locale_)
-    : caseSensitive(caseSensitive_)
-    , diacriticSensitive(diacriticSensitive_)
-    , locale(std::move(locale_))
-{}
 
+Collator::Collator(bool caseSensitive, bool diacriticSensitive, optional<std::string> locale_)
+    : impl(std::make_unique<Impl>(caseSensitive, diacriticSensitive, std::move(locale_)))
+{}
 
 bool Collator::operator==(const Collator& other) const {
-    return caseSensitive == other.caseSensitive &&
-        diacriticSensitive == other.diacriticSensitive &&
-        locale == other.locale;
+    return *impl == *(other.impl);
 }
 
 int Collator::compare(const std::string& lhs, const std::string& rhs) const {
-    NSStringCompareOptions options =
-        caseSensitive ? NSCaseInsensitiveSearch : 0 |
-        diacriticSensitive ? NSDiacriticInsensitiveSearch : 0;
-    
-    NSString* nsLhs = [NSString stringWithUTF8String:lhs.c_str()];
-    // TODO: verify "abc" != "abcde" -- the "range" argument seems strange to me
-    // https://developer.apple.com/documentation/foundation/nsstring/1414561-compare
-    
-    NSLocale* nsLocale = locale ?
-        [[NSLocale alloc] initWithLocaleIdentifier:[NSString stringWithUTF8String:locale->c_str()]] :
-        [NSLocale currentLocale];
-    
-    return [nsLhs compare:[NSString stringWithUTF8String:rhs.c_str()] options:options range:NSMakeRange(0, nsLhs.length) locale:nsLocale];
+    return impl->compare(lhs, rhs);
 }
 
 std::string Collator::resolvedLocale() const {
-    NSLocale* nsLocale = locale ?
-        [[NSLocale alloc] initWithLocaleIdentifier:[NSString stringWithUTF8String:locale->c_str()]] :
-        [NSLocale currentLocale];
-    
-    return [nsLocale localeIdentifier].UTF8String;
+    return impl->resolvedLocale();
 }
 mbgl::Value Collator::serialize() const {
     return mbgl::Value(true);