SDK runtime styling enumerations don’t match mbgl equivalents

[1ec5]

mbgl::LineJoinType has three values intended for public use, in order:

• Miter
• Bevel
• Round

By contrast, the iOS and macOS SDK’s MGLLineJoin type has:

• MGLLineJoinBevel
• MGLLineJoinRound
• MGLLineJoinMiter

So if you try to get the lineJoin of an MGLLineStyleLayer that has line-join set to round in the style JSON, you get an MGLStyleValue containing MGLLineJoinMiter, because MGLStyleValueTransformer assumes that the mbgl type was defined in the same order as the style specification. But the style specification defines these values in an object, so it’s unsafe to assume any order in the first place.

MGLStyleValueTransformer needs to explicitly map style JSON string values to Objective-C enumeration values. In this case, we’d use mbgl::LineJoinType_names. We should also add assertions or tests to catch these issues earlier: #6464.

The Android SDK doesn’t appear to suffer from this issue because it uses string constants throughout.

/cc @frederoni @jfirebaugh @bsudekum

[1ec5]

The Android SDK doesn’t appear to suffer from this issue because it uses string constants throughout.

Actually, I’m not sure if I follow the Android SDK source code well enough to draw this conclusion. @ivovandongen, can you verify that the Android SDK doesn’t assume a certain order for enumeration types?

[ivovandongen]

@1ec5 The Android API indeed uses strings throughout. The constants are defined in Java (generated). This is passed through jni to the conversion templates. So there is no redefinition of an enum in that sense.

[1ec5]

In this case, we’d use mbgl::LineJoinType_names.

This might get a little messy because the …_names tokens are actually #defines rather than types. It probably would’ve been a little cleaner before #6601 landed, when we handled enumerations entirely within a giant macro. But it should still be possible to deal with both C++ templates and #defines at the same time. 😬

[boundsj]

@1ec5

#7061 proposes a set of methods in MGLStyleValue_Private.h (so far there is only one) that will carry out the mapping on a per enum, value basis.

The problem seems to be 1) determining which enumeration is being set (i.e. line-join, line-cap, circle-pitch, etc) and then 2) determining which value is set in the JSON/mbgl object so the mapping can be done and the correct integer representation returned to the platform. Unfortunately this approach would expand like https://github.com/mapbox/mapbox-gl-native/blob/master/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/style/layers/PropertyFactory.java. Maybe it is a candidate for code generation?

Please let me know if I'm missing a better approach here (something to do with mbgl::LineJoinType_names ?). I'm stumped for now other than to resort to string representations like Android.

[1ec5]

Actually, I think you’re almost there. Indeed, we’ll need code generation, but the result will be pretty clean: #7061 (comment).

[boundsj]

This was fixed in #7061