Add projection expression syntax (#888)

[birkskyum]

This is a merging branch for

• Add projection expression syntax #888

Docs here
https://maplibre.org/maplibre-style-spec/projection/
https://maplibre.org/maplibre-style-spec/types/#projectiondefinition

Launch Checklist

• Confirm your changes do not include backports from Mapbox projects (unless with compliant license) - if you are not sure about this, please ask!
• Briefly describe the changes in this PR.
• Link to related issues.
• Include before/after visuals or gifs if this PR includes visual changes.
• Write tests for all new functionality.
• Document any changes to public APIs.
• Post benchmark scores.
• Add an entry to CHANGELOG.md under the ## main section.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 94.28571% with 4 lines in your changes missing coverage. Please review.

Project coverage is 92.78%. Comparing base (f91416f) to head (2eeba43).

Files with missing lines	Patch %	Lines
src/expression/index.ts	66.66%	2 Missing ⚠️
src/expression/types/projection_definition.ts	94.11%	1 Missing ⚠️
src/expression/values.ts	83.33%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #899   +/-   ##
=======================================
  Coverage   92.77%   92.78%           
=======================================
  Files         105      107    +2     
  Lines        4678     4739   +61     
  Branches     1323     1346   +23     
=======================================
+ Hits         4340     4397   +57     
- Misses        338      342    +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[HarelM]

See my additions here:

• Remove interpolate-projection #901

[birkskyum]

It's probably OK for the projection to have it's own type, but I am wondering a bit if it's too close to the StringType - the projection interpolation... it has, after removing the from: ProjectionPrimitive evolved into a generic string interpolation, or even an any transition catch-all, for whenever it's not possible to make an intermediate value, and the only option is to pass through the from, to and transition as an array like we do:

function number(from: number, to: number, t: number): number {
    return from + t * (to - from);
}

function projection(from: string, to: string, interpolation: number): Projection {
    return new Projection(from, to, interpolation);
}

function color(from: Color, to: Color, t: number, spaceKey: InterpolationColorSpace = 'rgb'): Color {
    switch (spaceKey) {

We could in principle have a fallback

function anyInterpolation<T,U>(from: T, to: U, t: number): [T, U, number]: AnyInterpolation {}

And for Projection, where we know the type

function string(from: string, to: string, t: number): StringInterpolate {}

[birkskyum]

The only problem I really have before merging this is that the interpolate-projection ran in GL JS, but the same expression now with interpolate is breaking... so i'm debugging

[HarelM]

Ok, if you need anything let me know.
The only thing I have left to consider in this PR is the following comment:
#888 (comment)

in proj4js they use the term ProjectionDefinition for the values related to the projection:
https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/proj4/index.d.ts

So the question is if to rename Projection to ProjectionDefinition and rename ProjectionConfigSpecifications to ProjectionSpecifications.
I'm not sure what I think about this... Let me know what you think...

[birkskyum]

Wrt. Projection -> ProjectionDefinition

I think that's a great idea - will get to it. "Projection" is overloaded by the class in geo/projection.

[birkskyum]

it's working again

[birkskyum]

I'm not sure if theres anything left

We could technically replace projectionDefinition type, with an expression compatible string-type. This can be done under the hood later anyway, affecting users just through a change to the url of the relevant documentation page section. Also, i'd prefer to introduce such big change separately, because a new string expression support, could affect all the existing string values. If we land it at some point we can just move projectionDefinition -> "string" / "expressionString". Haven't really thought it to completion, just have intuition there's a win-win kind of thing to get here, by both simplifying the code and bringing generic interpolation support for strings with same api as we have now for projections.

[birkskyum]

Maybe the array values in step could make sense to add

[HarelM]

I'm working to move the types to the right place in the folder structure, to make things a bit more organized. Just FYI...

[birkskyum]

I'm working to move the types to the right place in the folder structure, to make things a bit more organized. Just FYI...

Thanks, that does sounds great. Are you working on it in a separate cleanup PR, or on this branch?

[birkskyum]

Update:
I'm half-ways through refactoring to named exports, in separate PR. I find it'll be faster to work when that's in order, because we rename a lot (Projection -> ProjectDefinition etc.), and a named export will auto-propagate these change to the import statements and keep things consistent, where a default export does no such thing, so typecheck can't help.

Will wrap that up and get back to this

[HarelM]

I had the same observation lately when working with the code, this is needed indeed. Thanks!

[HarelM]

I've fixed the tests and addressed the comments I found when reviewing.
Feel free to review this.
I think this is ready to be merged.
Let me know if you are able to use it in maplibre-gl-js.
Note that I have removed the serialization to array of projectionDefinition as I'm not sure where this is needed.
I do see that color does get serialized to array somehow (saw it in test/integration/expression/tests/interpolate-hcl/linear/test.json test), but I couldn't figure out where it happens.

[birkskyum]

It's running fine in GL JS, with this syntax now

const exprestInt = createExpression(['interpolate', ['linear'], ['zoom'], 10, 'vertical-perspective', 12, 'mercator'], v8.projection.type)

Screen.Recording.2024-11-17.at.12.26.22.mov

I believe it's ready to go - I can't approve it, since I opened the PR.

[HarelM]

Great!