[FR] Are there J2CL annotations for this library?

[aschleck]

Hi folks,

I miss being part of Geo, hope you all are doing well! I was wondering if there are J2CL annotations for this library (@JsMethod/@JsType/etc), and if so would it be possible to update this library and release them?

Thank you!

[TorreyHoffman]

I have J2CL annotations internally. I'm hoping to update Github with our (extensive) internal improvements soon.

[aschleck]

Thank you so much for pushing the new version! Super exciting. Is it also possible to include the supersource Platform.native.js and what not?

As it is, J2CL compilation complains about:

Error:Platform.java:33: The method IEEEremainder(double, double) is undefined for the type Math
Error:Platform.java:38: The method getExponent(double) is undefined for the type Math
Error:Platform.java:57: The method printf(String, Object[]) is undefined for the type PrintStream
Error:Platform.java:68: The method format(String, Object[]) is undefined for the type String
Error:Platform.java:81: The method format(Locale, String, double) is undefined for the type String
Error:Platform.java:139: The method ulp(double) is undefined for the type Math
Error:Platform.java:147: The method nextAfter(double, double) is undefined for the type Math
Error:S2IndexingHelperImpl.java:309: The method format(String, String, long) is undefined for the type String

[TorreyHoffman]

Hi, sorry that the J2CL isn't quite there yet. Getting it working is a priority for me, but I know very little about how J2CL is used, especially outside of Google, and haven't yet spent the time reading documentation to figure it out. If you have advice on how the J2CL packaging and build should be set up that would be welcome.

For instance, do J2CL users also use Maven, and if so what Maven pom,xml changes would I use? A link to a working example would be great.

[aschleck]

Hmm that's a good question! You can see my project here: https://github.com/aschleck/trailcatalog . I think anyone using J2CL like this is probably using Bazel. And if you look at my project I need to note that I have an extra complication because I am not using ClosureJS (so I have to do the Java -> ClosureJS conversion and then wrap it into TypeScript. But for S2 itself let's just focus on the Java -> ClosureJS part and ignore the TypeScript since I think that's just a me-problem.)

First thing I did was make a BUILD file for this S2 repository. It would actually be amazing if you folks added Bazel BUILD support to this repository, but I wasn't sure if that was too much to ask. I recognize that everyone at Google is very busy.

package(default_visibility = ["//visibility:public"])

load("@com_google_j2cl//build_defs:rules.bzl", "j2cl_library")

j2cl_library(
    name = "s2-j2cl",
    srcs = glob([
        "library/src/**/*.java",
        "library/src/super-j2cl/**/*.java",
        "library/src/super-j2cl/**/*.js",
    ], exclude = [
        "library/src/com/google/common/geometry/Platform.java",
    ]),
    deps = [
        "@com_google_code_findbugs_jsr305-j2cl",
        "@com_google_elemental2//:elemental2-promise-j2cl",
        "@com_google_errorprone_error_prone_annotations-j2cl",
        "@com_google_guava-j2cl",
        "@com_google_j2cl//:jsinterop-annotations-j2cl",
        "@com_google_j2objc_annotations-j2cl",
        "@org_checkerframework_checker_qual-j2cl",
    ],
)

And then you can see I depend on that in the transitive dependencies here https://github.com/aschleck/trailcatalog/blob/main/java/org/trailcatalog/s2/BUILD#L32 (via https://github.com/aschleck/trailcatalog/blob/main/java/org/trailcatalog/s2/BUILD#L53).

In the S2 BUILD file I glob super-j2cl to pick up a J2CL compatible Platform.java via super sourcing. I am fairly certain that inside google3 there is a super sourced Platform.java or Platform.java.js file that is high quality, but in the absence of that I made a low quality version. I think I am doing something that is a little too complicated so I would defer to whatever is in google3 and already works.

diff --git a/library/src/com/google/common/geometry/S2IndexingHelperImpl.java b/library/src/com/google/common/geometry/S2IndexingHelperImpl.java
index 9a4301d..f88f918 100644
--- a/library/src/com/google/common/geometry/S2IndexingHelperImpl.java
+++ b/library/src/com/google/common/geometry/S2IndexingHelperImpl.java
@@ -306,7 +306,7 @@ public class S2IndexingHelperImpl implements S2IndexingHelper {

     @Override
     public String toString() {
-      return String.format("%s:%016X", type.name(), cellId);
+      return Platform.formatString("%s:%016X", type.name(), cellId);
     }
   }

diff --git a/library/src/super-j2cl/com/google/common/geometry/Platform.java b/library/src/super-j2cl/com/google/common/geometry/Platform.java
new file mode 100644
index 0000000..c9115da
--- /dev/null
+++ b/library/src/super-j2cl/com/google/common/geometry/Platform.java
@@ -0,0 +1,118 @@
+/*
+ * Copyright 2013 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.google.common.geometry;
+
+import com.google.common.annotations.GwtCompatible;
+import java.io.PrintStream;
+import java.math.BigDecimal;
+import java.util.Locale;
+import java.util.logging.Logger;
+import jsinterop.annotations.JsMethod;
+
+/**
+ * Contains utility methods which require different GWT client and server implementations. This
+ * contains the server side implementations.
+ */
+@GwtCompatible(emulated = true)
+final class Platform {
+
+  private Platform() {}
+
+  /** @see Math#IEEEremainder(double, double) */
+  @JsMethod
+  static native double IEEEremainder(double f1, double f2);
+
+  /** @see Math#getExponent(double) */
+  @JsMethod
+  static native int getExponent(double d);
+
+  /**
+   * Returns the {@link Logger} for the class.
+   *
+   * @see Logger#getLogger(String)
+   */
+  static Logger getLoggerForClass(Class<?> clazz) {
+    return Logger.getLogger(clazz.getCanonicalName());
+  }
+
+  /**
+   * Invokes {@code stream.printf} with the arguments. The GWT client just prints the format string
+   * and the arguments separately. Using this method is not recommended; you should instead
+   * construct strings with normal string concatenation whenever possible, so it will work the same
+   * way in normal Java and GWT client versions.
+   */
+  @JsMethod
+  static native void printf(PrintStream stream, String format, Object... params);
+
+  /**
+   * Returns {@code String.format} with the arguments. The GWT client just returns a string
+   * consisting of the format string with the parameters concatenated to the end of it. Using this
+   * method is not recommended; you should instead construct strings with normal string
+   * concatenation whenever possible, so it will work the same way in normal Java and GWT client
+   * versions.
+   */
+  @JsMethod
+  static native String formatString(String format, Object... prams);
+
+  /**
+   * Formats the double as a string and removes unneeded trailing zeros, to behave the same as
+   * printf("%.15g",d) in C++. The Javascript implementation does NOT have identical behavior.
+   */
+  @JsMethod
+  static native String formatDouble(double d);
+
+  /** A portable way to hash a double value. */
+  public static long doubleHash(double value) {
+    return Double.doubleToLongBits(value);
+  }
+
+  /**
+   * Returns the sign of the determinant of the matrix constructed from the three column vectors
+   * {@code a}, {@code b}, and {@code c}. This operation is very robust for small determinants, but
+   * is extremely slow and should only be used if performance is not a concern or all faster
+   * techniques have been exhausted.
+   */
+  public static int sign(S2Point a, S2Point b, S2Point c) {
+    Real bycz = Real.mul(b.y, c.z);
+    Real bzcy = Real.mul(b.z, c.y);
+    Real bzcx = Real.mul(b.z, c.x);
+    Real bxcz = Real.mul(b.x, c.z);
+    Real bxcy = Real.mul(b.x, c.y);
+    Real bycx = Real.mul(b.y, c.x);
+    Real bcx = bycz.sub(bzcy);
+    Real bcy = bzcx.sub(bxcz);
+    Real bcz = bxcy.sub(bycx);
+    Real x = bcx.mul(a.x);
+    Real y = bcy.mul(a.y);
+    Real z = bcz.mul(a.z);
+    return x.add(y).add(z).signum();
+  }
+
+  /**
+   * Returns the next representable value in the direction of 'dir' starting from 'x', emulating the
+   * behavior of {@link Math#nextAfter}.
+   */
+  @JsMethod
+  public static native double nextAfter(double x, double dir);
+
+  /**
+   * Returns a new {@code BigDecimal} instance whose value is the exact decimal representation of
+   * {@code x}, emulating the behavior of {@link BigDecimal#BigDecimal(double)}.
+   */
+  static BigDecimal newBigDecimal(double x) {
+    return new BigDecimal(x);
+  }
+}
diff --git a/library/src/super-j2cl/com/google/common/geometry/Platform.native.js b/library/src/super-j2cl/com/google/common/geometry/Platform.native.js
new file mode 100644
index 0000000..72abc7c
--- /dev/null
+++ b/library/src/super-j2cl/com/google/common/geometry/Platform.native.js
@@ -0,0 +1,51 @@
+Platform.IEEEremainder = function(f1, f2) {
+  // f1 - f2 * round_towards_even(f1 / f2)
+  const div = f1 / f2;
+  let rounded;
+  const remainder = div % 1;
+  if (remainder === 0.5 || remainder === -0.5) {
+    const fn = div < 0 ? Math.ceil : Math.floor;
+    rounded = fn(div) + fn(div % 2);
+  } else {
+    rounded = Math.round(div);
+  }
+  return f1 - (f2 * rounded)
+}
+
+// https://stackoverflow.com/questions/9383593/extracting-the-exponent-and-mantissa-of-a-javascript-number
+const gE_float = new Float64Array(1);
+const gE_view = new DataView(gE_float.buffer);
+Platform.getExponent = function(d) {
+  if (d === 0) {
+    return -Infinity;
+  }
+  gE_view.setFloat64(0, d, /* littleEndian= */ true);
+  return ((gE_view.getUint8(7) & 0x7f) << 4 | gE_view.getUint8(6) >> 4) - 0x3ff;
+};
+
+Platform.printf = function(stream, format, params) {
+  stream.printf(format);
+  stream.printf(params);
+};
+
+Platform.formatString = function(format, params) {
+  return `$format: ${params.join(' ')}`;
+};
+
+Platform.formatDouble = function(d) {
+  return String(d);
+};
+
+const nA_float = new Float64Array(1);
+const nA_view = new DataView(nA_float.buffer);
+const ONE = BigInt(1);
+const NEGATIVE_ONE = BigInt(-1);
+Platform.nextAfter = function(x, dir) {
+  nA_view.setFloat64(0, x, true);
+  if (dir < 0) {
+    nA_view.setBigUint64(0, nA_view.getBigUint64(0, true) + NEGATIVE_ONE, true);
+  } else {
+    nA_view.setBigUint64(0, nA_view.getBigUint64(0, true) + ONE, true);
+  }
+  return nA_view.getFloat64(0, true);
+};
\ No newline at end of file

Does that make any sense? What do you think?

[aschleck]

Thanks so much releasing the latest code @TorreyHoffman! I was looking at upgrading my j2cl project to use the new S2 but am a little puzzled by how to support the fastutil implementation under j2cl since it uses JNI. Would you be willing to share any information on how Google is making fastutil dependencies work under j2cl?

I could imagine super-sourcing the S2 code to use standard Java collections objects under JS, or possibly super-sourcing the entire fastutil library to use standard Java collections, but am a bit confused.

I did also notice

s2-geometry-library-java/library/src/com/google/common/geometry/S2IndexingHelperImpl.java

Line 311 in 4a6abc1

return String.format("%s:%016X", type.name(), cellId);

should use Platform.formatString instead of String.format so perhaps the answer is you don't support j2cl anymore? :D

[jmr]

I don't know much about Java or J2CL, but I just switched the String.format call to Platform.formatString, so it will be in the next update.

What is the native code in fastutil? I don't see it.

[TorreyHoffman]

J2CL in the open source release isn't supported yet, sorry about that! We do support J2CL for S2 internally, and the J2CL annotations we've added to the S2 code are included in the open source release, so it's part way there.

Fastutil doesn't support J2CL as provided from upstream, so that's a problem, but I am pretty sure Fastutil has no JNI.

To make the open source release of S2 officially J2CL-compatible, we'd need to provide the Platform.java used on J2CL, and upstream the changes we've made to Fastutil internally to make it work under J2CL. That last one is a challenge.

Making Fastutil work with J2CL wasn't rocket science, but was a bit of a grind, and it is far from complete. We've split Fastutil into smaller compilation targets, and only the ones needed in S2 have been made J2CL compatible. That leaves out all of Fastutil's IO code, Serialization support, and many other things. That would be enough for you, of course, but wouldn't be sufficient to make all of Fastutil J2CL compatible, which is why I haven't really considered upstreaming it. The actual Fastutil changes were mostly adding @GwtIncompatible annotations where needed (quite a lot of those), and a few minor code changes.

I could at least make my Fastutil changes public as a diff. I could also provide the J2CL version of our Platform.java, and then at least you'd have the missing pieces to use S2 under J2CL.

[aschleck]

Thank you both for such a thoughtful response! My apologies, I got confused by fastutil using the c preprocessor (and outputting files with a .c file extension) but you're correct it is producing pure Java. That makes sense that you ended up patching fastutil internally.

If you could make your fastutil diff and your Platform.java (and Platform.native.js if you have one) public that'd be fabulous. I have a Platform.native.js at https://github.com/aschleck/s2-geometry-library-java/blob/master/library/src/super-j2cl/com/google/common/geometry/Platform.native.js but I am certain yours is much better than mine and it'd be great to not have to play catch-up with your releases. Since Bazel allows applying diffs during workspace construction I think it'd be fairly easy to patch fastutil in my repo and have everything Just Work™ for Bazel users. Perhaps some day, if it actually is easy, it'd be worth upstreaming a MODULE.bazel here but don't see any reason to do that for now

[jmr]

Perhaps some day, if it actually is easy, it'd be worth upstreaming a MODULE.bazel here but don't see any reason to do that for now

Filed #46 for this.