Leaflet: Auto-detect changes to map size (and invalidateSize)

[broofa]

I have a map that's styled like this:

#map {position: absolute; top: 10px; left: 10px; right: 10px; bottom: 90px;}

... i.e. it has no explicit height. So, of course, leaflet isn't rendering all of the tiles. However I do setTimeout(map.invalidateSize.bind(map)), after creating the map it renders properly.

This has me thinking that it's not actually necessary for the map to have an explicit height - it's just a question of insuring Leaflet is smart about detecting the height at the appropriate time.

[broofa]

Actually, in looking at this further it seems like this is just an issue with how map.getSize() is caching the size. If I change the implementation to:

getSize: function () {
  return new L.Point(this._container.clientWidth, this._container.clientHeight);
}, 

... everything works. So what is the purpose of the caching code in https://github.com/CloudMade/Leaflet/blob/master/src/map/Map.js#L313 ? Removing this lets you get rid of all the code around invalidateSize (no?)

Let me know if/how you want to move forward on this and I can work up a pull request.

[danzel]

We cache client(Width/Height) as reading them will force a layout, which can be slow and will stop any animations in progress.
We also read it a lot, so keeping it quick is important.

It is interesting that we detect the initial size incorrectly. Could you make a minimal jsfiddle reproducing this please?
I'm wondering if it is something on your page causing the size detection to be incorrect the first time or if it is really just broken.
Thanks!

[broofa]

http://jsfiddle.net/dCK8j/ demonstrates the problem I'm seeing. (This repros on chrome, safari, firefox, and opera (all macosx).)

I believe the root cause here is that I have my map element hidden via display:none when I create the map and call setView the first time. As a result, getSize() caches a bounds of [0, 0]. Note that resizing the window after the page has loaded fixes the rendering problem (for whatever reason.)

[danzel]

I believe this is the same issue as seen on #926
The hidden element causes it, you can't measure a display:none element.

You can simplify the code a bit:
http://jsfiddle.net/dCK8j/6/
Just need to do

    map.invalidateSize();​

after making it display:block

[broofa]

Dave, thanks for being so responsive here. Yeah, I realized I don't actually need the setTimeout() to fix the problem shortly after posting that fiddle. :)

Can you tell me more about the conditions under which you found the clientWidth/Height query to be expensive? I'm actually pretty surprised by your comment there. The leaflet-container rule sets overflow:hidden which, in theory, should make it's dimensions immune to layout changes in it's child elements. I would fully expect browsers to be _very_ good about intelligently caching element dimensions.

If I'm wrong there, I'd love to know which platforms I need to look out for this problem on. Thanks!

[broofa]

(oh, and as for this specific issue, I'll understand if you want to argue that it's my responsibility to call invalidateSize when the map is displayed - feel free to close unless you think there's some concrete action that should come out of this.)

[danzel]

Yeah it is a weird use case. Looks like it is something possible to support though!
We could listen for changes to the style (display, width, height) attributes.

http://stackoverflow.com/questions/1397251/event-detect-when-css-property-changed-using-jquery
http://stackoverflow.com/questions/4562354/javascript-detect-if-event-lister-is-supported
http://stackoverflow.com/questions/1882224/is-there-an-alternative-to-domattrmodified-that-will-work-in-webkit

Probably worth adding, although there are only a few small uses cases for it. @mourner would be best to decide if we want it. Let's leave this open so he can take a look when he is back.
For now if you can handle invalidateSize at least that will get you going.

On the clientWidth/Height thing, I didn't write the caching code, but I believe the reasons for it are around keeping the browser from doing unneeded layouts. The overflow:hidden does make sense. Maybe in some (older) browsers measuring clientWidth/Height is slow or forces a full layout? @mourner wrote this code 2 years ago, maybe he can still remember why? :-)

[mourner]

As far as I remember, asking any visible element for dimensions like clientHeight always forces a reflow, no matter if it's overflow: hidden. See here http://gent.ilcore.com/2011/03/how-not-to-trigger-layout-in-webkit.html Caching it sped up everything considerably according to my profiling tests.

I've made a JSPerf test to demonstrate this: http://jsperf.com/reflow-with-clientheight-vs-cached-size

[mourner]

Regarding auto-detecting size changes, unfortunately mutation events are not supported in all browsers, and the only viable fallback for this is invalidating size periodically with a timer, which is a really bad solution performance-wise. So I don't see any way in which it can be implemented gracefully.

[broofa]

'Ran the jsperf test on a few more platforms. Unfortunately I couldn't get the test to run to completion on IE 7/8 so those results aren't valid. Aside from that, Opera (on my MBP) is the worst performer, coming in at ~50K ops/second for the uncached test.

I'm not sure comparing cached .vs. uncached perf is particularly meaningful though. The cached version of the test is practically a no-op, so will obviously be much faster in relative terms. But in absolute terms, even the slowest even the paltry 50K/sec that Opera's doing is plenty fast if all you're doing is testing clientWidth & clientHeight of the container in a 1-second timer. By my calculations that would add < 0.1% to CPU load.

[broofa]

So, by way of moving this forward, how would you feel about possibly adding a flag that allowed devs to specify whether the size should be cached? (I actually don't know that resize detection is necessary to fix my original issue, btw)

[jmkelly]

I think I have been hit by this one, with my use case a little different. I'm using twitter bootstrap's tabs, with the map being created on a tab that is hidden. See http://jsfiddle.net/ngyrL/3/

I'm not controlling the tab interactions, so adding map.invalidateSize();​ while doable, it is a pain, and I would much prefer Leaflet allowed me not to have to worry about this. Any more thoughts?

[danzel]

This probably shouldn't live in core, but something like this could work (Maybe a plugin?)
On map creation, check if element is display:none or visibility:hidden, if it is, fire up an interval that watches until that changes (it becomes visible), then call invalidateSize.
It won't be that great, as you need to wait for the interval to fire before showing the map, which may be moments after your map should really be visible.

On supported browsers the plugin could listen for property changes to make it snappy.

[mourner]

Closing the issue as we don't want to implement this for performance, browser support and insignificance reasons (you can easily just use invalidateSize when necessary).

[HarelM]

My two cents:
I think a minor fix would help in order to prevent a bug I had: keep trying to get the size if it is 0,0.

getSize: function () {
        if (!this._size || this._sizeChanged || (this._size && this._size.x == 0 && this._size.y == 0) {
            this._size = new Point(
                this._container.clientWidth || 0,
                this._container.clientHeight || 0);

            this._sizeChanged = false;
        }
        return this._size.clone();
    },

The element could be initialized with zero size but later change without calling resize (I think, at least this is what I think happened in the scenario I had to fix).

[CourtneyJane2904]

I appreciate that this issue has been closed; I had this issue today and reading this discussion inspired my fix (bare in mind that my case is VERY simple- just a map with a view and a marker)

It might help a fellow noob like myself:

$('#tab').click(function() {setTimeout(function(){setView|tileLayer|add to map|add markers},200);}

This prevents the default none display of tab content interfering with the tiling process (I'm sure this has probably been mentioned though!) :)