Canvas type #3765

lbud · 2016-12-08T01:30:08Z

Closes #3580.

This adds a canvas source type, which can be used in raster layers like existing image and video sources.

To be resolved:

We have no solid plan for parity with GL Native for this feature. It's also not something that will be very useful in api-styles, because we're loading same-page-based canvases, unlike other sources which often live in the ☁️. These things are weird and we should make sure we're comfortable with this approach.
add tests
add example

Related feature but not a blocker: raster querying #1404 (came up in #3745)

Launch Checklist

briefly describe the changes in this PR
write tests for all new functionality
document any changes to public APIs (generated)

lucaswoj · 2016-12-09T21:36:06Z

Do we want to support webgl contexts (less straightforward, not necessarily the use case we're targeting, and likely to be obseleted by custom layer types)?

Based on what I've heard about perf, we don't need to go out of our way to support WebGL canvases. If we get some support for free from texImage2D, that's a nice bonus.

jliebrand · 2016-12-16T18:37:48Z

Curious: will this feature allow me to draw to a 2d canvas, and have it show up on the map (correctly zooming/panning/etc)?

Eg will this allow me animate something on the chart, unrelated to geojson and webgl?

lucaswoj · 2016-12-16T18:59:15Z

Curious: will this feature allow me to draw to a 2d canvas, and have it show up on the map (correctly zooming/panning/etc)?

Yes!

Eg will this allow me animate something on the chart, unrelated to geojson and webgl?

Yup 😄

jliebrand · 2016-12-17T18:34:16Z

Awesome! Can't wait!

…

On Fri, Dec 16, 2016 at 6:59 PM Lucas Wojciechowski < ***@***.***> wrote: Curious: will this feature allow me to draw to a 2d canvas, and have it show up on the map (correctly zooming/panning/etc)? Yes! Eg will this allow me animate something on the chart, unrelated to geojson and webgl? Yup 😄 — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#3765 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AWUtDOVDYVlOagajPyhKwA262W_oJd-Aks5rIt-HgaJpZM4LHVng> .

lbud · 2016-12-22T18:15:32Z

package.json

@@ -45,6 +45,7 @@
    "babel-eslint": "^7.0.0",
    "benchmark": "~2.1.0",
    "browserify": "^13.0.0",
+    "canvas": "^1.6.2",


NOTE: this is a relatively nontrivial dep for something just used in one test…discuss

Can you create a fake Canvas API using Sinon?

lbud · 2017-01-05T22:47:10Z

As currently implemented, this can support reading from either 2D or webgl canvas contexts. 2D is ideal and more performant and, in fact, gl.texImage2D / gl.texSubImage2D can take an HTML5 canvas element as an argument, meaning we don't even have to manually copy pixels. (When using a webgl context however this causes very incorrect rendering/artifacts.) Do we want to support webgl contexts (less straightforward, not necessarily the use case we're targeting, and likely to be obseleted by custom layer types)?

Alright, I simplified this a lot by using texImage2D/texSubImage2D no matter the context/not even doing context detection. This also allows us to take out the dimensions property since we don't manually readPixels. It performs fine on my simple canvas debug pages but I haven't made anything super flashy to stress test it with yet… We could prohibit webgl contexts, or we could just caveat emptor it. 🤔

Once I pull together a good example and tighten up the docs this should be good to 👀 & 🚢

We have no solid plan for parity with GL Native for this feature. It's also not something that will be very useful in api-styles, because we're loading same-page-based canvases, unlike other sources which often live in the ☁️. These things are weird and we should make sure we're comfortable with this approach.

🤔

… events

…ce, set evented parent before possible racing for ImageSprite

…Image2D if we resize the canvas

…if 2D context (rather than webgl) is present

(Enables removal of dimensions and secondary _canvas)

lbud · 2017-01-19T02:02:05Z

Rebased; I think this is good to go. I actually think this is fine to merge without an example, at least for now. I do have an example I've been playing around with, but it depends on loading 60-some images…this can be a follow-up task 🤔🌩

Secondly, tests were failing by hanging/timing out at the ignored line-width property function test because the spec canvas_type branch is rebased off of current mb-pages, which says line-width property functions are not supported (they were rolled back). The suite implementation still renders even ignored tests, and just doesn't count them against the fail count. But in this case, it errors and hangs on validating the line-width property function test. It's not the cleanest, but I added a commit to this PR that creates a style separately of the map for each render test and then adds it to the map, so that we can pass in a validate boolean based on whether a test is ignored. 👍/:-1:? (This test has been hanging locally for me for weeks 😕)

lucaswoj · 2017-01-19T19:11:21Z

js/source/canvas_source.js

+
+        if (!this.tile) return; // not enough data for current position
+        this._prepareImage(this.map.painter.gl, this.canvas, this.resize);
+        this.resize = false;


Maybe convert this to a local var?

lucaswoj · 2017-01-19T19:12:39Z

js/source/canvas_source.js

+        }
+        if (this._hasInvalidDimensions()) {
+            return this.fire('error', new Error('Canvas dimensions cannot be less than or equal to zero.'));
+        }


Maybe make this a no-op instead of a failure

lucaswoj · 2017-01-19T19:18:11Z

test/suite_implementation.js

        classes: options.classes,
        interactive: false,
        attributionControl: false,
        preserveDrawingBuffer: true
    });

+    const _style = new Style(style, map, { validate: !options.ignored });
+    map.setStyle(_style);
+


We can use skipped flags to get around validating these tests without adding new code

* make resize a local var * fail silently on invalid canvas dimensions * use 'skipped' rather than changing suite_implementation for line-width property fn test

lucaswoj

There's a regression test that also uses a data-driven line width. If you skip that one, everything should be 🍏 and you're clear to !

lbud · 2017-01-20T18:55:20Z

There's a regression test that also uses a data-driven line width. If you skip that one, everything should be 🍏 and you're clear to !

aha 🕵️‍♀️ I saw line-width was still failing and didn't get a chance to look into it yet. Alright, will do!

averas · 2017-01-26T22:41:11Z

This is awesome! One use case I feel I haven't found an optimal solution for has been to effectively show small thumbnail images in Mapbox GL JS, to create maps like this:

While it is possible to come up with something quite close to that with the marker plugin I found it hard to make a seamless experience that switches between symbols (dots/icons) and thumbnails based on zoom, and the markers are not data-driven in a similar manner as with other sources which makes it tedious to add and remove them based on map interaction.

Could the canvas source type be used to create a data driven thumbnail map like the one above?

lucaswoj · 2017-01-27T01:56:44Z

@averas

It would be possible to create a canvas source which renders and places all of the thumbnails. I anticipate the difficulty of implementing this (you'll need to work with the canvas API and explicitly do some projections) may outweigh the benefits.

Have you tried using implementing this using the image source?

averas · 2017-01-27T02:04:58Z

@lucaswoj

Have you tried using implementing this using the image source?

No..? The image source can only reference a single image, and once created can't be mutated (one has to replace the source to update the image). I imagine it would be quite ineffective to push/pop potentially hundreds of sources/layers when panning/zooming a map, but I may be wrong?

I think the best Mapbox GL JS examples of this are:
https://www.mapbox.com/blog/custom-markers-mapboxgl/
https://www.mapbox.com/mapbox-gl-js/example/custom-marker-icons/

.. but again, both of these use markers and are therefore 1) not data driven by sources (i.e. you have to micro manage the marker behaviour with discrete code hooked into map interaction and other events), and 2) inherently not benefited by WebGL as they are based on regular HMTL in the DOM.

What I typically do in a case like this is falling back to a data driven, projected SVG overlay using d3.js. That often solves some of the hurdles, such as projection and a declarative data-driven approach, however, it obviously lacks the ability to work directly with Mapbox sources, such as vector tiles, and you are still not benefiting from WebGL.

yangdonglai · 2017-06-01T06:47:52Z

marked and keep attention

bennlich · 2022-01-27T05:36:27Z

(you'll need to work with the canvas API and explicitly do some projections)

@lucaswoj I'm working with a canvas source right now, wondering this exact thing. I how do I project from lat/lng to canvas coordinates?

lucaswoj added the under development label Dec 13, 2016

lbud force-pushed the canvas_type branch from 8df52bd to e389c74 Compare December 22, 2016 18:13

lbud commented Dec 22, 2016

View reviewed changes

lbud force-pushed the canvas_type branch from d0717f8 to f493f12 Compare January 5, 2017 01:04

Lauren Budorick added 19 commits January 18, 2017 16:44

Only start source loading/event firing onAdd so as not to have racing…

f845fad

… events

Fix hang caused by missing eventedParent argument in raster_tile_sour…

bcbc0dd

…ce, set evented parent before possible racing for ImageSprite

Minimize diff, use onAdd pattern for VT source test

ab464e3

Load source per test

ee6e508

Revert a few more unnecessary changes contributing to diff size

f66b053

(wip) canvas source type

3e36acc

Enable reading (pixels) for either 2d canvas or webgl contexts

ec13892

Update spec SHA

ef15c7d

Add CanvasSource#setDimensions, re-call texImage2D rather than texSub…

e035807

…Image2D if we resize the canvas

Update spec SHA + delete extra shaders SHA

7cdcccb

Rebase on 3735, read from canvas directly rather than copying pixels …

e825a33

…if 2D context (rather than webgl) is present

Spec SHA

bd1d102

Rebase + add some basic tests

655a2a8

Stub canvas.getContext rather than using npm package

04281be

lint

dc0302a

Update test-suite SHA, fix for window.ImageData check in tests

8187f48

Simplify by using texImage2D/texSubImage2D for any context

26d8705

(Enables removal of dimensions and secondary _canvas)

Canvas documentation tweaks

02e4c61

Update package.json

8ff420f

lbud force-pushed the canvas_type branch from 773993f to 8ff420f Compare January 19, 2017 00:45

Update spec SHA

52c0af1

lbud mentioned this pull request Jan 19, 2017

Canvas type mapbox/mapbox-gl-style-spec#656

Merged

Lauren Budorick added 2 commits January 18, 2017 17:50

Amend suite_implementation to not validate styles that are ignored

51bfa6a

lint

5838837

lucaswoj removed the under development label Jan 19, 2017

lucaswoj changed the title ~~[not ready] Canvas type~~ Canvas type Jan 19, 2017

lucaswoj reviewed Jan 19, 2017

View reviewed changes

Address review comments:

cddc587

* make resize a local var * fail silently on invalid canvas dimensions * use 'skipped' rather than changing suite_implementation for line-width property fn test

lucaswoj approved these changes Jan 20, 2017

View reviewed changes

Skip line-width property fn regression test

5a38282

lbud merged commit 5082fec into master Jan 20, 2017

lbud deleted the canvas_type branch January 20, 2017 19:57

lbud restored the canvas_type branch August 16, 2017 18:17

jfirebaugh deleted the canvas_type branch September 21, 2017 17:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Canvas type #3765

Canvas type #3765

lbud commented Dec 8, 2016 •

edited

Loading

lucaswoj commented Dec 9, 2016

jliebrand commented Dec 16, 2016

lucaswoj commented Dec 16, 2016

jliebrand commented Dec 17, 2016 via email

lbud Dec 22, 2016

lucaswoj Dec 23, 2016

lbud commented Jan 5, 2017

lbud commented Jan 19, 2017

lucaswoj Jan 19, 2017

lucaswoj Jan 19, 2017

lucaswoj Jan 19, 2017

lucaswoj left a comment

lbud commented Jan 20, 2017

averas commented Jan 26, 2017

lucaswoj commented Jan 27, 2017

averas commented Jan 27, 2017 •

edited

Loading

yangdonglai commented Jun 1, 2017

bennlich commented Jan 27, 2022

Canvas type #3765

Canvas type #3765

Conversation

lbud commented Dec 8, 2016 • edited Loading

Launch Checklist

lucaswoj commented Dec 9, 2016

jliebrand commented Dec 16, 2016

lucaswoj commented Dec 16, 2016

jliebrand commented Dec 17, 2016 via email

lbud Dec 22, 2016

Choose a reason for hiding this comment

lucaswoj Dec 23, 2016

Choose a reason for hiding this comment

lbud commented Jan 5, 2017

lbud commented Jan 19, 2017

lucaswoj Jan 19, 2017

Choose a reason for hiding this comment

lucaswoj Jan 19, 2017

Choose a reason for hiding this comment

lucaswoj Jan 19, 2017

Choose a reason for hiding this comment

lucaswoj left a comment

Choose a reason for hiding this comment

lbud commented Jan 20, 2017

averas commented Jan 26, 2017

lucaswoj commented Jan 27, 2017

averas commented Jan 27, 2017 • edited Loading

yangdonglai commented Jun 1, 2017

bennlich commented Jan 27, 2022

lbud commented Dec 8, 2016 •

edited

Loading

averas commented Jan 27, 2017 •

edited

Loading