Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

expose mercator projection for custom layers #7485

Closed
ansis opened this issue Oct 25, 2018 · 3 comments · Fixed by #7488
Closed

expose mercator projection for custom layers #7485

ansis opened this issue Oct 25, 2018 · 3 comments · Fixed by #7488

Comments

@ansis
Copy link
Contributor

ansis commented Oct 25, 2018

Motivation

The matrix we give custom layers expects "mercator coordinates" as input. We provide no way of converting LngLat to mercator coordinates. We have this projection implemented internally but we don't expose this to users. Exposing it is mainly a design question.

If we can settle on a design I think we could sneak this into the next release.

@mourner can you help pick a design for this?

Relevant comments and workarounds:

Design Alternatives

I'm splitting this into two parts: a type for mercator coordinates and the methods used to convert.

Coordinate type

//1A: a three component array
typedef MercatorCoordinate = Array<number>;
[x, y, z]

// 1B: a class
class MercatorCoordinate {
    constructor(x, y, z) {
        this.x = x;
        this.y = y;
        this.z = z;
    }
}

projection method signature

Mercator coordinates are three dimensional. Should we support projecting and unprojecting features with altitude. How?

// 2A: using existing LngLat type
// downside: unprojecting and getting the altitude requires a separate method
projectMercator(LngLat: number, altitudeMeters?: number) : MercatorCoordinate
unprojectMercator(coord: MercatorCoordinate) : LngLat
// and maybe:
unprojectMercatorAltitude(coord: MercatorCoordinate) : number

// 2B: add optional altitude to LngLat type
LngLat(lng: number, lat: number, altitude?: number)
projectMercator(lngLatAlt: LngLat) : MercatorCoordinate
unprojectMercator(coord: MercatorCoordinate) : LngLatAlt

// 2C: like 2B except add a new LngLatAlt type
LngLatAlt(lng: number, lat: number, altitude?: number)
projectMercator(lngLatAlt: LngLat | LngLatAlt) : MercatorCoordinate
unprojectMercator(coord: MercatorCoordinate) : LngLatAlt

projection method location

// 3A on map class (requires an instantiated map)
map.projectMercator(...);
map.unprojectMercator(...);

// 3B without instantiated map
mapboxgl.projectMercator(...);
mapboxgl.unprojectMercator(...)

// 3C without instantiated map but namespaced
mapboxgl.mercator.project(...);
mapboxgl.mercator.unproject(...);

// 3D a mercator class
const mercator = new mapboxgl.MercatorProjection(optionalWorldSize);
mercator.project(...);
mercator.unproject(...);

// 3E like 3D but a bit more and with a helper for constructing the appropriate matrix for an area. Broad sketch:
const origin = LngLat({ lng: -79, lat: 42 });
const mercator new mapboxgl.MercatorProjection(origin)
mercator.project(...);
mercator.unproject(...);
map.getViewProjectionMatrix(mercator)

// 3F static method on MercatorCoordinate type
MercatorCoordinate.project(...)
MercatorCoordinate.unproject(...)
// alternative named `.fromLngLat(...)` and `toLngLat(...)`

// 3G static method for projection, regular method for unprojection
const coord = MercatorCoordinate.project(...);
const lngLat = coord.uproject();

// 3H constructor on MercatorCoordinate type
// constructor(xOrLngLat: LngLat | number, yOrAltitude?: number, z?: number)
const coord = new MercatorCoordinate(lngLat, altitude);
const lngLat = coord.unproject();

Design

I'm not sure which is better. @mourner I need your design expertise.

I'm slightly leaning towards 1B over 1A because it makes the typing specific in user code but I could definitely be swayed towards 1A if someone has a strong preference for that.

I kind of like the design adding altitude to LngLat but that might be a big change that we want to spend more time considering. For example, is the altitude the altitude above the surface? or above the wgs84 spheroid? how does this work for flat maps? how would this work if we added 3d terrain? 2A is nice because it sidesteps these questions but it is not as nice.

For part 3, I think it's good to be able to use this without an instantiated map. I think I like 3G because it provides separation and keeps the projection close to the type. I like 3D because it namespaces the methods, allows for future expansion to 3E-like things. I like 3A and 3B because they are simple.

Concepts

Documentation for the mercator type will explain the coordinate space. Documentation for custom layers will reference these methods. Custom layer examples will be switched to use this projection.

Implementation

Factoring worldSize out of here and exposing it. And adding tests and documentation.

@ansis
Copy link
Contributor Author

ansis commented Oct 25, 2018

I listed way too many possibilities. I think it's worth reading through them but overall I'm leaning towards 3Gish combined with 1A and 2A:

class MercatorCoordinate {
    constructor(x: number, y: number, z: number) {
        this.x = x;
        this.y = y;
        this.z = z;
    }

    // alternatively called project
    static fromLngLat(lngLat: LngLat, altitudeMeters?: number) : MercatorCoordinate { ... }

    // alternatively called unproject
    toLngLat() : LngLat { ... }

    // alternatively called unprojectAltitude
    toAltitude() : number { ... }
}

@asheemmamoowala
Copy link
Contributor

@ansis the MercatorCoordinate class with static methods is preferable to having instance methods on the Map object.

For cases where the transform matrix and coordinates are begin fed to GL shaders directly, might it be faster to convert the LngLats to Mercator in the shader? Its probably not the answer for this ticket, but I thought I'd share a snippet that I have used with gl-native:

vec2 p_pos;
p_pos.x = 180.0 + a_pos.x;
p_pos.y = 180.0 - degrees(log(tan(3.141592653589793/4.0 + 0.5 * radians(a_pos.y))));
float scale = pow(2.0, u_zoom) * 512.0 / 360.0;
vec2 x_pos = p_pos * scale;
gl_Position = u_matrix * vec4(x_pos, 0, 1);

ansis added a commit that referenced this issue Oct 26, 2018
ansis added a commit that referenced this issue Oct 26, 2018
@ansis ansis mentioned this issue Oct 26, 2018
7 tasks
@andrewharvey
Copy link
Collaborator

These helper methods would be great and make it much easier to build with CustomLayers 👍

I left a comment on the PR at #7488 (comment) about if we should be more explicit that this MercatorCoordinate coordinate system is not the same as ESPG:3857 web mercator, which I'm guessing is what most geospatial people would think of when hearing web mercator.

ansis added a commit that referenced this issue Oct 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants