Add perspective option to custom markers #3062

yeldarby · 2016-08-24T23:00:04Z

This pull request adds the option to make markers scale with the map perspective. This comes into play when zooming or changing the bearing on a pitched map.

This also fixes z-indexing of markers (previously they were z-indexed haphazardly; this changes them to be z-indexed with the closest marker on top using translate3d).

Example usage:

var marker = new mapboxgl.Marker(null, {
        perspective: true
    }) .setLngLat(toSet).addTo(map);

This commit adds an option to custom markers to enable perspective. If `perspective` is set to true they will resize as they go into the background when changing the bearing on a pitched map and also resize when you zoom in or out. This commit also changes the positioning to use `translate3d` in order to fix wonky z-Indexing of markers in previous releases.

yeldarby · 2016-08-25T00:30:51Z

In thinking about this on my drive home I no longer think setting the default zoom of the marker to be the zoom of the map at the time it is added is a good idea.

Tomorrow I plan to update the default zoom level of he marker to be user configurable instead.

Reasoning for this being: imagine you add a marker, the user zooms out then you add another marker. One will be big one will be small currently which is probably not desired in most instances. Current functionality would still be easy for the developer to opt into using map.getZoom() as the value for native zoom.

1ec5 · 2016-08-25T00:36:52Z

js/ui/marker.js

@@ -13,6 +13,7 @@ var Point = require('point-geometry');
 * @param {HTMLElement=} element DOM element to use as a marker (creates a div element by default)
 * @param {Object=} options
 * @param {PointLike=} options.offset The offset in pixels as a [`PointLike`](#PointLike) object to apply relative to the element's top left corner. Negatives indicate left and up.
+ * @param {boolean=} [options.perspective=false] Whether the marker should scale along with the map geometry.


Perspective can mean many things. For example, in some cases it might be desirable to not only scale a marker but also tilt it to lie flat against the tilted map. (More ideas at mapbox/mapbox-gl-native#5245.)

In the meantime, merely scaling the marker would still be desirable functionality; how about a more descriptive term, like scalesWithViewingDistance?

Or perhaps pitchScale: "map", for consistency with the circle-pitch-scale: map style property?

yeldarby · 2016-08-25T01:39:58Z

That's a good point. I will read through that other thread and update the nomenclature tomorrow to try to maintain consistency.

yeldarby · 2016-08-25T15:22:27Z

Ok, I've gone ahead and split this out into the two different options.

pitchScale for scaling the marker as the map is pitched
and zoomScale for scaling the marker as the map is zoomed

zoomScale also has some other configurable options (how much to zoom the marker and what the native zoom level for the marker is).

It's likely that in the future we may want to add a maxZoom and minZoom value and possibly an option to pass a zoom function to determine scale by zoom as well in case you want custom values.

lucaswoj · 2016-08-29T20:58:29Z

dist/mapbox-gl.css

+.mapboxgl-canvas-container {
+    -webkit-transform-style: preserve-3d;
+    -moz-transform-style: preserve-3d;
+    transform-style: preserve-3d;


Unfortunately preserve-3d is not supported in one of our supported platforms: IE11

This should fall back to the behavior currently in production in IE11. Using preserve-3d lets us set the z-indexing of markers with a single CSS transform.

The workaround for IE would probably be to manually set the z-Index of the elements instead of using a transform on the z axis (which would likely be slower but should get the job done).

What would be the best way to detect when the client is IE11? Usually I'd use Modernizr but it doesn't look like you guys are pulling that in currently.

The testing and debugging overhead of a separate codepath for IE11 would likely be a blocker for us. Have you measured the performance of manually setting the z-index in all browsers?

Looks like the performance impact is actually relatively minor. I did some tests using debug/markers3d.html using 50 markers to check the framerate with different z-Indexing strategies.

No markers (baseline):

Current version in production (ignores zIndexing and scaling completely):

With translate3d

With zIndex property

I changed the functionality to set the zIndex by default so it will work as expected in IE11 with an option to disable if the speed bump is necessary.

I'm glad to hear that performance of the z-index option is good! 🎉

We do not feel comfortable including both the z-index and transform-style options. Having two options will roughly double the amount of testing, support, and debugging required by Mapbox & our contributors. Let's drop the option and stick with just z-index for now.

lucaswoj · 2016-08-30T18:31:22Z

js/geo/transform.js

-        vec4.transformMat4(p, p, this.pixelMatrix);
-        return p;
+        if(shouldReturnRawVector) return p;
+        return new Point(p[0] / p[3], p[1] / p[3]);


👎 on changing the return type based on an argument. Two seperate methods are preferable.

It looks like this code could be written most succinctly if coordiantePoint called coordinatePoint3d

coordinatePoint: function(coord) { var p = this.coordinatePoint3d(coord); return new Point(p[0] / p[3], p[1] / p[3]); }

Thanks; I've refactored it back so that coordinatePoint calls coordinatePoint3d

lucaswoj · 2016-09-01T21:23:55Z

debug/markers3d.html

+</script>
+
+</body>
+</html>


It makes sense that you built a debug page to use while developing this feature.

Because repository size, simplicity, and stability is important to us, we would prefer not to merge this file into master. Can you please remove this file from the PR?

lucaswoj · 2016-09-01T21:25:43Z

@mourner This is close-to-ready from an architectural standpoint. I am interested to hear your thoughts on the concept and implementation.

lucaswoj · 2016-09-02T20:33:17Z

docs/_posts/examples/3400-01-24-custom-marker-with-perspective.html

+    m.style.backgroundColor = 'rgb(' + [Math.floor(Math.random() * 16) * 16, Math.floor(Math.random() * 16) * 16, Math.floor(Math.random() * 16) * 16].join(',') + ')';
+}
+
+for (var i = 0; i < points; i++) addAnotherMarker();


I appreciate the aesthetics of this demo and its utility for debugging this PR. Our intent with the GL JS examples is to provide the simplest possible code for demonstrating a feature, making it easy for users to see how they can adapt the feature to their use case. In this case, the simplest possible code would probably look like instantiating a single pitchScale-ed andzoomScale-ed marker on a tilted map.

In that case, how would you feel about updating the existing marker example instead to pass these options?

There are situations in which our ability to explain concepts to users would be aided by having both a bare bones marker example and a perspective-scaled marker example. Let's keep this as two examples for now.

lucaswoj · 2016-09-02T20:45:18Z

This is great! We're so close to being able to 🚢. Let me know if you have any questions or need any assistance 🙇

lucaswoj · 2016-11-22T19:20:49Z

I am closing this PR because it has been inactive for a long time. The branch isn't going anywhere so please keep working on this feature and re-open the PR when it is ready!

Let me know if you have any questions.

yeldarby added 2 commits August 24, 2016 17:52

Fix failing tests

eb099ea

1ec5 reviewed Aug 25, 2016
View reviewed changes

Split out functionality into multiple configurable options

9fbbfd6

lucaswoj reviewed Aug 29, 2016
View reviewed changes

yeldarby added 3 commits August 30, 2016 09:08

Refactor

e5e47b9

Refactor map.js to match

be8d8f4

Fix linter error

8e8c228

lucaswoj reviewed Aug 30, 2016
View reviewed changes

Refactor, Add IE11 support

46cc45b

yeldarby mentioned this pull request Aug 31, 2016

markers appear in front of popup #3109

Closed

lucaswoj reviewed Sep 1, 2016
View reviewed changes

Remove preserve-3d due to no IE11 support

bfbbb6e

lucaswoj reviewed Sep 2, 2016
View reviewed changes

lucaswoj added the under construction label Sep 6, 2016

yeldarby mentioned this pull request Sep 15, 2016

Adds support for overflowing stops mapbox/mapbox-gl-function#23

Closed

4 tasks

lucaswoj closed this Nov 22, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add perspective option to custom markers #3062

Add perspective option to custom markers #3062

yeldarby commented Aug 24, 2016 •

edited

Loading

yeldarby commented Aug 25, 2016

1ec5 Aug 25, 2016

1ec5 Aug 25, 2016

yeldarby commented Aug 25, 2016

yeldarby commented Aug 25, 2016

lucaswoj Aug 29, 2016

yeldarby Aug 29, 2016

lucaswoj Aug 30, 2016

yeldarby Aug 30, 2016

yeldarby Aug 31, 2016

lucaswoj Sep 1, 2016

lucaswoj Aug 30, 2016 •

edited

Loading

yeldarby Aug 31, 2016

lucaswoj Sep 1, 2016 •

edited

Loading

lucaswoj commented Sep 1, 2016

lucaswoj Sep 2, 2016

yeldarby Sep 2, 2016

lucaswoj Sep 2, 2016

lucaswoj commented Sep 2, 2016

lucaswoj commented Nov 22, 2016

Add perspective option to custom markers #3062

Add perspective option to custom markers #3062

Conversation

yeldarby commented Aug 24, 2016 • edited Loading

yeldarby commented Aug 25, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yeldarby commented Aug 25, 2016

yeldarby commented Aug 25, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lucaswoj Aug 30, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lucaswoj Sep 1, 2016 • edited Loading

Choose a reason for hiding this comment

lucaswoj commented Sep 1, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lucaswoj commented Sep 2, 2016

lucaswoj commented Nov 22, 2016

yeldarby commented Aug 24, 2016 •

edited

Loading

lucaswoj Aug 30, 2016 •

edited

Loading

lucaswoj Sep 1, 2016 •

edited

Loading