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

Map tiles not loading until a zoom is executed #3002

Closed
RobbieTheWagner opened this issue Nov 6, 2014 · 61 comments
Closed

Map tiles not loading until a zoom is executed #3002

RobbieTheWagner opened this issue Nov 6, 2014 · 61 comments

Comments

@RobbieTheWagner
Copy link

After updating to Leaflet 0.8-dev, my tiles are not loading smoothly. The initial map is missing one tile it appears and when I change to other tile layers with the layer control, I end up with a completely blank map. No errors in the console. If I zoom though, the tiles refresh and all of them load. See attached screenshots.
screenshot at nov 06 14-40-53
screenshot at nov 06 14-41-18

@snkashis
Copy link
Member

snkashis commented Nov 6, 2014

Can you link to a JSFiddle or example page?

@RobbieTheWagner
Copy link
Author

I am not able to reproduce the error at this time. I am working on a plunker, but it seems to load fine, so not sure what I am missing. I cannot share a link to my actual code, I don't think. I was hoping someone would have seen a similar issue and know how to solve it. I'll continue to work on reproducing it, but we have a ton of code I'll need to reproduce.

@jfirebaugh
Copy link
Member

Ok, feel free to reopen if you have a reproducible test case.

@RobbieTheWagner
Copy link
Author

I took another look at the tiles being sent back, and it appears there is not a request sent for the one I am not seeing. Then, if I zoom out and back in, it requests all the tiles correctly. I know there was a major TileLayer refactor, so any chance someone knows why a request or two would be missing initially, and then work after zooming?

@mourner
Copy link
Member

mourner commented Nov 7, 2014

No idea. We need a test case to figure out.

@RobbieTheWagner
Copy link
Author

Is there a way to just manually reload the tiles or something? I cannot produce a test case, we have way too much going on in our code.

@RobbieTheWagner
Copy link
Author

I actually was able to make a test case. Here it is: http://plnkr.co/edit/ny5hAUNVvnA5uNGPMRyg?p=preview

Notice when it loads, only half of the map loads, then if you zoom in and out a couple times, the rest will load.

Additionally, I see in the changelog: removed the need for TileLayer hacks when using custom projections.

What does that refer to? We are extending 4326 to get the scale and transformation right. Is that no longer necessary?

@mourner mourner reopened this Nov 7, 2014
@RobbieTheWagner
Copy link
Author

Has anyone looked at the test case I provided? I would really appreciate some help!

@mourner
Copy link
Member

mourner commented Nov 12, 2014

@perliedman some weird projections stuff

@RobbieTheWagner
Copy link
Author

Just this and another issue or two and we should be up and running on Leaflet 0.8-dev. I realize we may have a bunch of uncommon requirements, but I really appreciate everything all of you have done to help out! It would be great if we could nail down the source of these remaining issues in the next few days. Please let me know what I can do to help.

@RobbieTheWagner
Copy link
Author

Anyone have any idea on why these tiles aren't loading? Is @perliedman the guy in charge of projections related things?

@perliedman
Copy link
Member

Hi, I've looked at the test case and been able to reduce the breaking code to this:

var mapCrs = L.extend({}, L.CRS.EPSG4326, {
        scale: function(zoom) {
            return 512 * Math.pow(2, zoom);
        },
        wrapLat: false,
        wrapLng: false
    }),
    map = new L.map('map', {
        center: [0, 0],
        zoom: 0,
        crs: mapCrs
    });

L.tileLayer('http://services.arcgisonline.com/arcgis/rest/services/ESRI_Imagery_World_2D/MapServer/tile/{z}/{y}/{x}', {
    attribution: false,
    continuousWorld: true,
    tileSize: 512
}).addTo(map);

map.setView([0, 0], 0, {
    reset: true
});

Removing reset: true from the call to setView fixes the problem and shows the map correctly. I haven't looked deeper yet, but since it's a pretty simple example, it looks similar to the lockups we've had with setView before.

I'm obviously too stupid to figure out how to use plunkr, so I gave up with making a fork of the original code, I guess you can just copy the code above into the original plunkr.

@rwwagner90 I guess you already know that changing CRS after the map is initialized, like your example does, isn't currently officially supported... just beware that it might result in some tricky issues... although this problem does not appear to be directly related to it.

@perliedman
Copy link
Member

@mourner I'm not familiar enough with how GridLayer and TileLayer works to be certain if this is a problem, but it looks to me like the following happens:

  • resetting the view causes TileLayer._abortLoading to be called, and setting the tile srcs to emptyImageUrl
  • when _addTiles is later called, the tiles are still in the _tiles cache, and are not added to the load queue

Shouldn't the tiles be removed from the _tiles cache when they're aborted?

When reset: true isn't used, the tile loading is never aborted, which I guess solves the problem.

@mourner
Copy link
Member

mourner commented Nov 13, 2014

@perliedman ha, nice investigation! Yep, it's probably a race condition where _tiles cache isn't cleared up in time — it's supposed to be cleared up later but _addTiles kicks in before that.

@mourner mourner added the bug label Nov 13, 2014
@mourner mourner added this to the 1.0-beta1 milestone Nov 13, 2014
@RobbieTheWagner
Copy link
Author

@perliedman so you are just destroying the entire map and creating a new one to work around this? It would be better for us to not do that. We'll have people placing pins and things on our maps, and we don't want to destroy all of that, just have to allow switching tile layers and they could be any CRS.

Is this race condition something easily fixable?

@mourner
Copy link
Member

mourner commented Nov 13, 2014

Yes, probably easily fixable. Will look at it tomorrow.

@perliedman
Copy link
Member

@rwwagner90 actually not sure what I'd recommend, just wanted to warn you that Leaflet hasn't been designed with this in mind AFAIK, and you might run into weirdness. On the other hand, as you've shown, it looks like to can be made to work with some fiddling.

If Leaflet were to support this in the API, I think there should be a setCrs method or similar, to hide these technicalities, and also make it possible for plugins to react to CRS changes by firing an event.

Also, I think we would need someone who actually requires this feature to step up and implement it 😉

Further discussion regarding this should be posted in #2553.

@RobbieTheWagner
Copy link
Author

@mourner If you could fix the race condition today, that would be awesome! No worries if you are busy, but I'd love to use leaflet 0.8-dev with all the fancy new stuff for our demo early Monday morning.

@perliedman Once I get all my features working, I might have time to implement a setCrs method. I do think it would be a good idea to support.

@mourner
Copy link
Member

mourner commented Nov 14, 2014

@rwwagner90 I'll see what I can do about the race condition.

Changing CRS dynamically is not planned for Leaflet any time soon. If it means lots of complex changes and additional code to the Leaflet core, I'll probably wont accept a pull request either.

@RobbieTheWagner
Copy link
Author

@mourner Thank you very much!

As for CRS, it should basically just be pretty much the method I used. Perhaps we could just tweak things slightly and offer that workaround as how to do it. That way, it would be supported, but we wouldn't have to make any major changes to the Leaflet codebase.

@RobbieTheWagner
Copy link
Author

@mourner any luck with the race condition? I would really appreciate it being fixed!

@RobbieTheWagner
Copy link
Author

@mourner @perliedman If someone could point me to where the race condition is happening, I could take a look. This is one of only two remaining problems I have switching to Leaflet 0.8-dev.

@mourner
Copy link
Member

mourner commented Nov 18, 2014

@rwwagner90 pretty much described in #3002 (comment), you can search by method names in the source code.

@RobbieTheWagner
Copy link
Author

@mourner I just added:

this._tiles = {};

So _addTiles now just clears out the tiles before adding more. This seems to work, but I assume defeats the purpose of having the tiles cache?

EDIT:
Actually, adding it to the end of _abortLoading might make more sense, but still probably isn't what we want.

_abortLoading: function () {
        var i, tile;
        for (i in this._tiles) {
            tile = this._tiles[i];

            tile.onload = L.Util.falseFn;
            tile.onerror = L.Util.falseFn;

            if (!tile.complete) {
                tile.src = L.Util.emptyImageUrl;
                L.DomUtil.remove(tile);
            }
        }
        this._tiles = {};
    }

@mourner
Copy link
Member

mourner commented Nov 18, 2014

Yes, this is wrong. Instead, you need to clear tiles that were aborted in _abortLoading.

@RobbieTheWagner
Copy link
Author

@mourner Okay, I am attempting to just remove ones that were not complete now. This is what I did:

_abortLoading: function () {
        var i, tile;
        for (i in this._tiles) {
            tile = this._tiles[i];

            tile.onload = L.Util.falseFn;
            tile.onerror = L.Util.falseFn;

            if (!tile.complete) {
                tile.src = L.Util.emptyImageUrl;
                L.DomUtil.remove(tile);
                delete this._tiles[i];
            }

        }
    }

Is this the correct solution?

@bmulcahy
Copy link

I think it is that @rwwagner90 .

I added this to my code after my page load
app.map.refresh(0);

app.map.refresh = function(timeout){
      window.setTimeout(function(){
      app.map.invalidateSize();
},timeout);
    };

I added the timeout because I have a collapsible panel in my app that does not throw the invalidateSize after the animation(0.5s) is complete, it will throw it as soon as it starts, so I'm left with the grey space in my map container.

Adding the invalidateSize forces the leaflet to double check the true size of the container.

@RobbieTheWagner
Copy link
Author

We should not have to introduce hacks to get it to finish loading tiles though. There is still some sort of bug here.

@bmulcahy
Copy link

@rwwagner90 agreed I was just supplying a temporary fix.

@techieshark
Copy link

Probably unrelated but I had an issue where if I called marker.openPopup() and then immediately called map.fitBounds() the tiles stopped loading and I got a blank black map.

Putting this here in case it applies to anyone else who stumbles upon this issue via a random google search.

@patriciaorgan
Copy link

I have this issue also, but the map worked fine in a div until I moved the div to inside my form. not all tiles load but if I resize the browser they all load fine. the zoom for me does not reload the tiles. My Height was at a fixed px, tried the width at 100% and at fixed but did not change, still missing tiles.Using leaflet 0.7.3 still, if this is solved in 0.8-dev will move to it.

@mourner
Copy link
Member

mourner commented Jul 3, 2015

@patriciaorgan call map.invalidateSize() after manipulating the container and it should be fine.

@osro
Copy link

osro commented Aug 3, 2015

I also encountered this issue with 1.0 beta 1. Map worked fine until I added setView:true on locate options. The map zooms correctly to the location, but the tiles are not loaded until I pan or zoom out.

map.locate({
  setView: true
});

edit: there was another issue for this already #3659

@perliedman
Copy link
Member

I made a new version of @rwwagner90's plunkr, that uses latest dev-version of Leaflet: https://playground-leaflet.rhcloud.com/jis/edit?html,output

It seems the problem has been resolved, since all tiles load without problem. Checking @jieter's jsfiddle (http://jsfiddle.net/rord0mak/) seems to indicate the same thing.

I'm closing this as fixed, but feel free to add more info if the problem persists.

@qholness
Copy link

qholness commented Aug 6, 2016

This is probably a little silly but I was having this issue and then I realized I was running JS before I was loading css.
For some odd reason, when I swap their positions (i.e., load CSS first, then load JS) then this issue stops happening.

Dunno if anyone else had a similar issue but it fixed this problem for me.

@michaeltrim
Copy link

@mourner So using leaflet 1.0.3 I have been getting this error quite a lot, its on larger maps and is definitely a race condition bug that happens intermittently. If I comment out the contents of _abortLoading the issue goes away, what impact would that have if I were to run with it as a solution?

@perliedman
Copy link
Member

@michaeltrim do you have a test case that reproduces the problem? Also check #5622 and #5381 to see if they apply to you, they are related.

@michaeltrim
Copy link

@perliedman It's in a large client codebase so not easily as the problem is so intermittent anyway. It looks like it could well be related to those two issues. My app is running in electron and loading tiles from the filesystem. Is that fix in 1.1.0 ?

@perliedman
Copy link
Member

@michaeltrim #5381 was fixed in 1.1.0 but introduced another problem (#5622), that we're fixing in the upcoming release.

@canonex
Copy link

canonex commented Sep 5, 2017

I experienced the same issue happens using less-js compiler.
Removed the compiler the issue is gone.

@karimlouk
Copy link

github
i have the same problem with my react-leaflet app.

@humblecoder
Copy link

Having a very similar issue using "leaflet": "^1.3.4" with "vuetify": "^1.2.0" No solution found.
screen shot 2018-10-13 at 8 48 13 pm

@cherniavskii
Copy link
Sponsor Collaborator

@humblecoder most probably you forgot to import leaflet.css styles

@humblecoder
Copy link

@cherniavskii You were right . . .kinda. I was using https://unpkg.com/leaflet@1.3.4/dist/leaflet.css but it wasn't working. I switched to http://cdn.leafletjs.com/leaflet/v0.7.7/leaflet.css and it worked. 🤷‍♂️

@cherniavskii
Copy link
Sponsor Collaborator

but it wasn't working

What do you mean by that? That link works for me.

I switched to http://cdn.leafletjs.com/leaflet/v0.7.7/leaflet.css and it worked.

Note, that those are styles for leaflet v0.7.7. You should use the same version of .js and .css Leaflet files

@humblecoder
Copy link

humblecoder commented Oct 14, 2018

@cherniavskii Correct, the 0.7.7 link works. The one I was using previously (1.3.4) didn't. FYI, I'm simply require ('leaflet') in VueJS, not importing the JS from CDN.

@cherniavskii
Copy link
Sponsor Collaborator

Both of them work for me.

Btw, you can import css from node_modules as well

@humblecoder
Copy link

Yup, just did that as well 😆 . . . import from leaflet/dist/leaflet.css. All is well now. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests