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

Distinguish offsets and padding from arrays in expressions #10770

Closed
1ec5 opened this issue Dec 20, 2017 · 1 comment
Closed

Distinguish offsets and padding from arrays in expressions #10770

1ec5 opened this issue Dec 20, 2017 · 1 comment
Labels
archived Archived because of inactivity Core The cross-platform C++ core, aka mbgl needs discussion runtime styling

Comments

@1ec5
Copy link
Contributor

1ec5 commented Dec 20, 2017

The style specification has never made a strong distinction between offsets, edge padding, and other arrays – they’re all represented by JSON arrays. However, mbgl has represented offsets as std::array<float, 2> and edge padding as std::array<float, 4>. Since #6079, we’ve represented offsets as CGVectors and edge padding as NSEdgeInset on macOS and UIEdgeInset on iOS.

Using these structs has been particularly useful because the screen origin differs between the two platforms – the upper-left on iOS versus the lower-left on macOS – resulting in different interpretations of these types. Moreover, edge padding is conventionally expressed in counterclockwise order on both iOS and macOS, as opposed to the clockwise order defined in the style specification.

Now that the JSON expression syntax introduces general-purpose array literals, mbgl::style::expression doesn’t appear to make the same distinctions that the rest of mbgl does: anything represented in JSON as an array is an std::vector. This is understandable, because an array used as an offset can be built from a general-purpose array. Unfortunately, when converting an mbgl::style::expression::Expression to an NSExpression, it’s very difficult – perhaps impossible – to know whether a given array literal should be a CGVector, NSEdgeInsets/UIEdgeInsets, or NSArray.

On the one hand, we can certainly land #10726 with a naïve implementation that treats these types as ordinary arrays, which would allow developers to use NSExpression’s built-in array literal syntax for all these types instead of fussing with NSValue. On the other hand, anything involving an offset or padding will be fragile and require additional documentation.

Would it be possible for mbgl to distinguish offset and padding arrays using a separate type such as std::array? Or would that require changing the style specification to introduce formal Offset and Padding types distinct from the Array type?

/cc @anandthakker @jfirebaugh @tobrun

@1ec5 1ec5 added Core The cross-platform C++ core, aka mbgl needs discussion runtime styling labels Dec 20, 2017
@stale stale bot added the archived Archived because of inactivity label Oct 30, 2018
@stale
Copy link

stale bot commented Dec 5, 2018

This issue has been automatically detected as stale because it has not had recent activity and will be archived. Thank you for your contributions.

@stale stale bot closed this as completed Dec 5, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
archived Archived because of inactivity Core The cross-platform C++ core, aka mbgl needs discussion runtime styling
Projects
None yet
Development

No branches or pull requests

1 participant