mapbox / mapbox-gl-draw

Draw tools for mapbox-gl-js
https://www.mapbox.com/mapbox-gl-js/example/mapbox-gl-draw/
ISC License
947 stars 592 forks source link

crash after adding a canvas to map (no error thrown) #639

Open juliangums opened 7 years ago

juliangums commented 7 years ago

mapbox-gl-js version: 0.36.0 mapbox-gl-draw version: 0.17.0

Steps to Trigger Behavior

Add a canvas source like:

map.addSource('heatmapsource', {
    type: 'canvas',
    canvas: 'heatmap-canvas',
    animate: true,
    coordinates: [
      [west, north],
      [east, north],
      [east, south],
      [west, south]
    ]
  });

It does not matter if you add it before or after initialising Mapbox Draw

Expected Behavior

It should work.

Actual Behavior

It doesn't. All features disappear, when clicking a draw control, the cursor does not even become a crosshair, total f up. No error thrown :-(

mcwhittemore commented 7 years ago

Julian,

Please provide a fiddle of bugs per the new issue instructions. It greatly helps reduce the number of clarifying questions people have to ask each other. Thanks.

It does not matter if you add it before or after initialising Mapbox Draw

Does the error happen even if you never initialize Draw?

Regards, Matthew

juliangums commented 7 years ago

Does the error happen even if you never initialize Draw?

Not sure what you mean. If I don't initialise it it won't crash obviously, as it's not loaded. Everything else (I am using it on a very complex application) still continues working.

Here a simplified fiddle: https://jsfiddle.net/5mascc3w/6/

mcwhittemore commented 7 years ago

Julian,

Thanks! The fiddle helped a bunch. It looks like there is some sort of problem with canvas source types and Mapbox Draw. I'm going to need to comb though the mapbox-gl-js issues to see if this is a bug upstream which is my gut reaction here.

mcwhittemore commented 7 years ago

There is a bug in mapbox-gl-js which results in canvas sources breaking the map. As this issue isn't caused by Draw, I am closing here and sharing your fiddle with that team.

For documentation reasons, here is my fork of your fiddle that shows removing the canvas source fixes the problem (though still makes your application unusable).

juliangums commented 7 years ago

That's a different bug! They guy says the map goes white on large canvases. I have a tiny canvas and the map is not going white. As you see the map is NOT breaking. As I said I am using it in a very complex application and NOTHING BREAKS. Only mapbox draw seems to have a problem with it. Therefore it is most likely be draw related. But it is definitely an entirely different bug then the one you have found there...

On 29 Apr 2017, at 05:16, Matthew Chase Whittemore notifications@github.com wrote:

There is a bug in mapbox-gl-js which results in canvas sources breaking the map. As this issue isn't caused by Draw, I am closing here and sharing your fiddle with that team.

For documentation reasons, here is my fork of your fiddle that shows removing the canvas source fixes the problem (though still makes your application unusable).

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

mcwhittemore commented 7 years ago

Can you provide a gif of the problem? I'm seeing the this bug when running your fiddle. Also, I'm not sure if your comments are trying to be aggressive or just coming across that way, but I'm just trying to help you out.

juliangums commented 7 years ago

by no means trying to be aggressive or offensive. I appreciate your help! you're awesome! you're creating brilliant software! English isn't my first language and I was thinking maybe it was me not expressing the issue properly so I used caps to make it more vivid. not wanting to make it look shouty or aggressive. if I came across as rude, it is just me wanting to be more clear about what i mean. sry for that.

the point I was trying to make is that the difference between my issue and https://github.com/mapbox/mapbox-gl-js/issues/4262 is: 1.) 4262 is canvas size dependent. mine isn't 2.) 4262 breaks the entire map. makes it go white. mine works fine with all other map layers (using 24 layers). only mapbox draw seems to have a problem 3.) 4262 is browser dependent. i have tested mine with chrome, firefox and safari on linux, ios, macos. so it seems to be irrelevant. 4.) in 4262 they have not added mapbox draw. I have no problems with the canvas if i don't add draw. It's not interfering with anything else.

I think I also said "all features disappear". That was a little misleading. It sounds like it effects all features on all layers. It does not though. So basically I am adding polygons programmatically and they disappear when a canvas source is added. So basically what I meant was everything draw related crashes. only the controls buttons are still there.

So when you do the fiddle you don't see the map going white or do you? Here are three images how it looks on my screen. On the first one you can see mapbox draw works, as I've disabled the canvas. screenshot from 2017-04-29 20-59-01

the second one with that has the canvas works just fine as well. no white background or so. screenshot from 2017-04-29 21-49-13

the third one with both activated works fine for the map with all it's layers but mapbox draw fails screenshot from 2017-04-29 20-59-30

thanks for your help!

mcwhittemore commented 7 years ago

if I came across as rude, it is just me wanting to be more clear about what i mean. sry for that.

Cool! Thanks for clarifying! :)

So when you do the fiddle you don't see the map going white or do you?

Yea, the map goes white like what is talked about in mapbox/mapbox-gl-js#4262. Here is a screenshot of what I'm seeing.

image

With that said, I'm able to get the fiddle to work with Safari. 🎉.

After poking around a bit, it looks like Draw isn't running into any problems but rather just stopping after setting up the draw_polygon mode but before starting it. This explains why we aren't seeing an error or anything in the console. My guess is that either gl-js isn't emitting an event or that the event it is emitting look different than Draw expects. I'll need to test this out locally to confirm.

juliangums commented 7 years ago

that explains a lot! :) fingers crossed it's an easy fix!

juliangums commented 7 years ago

dude, it actually works when initialising draw first! The reason why it didn't work for me was because I was inserting my canvas layer before a layer that is added before draw. So it seems to have something to do with the layer order. if draw is behind the canvas it works, if draw is in front of it it fails. however, even if I add a canvas source before draw, it fails as well. hope it helps

mcwhittemore commented 7 years ago

Thanks for writing back @juliangums.

juliangums commented 7 years ago

the issue still exists though. and the bypass only works until i want to hide mapbox draw.

map.removeControl(draw); map.addControl(draw);

breaks it. as now I am adding the thing after the problem causing canvas

mcwhittemore commented 7 years ago

I'm still pretty confused about what the bug is here. Are you still seeing this @juliangums, do you have more insight into the problem?

juliangums commented 7 years ago

still get the issue. updated mapbox gl and draw in the fiddle https://jsfiddle.net/5mascc3w/ I am unable to draw anything across all browsers. does it work for you? however if you initialise draw first and then add the canvas is works unless you add it to a layer that is under the draw layers. so people that want canvas layers underneath draw or that need to initialise the canvas before draw for whatever reason won't be able to do it. I also found out that even if i add the layer after draw but have added the source before draw it won't work. can't really say more to this as there's no error logged or anything and I'm too dumb to find the issue in your code. sorry i can't be of more help

Nikcharn commented 3 years ago

In my project I fixed this problem like that:

  1. I added function to toggle all CanvasSource playing

    
    function toggleCanvasSourcesPlaying(map: Map, enabled: boolean) {
        const sources = map.getStyle()?.sources || {};
    
        Object.keys(sources)
            .forEach(id => {
                const source: any = map.getSource(id);
                if (source && source.animate) {
                    enabled ? source.play() : source.pause();
                }
            })
    }
2. Before adding "MapboxDraw", I simply turn off all animation. And after that I turn on

... MapStyleUtil.toggleCanvasSourcesPlaying(this.map, false); this.timeout = setTimeout(() => { this.map.addControl(this.draw); MapStyleUtil.toggleCanvasSourcesPlaying(this.map, true); this.draw.changeMode(MapboxDrawModes.drawPolygon); }) ...


Of course this is not the best solution.
CaviarChen commented 1 year ago

Encountered a similar issue and I think it is related. After adding a canvas source with animate set to true (and a rester layter referencing this canvas source), this.map.addControl(mapboxDraw, "bottom-left"); stops working at all. No error thrown and if I go through all layers on the mapbox, there is no mapbox-gl-draw related layer.