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.

Sign up for GitHub

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

fix discontinuity when transitioning from globe to mercator in custom layer example #12544

Merged
merged 2 commits into from
Jan 30, 2023
Merged

fix discontinuity when transitioning from globe to mercator in custom layer example #12544

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
49caf37
fix discontinuity when transitioning from globe to mercator in globe …
akoylasar Jan 27, 2023
a017240
minor cleanup
akoylasar Jan 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions debug/satellites-custom-layer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,19 @@ const globeVertCode = `
uniform mat4 u_projection;
uniform mat4 u_globeToMercMatrix;
uniform float u_globeToMercatorTransition;
uniform vec2 u_centerInMerc;
uniform float u_pixelsPerMeterRatio;

void main() {
vec4 p = u_projection * u_globeToMercMatrix * vec4(a_pos_ecef, 1.);
p /= p.w;
if (u_globeToMercatorTransition > 0.) {
vec4 merc = u_projection * vec4(a_pos_merc, 1.);

vec4 merc = vec4(a_pos_merc, 1.);
merc.xy = (merc.xy - u_centerInMerc) * u_pixelsPerMeterRatio + u_centerInMerc;
merc.z *= u_pixelsPerMeterRatio;

merc = u_projection * merc;
Comment on lines 14 to +23
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@astojilj @akoylasar Are there mechanisms that we could adopt to include related code/functions to facilitate these conversions for the user? I imagine that this isn't trivial knowledge for them to successfully implement the conversions. As a user I would only want to care about my custom layer implementation (creating data in each of these world coordinates + projecting it).

My main concern is that this requires quite a bit of understanding of the internal workings of mapbox-gl. What I would want to care as a developer using globe custom layers is the following:

  • creating ECEF coordinates for globe
  • creating mercator coordinates for mercator
  • projecting each of these without thinking about internal details

The most minimal custom layer code I'd imagine someone would write:

gl_Position = custom_layer_project(a_pos_ecef, a_pos_merc);

Then, our internal boilerplate code would be injected and have the following:

uniform mat4 u_projection;
uniform mat4 u_globeToMercMatrix;
uniform float u_globeToMercatorTransition;
uniform vec2 u_centerInMerc;
uniform float u_pixelsPerMeterRatio;

vec4 project_custom_layer(vec3 pos_ecef, vec3 pos_merc) {
    vec4 projected_position = u_projection * u_globeToMercMatrix * vec4(a_pos_ecef, 1.);
    projected_position /= projected_position.w;
    if (u_globeToMercatorTransition > 0.) {
        vec4 merc = u_projection * vec4(a_pos_merc, 1.);
        vec4 merc = vec4(a_pos_merc, 1.);
        merc.xy = (merc.xy - u_centerInMerc) * u_pixelsPerMeterRatio + u_centerInMerc;
        merc.z *= u_pixelsPerMeterRatio;
        merc = u_projection * merc;
        merc /= merc.w;
        projected_position = mix(projected_position, merc, u_globeToMercatorTransition);
    }
    return projected_position;
}

The above shader code would be injected by default, and uniforms could be setup on our side prior to call onAdd and updated prior to the render function.

By hiding some of that complexity, we would also not have have to carry so many parameters to the render function.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More thoughts on the shader code injection point, the header shader code could be simply added as

this.globeProgram = createProgram(gl, mapboxgl.getGlobeVertexHeader().concat(globeVertCode), fragCode);

And uniforms uploaded with a specific function taking in the program id

mapboxgl.updateCustomGlobeShader(this.globeProgram)

and I think that would cover it?

Then, render interface becomes:

render (gl, projectionMatrix, projection)

instead of:

render (gl, projectionMatrix, projection, globeToMercMatrix, transition, centerInMercator, pixelsPerMeterRatio)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tried those suggestions in #12545.

merc /= merc.w;
p = mix(p, merc, u_globeToMercatorTransition);
}
Expand Down Expand Up @@ -141,21 +148,22 @@ const satellitesLayer = {
}
},

render (gl, projectionMatrix, projection, globeToMercMatrix, transition) {
render (gl, projectionMatrix, projection, globeToMercMatrix, transition, centerInMercator, pixelsPerMeterRatio) {
if (this.satData) {
this.updateBuffers();

const primitiveCount = this.posEcef.length / 3;
gl.enable(gl.DEPTH_TEST);
gl.disable(gl.DEPTH_TEST);
if (projection && projection.name === 'globe') { // globe projection and globe to mercator transition
gl.useProgram(this.globeProgram);

updateVboAndActivateAttrib(gl, this.globeProgram, this.posEcefVbo, this.posEcef, "a_pos_ecef");
updateVboAndActivateAttrib(gl, this.globeProgram, this.posMercVbo, this.posMerc, "a_pos_merc");

gl.uniformMatrix4fv(gl.getUniformLocation(this.globeProgram, "u_projection"), false, projectionMatrix);
gl.uniformMatrix4fv(gl.getUniformLocation(this.globeProgram, "u_globeToMercMatrix"), false, globeToMercMatrix);
gl.uniform1f(gl.getUniformLocation(this.globeProgram, "u_globeToMercatorTransition"), transition);
gl.uniform2f(gl.getUniformLocation(this.globeProgram, "u_centerInMerc"), centerInMercator[0], centerInMercator[1]);
gl.uniform1f(gl.getUniformLocation(this.globeProgram, "u_pixelsPerMeterRatio"), pixelsPerMeterRatio);

gl.drawArrays(gl.POINTS, 0, primitiveCount);
} else { // mercator projection
Expand Down
6 changes: 6 additions & 0 deletions src/geo/transform.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1114,6 +1114,12 @@ class Transform {
// Point at center in world coordinates.
get point(): Point { return this.project(this.center); }

// Point at center in Mercator coordinates.
get pointMerc(): Point { return this.point._div(this.worldSize); }

// Ratio of pixelsPerMeter in the current projection to Mercator's.
get pixelsPerMeterRatio(): number { return this.pixelsPerMeter / mercatorZfromAltitude(1, this.center.lat) / this.worldSize; }

setLocationAtPoint(lnglat: LngLat, point: Point) {
let x, y;
const centerPoint = this.centerPoint;
Expand Down
6 changes: 4 additions & 2 deletions src/render/draw_custom.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ function drawCustom(painter: Painter, sourceCache: SourceCache, layer: CustomSty
context.setColorMode(painter.colorModeForRenderPass());

if (painter.transform.projection.name === "globe") {
prerender.call(implementation, context.gl, painter.transform.customLayerMatrix(), painter.transform.getProjection(), painter.transform.globeToMercatorMatrix(), globeToMercatorTransition(painter.transform.zoom));
const center = painter.transform.pointMerc;
prerender.call(implementation, context.gl, painter.transform.customLayerMatrix(), painter.transform.getProjection(), painter.transform.globeToMercatorMatrix(), globeToMercatorTransition(painter.transform.zoom), [center.x, center.y], painter.transform.pixelsPerMeterRatio);
} else {
prerender.call(implementation, context.gl, painter.transform.customLayerMatrix());
}
Expand Down Expand Up @@ -76,7 +77,8 @@ function drawCustom(painter: Painter, sourceCache: SourceCache, layer: CustomSty
context.setDepthMode(depthMode);

if (painter.transform.projection.name === "globe") {
implementation.render(context.gl, painter.transform.customLayerMatrix(), painter.transform.getProjection(), painter.transform.globeToMercatorMatrix(), globeToMercatorTransition(painter.transform.zoom));
const center = painter.transform.pointMerc;
implementation.render(context.gl, painter.transform.customLayerMatrix(), painter.transform.getProjection(), painter.transform.globeToMercatorMatrix(), globeToMercatorTransition(painter.transform.zoom), [center.x, center.y], painter.transform.pixelsPerMeterRatio);
} else {
implementation.render(context.gl, painter.transform.customLayerMatrix());
}
Expand Down
2 changes: 1 addition & 1 deletion src/style/style_layer/custom_style_layer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import assert from 'assert';
import type {ValidationErrors} from '../validate_style.js';
import type {ProjectionSpecification} from '../../style-spec/types.js';

type CustomRenderMethod = (gl: WebGLRenderingContext, matrix: Array<number>, projection: ?ProjectionSpecification, projectionToMercatorMatrix: ?Array<number>, projectionToMercatorTransition: ?number) => void;
type CustomRenderMethod = (gl: WebGLRenderingContext, matrix: Array<number>, projection: ?ProjectionSpecification, projectionToMercatorMatrix: ?Array<number>, projectionToMercatorTransition: ?number, centerInMercator: ?Array<number>, pixelsPerMeterRatio: ?number) => void;
Copy link
Contributor

@karimnaaji karimnaaji Jan 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding more parameter gives me another reason to point out this suggestion: #12182 (comment) to clean this interface before we publish it publicly.

Or, if we could eliminate most of these parameters per below comment altogether that would be even more ideal to reduce a bit of the complexity for the user; e.g. remove the need to pass along globeToMercMatrix, transition, centerInMercator, and pixelsPerMeterRatio.


/**
* Interface for custom style layers. This is a specification for
Expand Down