Leaflet / Leaflet

πŸƒ JavaScript library for mobile-friendly interactive maps πŸ‡ΊπŸ‡¦
https://leafletjs.com
BSD 2-Clause "Simplified" License
40.28k stars 5.76k forks source link

Space between tiles on fractional zoom levels #3575

Open Eschon opened 8 years ago

Eschon commented 8 years ago

Note

This issue now has 150+ comments, most of which are concerned with hacky workarounds. I've compiled a list of a few comments that are discussing potential solutions:

(See also https://github.com/Leaflet/Leaflet/issues/3575#issuecomment-1108533500.)

Relevant browser bugs, filed by @hyperknot:


(Also, mix-blend-mode is now used to alleviate the tile gaps in Chromium-based browsers: https://github.com/Leaflet/Leaflet/pull/8891.)

/@Malvoz

screen shot 2015-06-30 at 14 39 12

As the screenshot shows there is a small space between the tiles. This happens to me on fractional zoom levels in Safari, Chrome and Opera. It seems like this was fixed in #2377 but later removed in 3ccbe5b in favour of better performance.

I tried reimplementing the fix to see the performance impact myself and it was pretty bad furthermore it didn't even fix the problem for Chrome.

Does anyone have an idea how this can be fixed? Fractional zoom levels would be a great feature for our app but as long as this bug remains we can't really use them.

Edit: I set up a jsFiddle that shows the problem: http://jsfiddle.net/Eschon/pw9jcjus/1/

mourner commented 8 years ago

No idea. :( Although I only saw the problem in Safari, β€” Chrome/Opera worked fine.

IvanSanchez commented 8 years ago

Does anyone have an idea how this can be fixed?

I have an idea, but it's linked to the words "radical" and "experimental":

https://github.com/IvanSanchez/Leaflet.gl

One of the things I discovered is that textures have to be clamped. i.e. if this line is removed : https://github.com/IvanSanchez/Leaflet.gl/blob/master/src/core/GlUtil.js#L74 slivers ("spaces between tiles") appear (as the texture wraps around the triangle and half a pixel of the other side of the tile is shown). So, my educated guess is that Webkit is antialiasing half a pixel on the edge of each image.

As of now, the tilelayer triangles form a complete mesh that is rendered without slivers... but the words "experimental" will be all over that for the time being.

Eschon commented 8 years ago

@mourner It has a stronger effect in Safari but it is also visible in Chrome. screen shot 2015-06-30 at 15 42 07

@IvanSanchez Wow nice work, but I don't think that I can use that (yet).

mourner commented 8 years ago

@Eschon yeah, I probably didn't notice this because on Retina screen, it's even less visible.

Eschon commented 8 years ago

This seems to affect Firefox now too in some cases. I couldn't reproduce it in jsFiddle but if you look at that site I get the same spaces in Firefox 39 and 41.0a2

Edit: This seems to be a problem with iframes. If I open the map directly it doesn't reproduce.

IvanSanchez commented 8 years ago

I doubt this will be possible for 1.0 without a lot of hacking. Pushing to the bottom of the backlog.

afilp commented 8 years ago

We experience the same problem on latest Chrome too. I wish that you find a possible fix as OpenStreet maps do not show well with these lines. Thanks.

cmulders commented 8 years ago

Especially on darker maps or images (in my case) the issue is quite noticeable. I think that antialiasing in the browser causes the spaces between tiles due to the fractional transform during zooming, as mentioned by Ivan.

Until a better solution is available, I will use this workaround (or hack) to make the tiles 1 pixel larger, with the side effect that they overlap by 1 px. Besides that the tiles will be slightly enlarged and scaled (by 1px).

/* 
 * Workaround for 1px lines appearing in some browsers due to fractional transforms
 * and resulting anti-aliasing.
 * https://github.com/Leaflet/Leaflet/issues/3575
 */
(function(){
    var originalInitTile = L.GridLayer.prototype._initTile
    L.GridLayer.include({
        _initTile: function (tile) {
            originalInitTile.call(this, tile);

            var tileSize = this.getTileSize();

            tile.style.width = tileSize.x + 1 + 'px';
            tile.style.height = tileSize.y + 1 + 'px';
        }
    });
})()
Eschon commented 8 years ago

@cmulders Thanks for the workaround! Since we mainly have maps of ski resorts that are mostly white and blue. We ignored the spaces for now, with your workaround it looks a lot better.

hyperknot commented 8 years ago

Made a jsbin with cartodb dark tiles, the lines are visible even on retina screen: http://jsbin.com/ratoli/5/edit?html,js,output

Safari / Chrome buggy, FF OK for me.

hyperknot commented 8 years ago

We should make a Leaflet-less Chrome bug report for this, so it's fixed in future Chrome versions.

Eschon commented 8 years ago

@hyperknot I think there is already a Chrome bug report, but I can't find it right now.

mehany commented 8 years ago

I am using leafletjs 1.0 beta and I am seeing this bug as well sometimes on ( latest: chrome , safari, firefox ) on osx El Capitan. when I edit an images's transform css property by 1px, the edited tile aligns with the tile next to it.

// ex.
 transform: translate3d(1556px, -81px, 0px);
 // to
 transform: translate3d(1555px, -80px, 0px);

Sometimes this bug does not appear though ( that 1px of difference ) !! not sure why :( but I am currently guessing it has to do with the styling of the map container.

Edit: Say the tiles align correctly on render and does not show any 1px of difference, a gap will visible on map move event. The behavior is like a flash of a 1px border around the tile.

href commented 8 years ago

I found a css workaround that gets rid of the problem for me on Safari 9.1 on OS X El Capitan:

.leaflet-tile-container img {
    box-shadow: 0 0 1px rgba(0, 0, 0, 0.05);
}

I can't take much credit however, I took it from the following Stackoverflow answer which also hints at why this helps: http://stackoverflow.com/a/17822836/138103

Using box-shadow solves the problem more directly: it extends the repaint dimensions of the box, forcing WebKit to repaint the extra pixels. The known performance implications of box-shadow in mobile browsers are directly related to the blur radius used, so a one pixel shadow should have little-to-no effect.

Maybe this can be used as a proper fix, though it would probably be good if others tried this first. It works for me however and doesn't seem to lead to any problems in other browsers.

IvanSanchez commented 8 years ago

@href Maybe you could make a pull request with that fix? That'd make it easier to test.

hyperknot commented 8 years ago

@href @IvanSanchez Are you sure that this won't affect rendering performance? I think we should profile how does adding a box-shadow with an alpha opacity affect mobile devices for example, it might have a visible effect on performance, or even possibly disable hardware acceleration.

href commented 8 years ago

@IvanSanchez done.

Are you sure that this won't affect rendering performance? I think we should profile how does adding a box-shadow with an alpha opacity affect mobile devices for example, it might have a visible effect on performance, or even possibly disabling hardware acceleration for panning.

I'm not at all sure what the effect of this is. From the Stackoverflow answer this doesn't seem to have an effect, but that's just trusting someone else's opinion on a tangentially related issue. It would surely be a great idea if someone more knowledgable could look into it and profile my workaround.

I just got lucky when I googled around and I wanted others to know.

IvanSanchez commented 8 years ago

@hyperknot @href I'll happily perform some benchmarks in a couple of weeks against my set of test browsers.

hyperknot commented 8 years ago

@IvanSanchez @href Also, it might be good to limit this hack to affected browsers only, not just apply it globally.

@IvanSanchez is there a recommended workflow for profiling Leaflet rendering performance?

href commented 8 years ago

Also, it might be good to limit this hack to affected browsers only, not just apply it globally.

I personally only see it in Safari, so it would make sense to limit it for safari. There were mentions of people seeing this in Chrome however. Does anyone still see this bug in other webkit browsers?

IvanSanchez commented 8 years ago

It's quite trivial to add a CSS class to the map only for selected browsers, in the L.Map constructor stuff.

@href The issue is present in my Chromium 49

@hyperknot I'm not aware of the best way to profile these kind of things. Let's talk about that this weekend ;-)

IvanSanchez commented 8 years ago

@href Also, this fix kinda messes up tiles in my Chromium 49... it blurs the whole image instead of just the edge. Original, with .leaflet-container { background: black }:

fractional-space

With img.leaflet-tile.leaflet-tile-loaded { box-shadow: 0 0 1px rgba(0, 0, 0, 0.05); }:

fractional-space1

:crying_cat_face:

href commented 8 years ago

That's weird. The issue is not present for me in Chromium 49 (OS X El Capitan), nor does the box-shadow seem to have any influence either way.

hyperknot commented 8 years ago

Made an up to date playground: http://playground-leaflet.rhcloud.com/yid/1/edit?html,output

I will be submitting Leaflet-less bugs to Webkit and Blink teams.

hyperknot commented 8 years ago

OK, successfully made a minimal Leaflet-less example reproducing the issue: http://playground-leaflet.rhcloud.com/say/1/edit?html,output

hyperknot commented 8 years ago

Chromium bug report submitted: https://bugs.chromium.org/p/chromium/issues/detail?id=600120

hyperknot commented 8 years ago

Webkit bug submitted: https://bugs.webkit.org/show_bug.cgi?id=156130

hyperknot commented 8 years ago

img-less version: http://playground-leaflet.rhcloud.com/foqi/1/edit?html,output

hyperknot commented 7 years ago

Some update from Chromium bug report, from trchen@chromium.org

Hey just want to give you guys a quick update. I'm actively working on this issue.

The problem was due to we raster things in an element's local space. If the element has a fractional translation, then the rasterized texture would be pasted onto the screen with the fractional translation using linear resampling, resulting in the blur.

The solution is to raster things in a space that is pixel-aligned to our physical screen. i.e. applying the fractional translation to the vector graphics commands instead of rastered texture.

The fix will be coming in two parts:

  1. The first part that allows our raster system to raster with any general matrix. This part is almost done. I have a WIP but it still has a bug that causes performance regression. I expect to finish it within a few days. https://codereview.chromium.org/2075873002/
  2. The second part that allows our tiling management to be able to manage set of textures that comes with different raster translation. I was originally going for general matrix but turns out the tile coverage computation becomes very difficult. I will instead do a simpler version that only supports translation and scale. I estimate this will need another week of work.

I will try to land this by M53 branch but time is tight. Pretty sure this can be fixed by M54.

hyperknot commented 7 years ago

https://bugs.chromium.org/p/chromium/issues/detail?id=600120

To the bug reporter:

The test case you uploaded triggered an edge case in Chromium that we handled pixel snapping in a different way. If you add some contents to the tiles you will no longer see seams in the background. We can easily fix the edge case, however, there can still be seams in the tile contents.

This is a hard problem, and the only known perfect solution is to use FSAA, which no browsers do due to the CPU/memory costs. Every vendors develop their own heuristics to make some contents look nicer, at the costs of some contents look incorrect.

For example: http://jsbin.com/wayapiz/

Chrome draws with the right geometry, but edges bleed like a stuck pig. Firefox draws the first box as 49x49, the second box as 49x50. Edge draws the first box as 50x50, the second box as 50x49. Both with aliasing.

Also if you add rotation to the container, all implementation bleed the edges.

My recommended workaround is to add some overlap between tiles. You know, just like making a poster with A4 papers. The idea is to make sure each tile overdraw at least one pixel after all the scaling, so each physical pixel is guaranteed to be covered by at least one opaque pixel. The width of the overlap depends on the smallest scale you want to support. For example, if you want to support down to 1/4 size, then add 4 pixels of overlap.

Below are the detailed analysis for other developers who wants to tackle this.

The repro uploaded by the reporter wasn't a very good repro because it triggers an optimized code path in cc. When we detected a layer completely solid color, we draw it as a SolidColorLayerImpl, which doesn't have the concept of contents scale. Here is a simplified repro: http://jsbin.com/hodoxew/

The black box will draw as a SolidColorLayerImpl of size (100, 100), which will generate a single solid color quad of size (100, 100), which will be projected to the screen by draw transform to become (99.5, 99.5). With anti-alias, the rightmost column and bottom row of pixels will be drawn in semi-transparent.

On the other hand, if we add something to the layer to trick cc into thinking the layer being non-solid color, now the problem comes in two cases:

  1. The last tile being solid color, for example: http://jsbin.com/zexawa/

The contents size will be rounded up, so although the scaled layer size is only (600, 300)*0.995 = (597, 298.5), it will be over-drawn as (597, 299).

  1. The list tile being non-solid color, for example: http://jsbin.com/mewaguf/

The contents size will be rounded up. Again the actual scaled contents only covers (600, 300)*0.995 = (597, 298.5), the generated quads will cover (597,299). The last row of pixels are supposed to be semi-transparent due to anti-alias, but we actually fill it with layer background color, which is the background color of the element that created the layer.

There will be no seam in the background, however, background can bleed on the edges when the contents are supposed to cover it. For example: http://jsbin.com/vigufo/

hyperknot commented 7 years ago

An other test case, using non-solid color tiles: http://playground-leaflet.rhcloud.com/giqo/edit?output

mourner commented 7 years ago

@hyperknot fantastic update, but those two quotes seem to contradict each other (in one, a person says he works on a fix, and the next one says no good fix possible). So do we wait for a fix or just accept that it will be this way forever?

hyperknot commented 7 years ago

@mourner sorry I didn't include the full context, and you are right of course. So original bug report was: https://bugs.chromium.org/p/chromium/issues/detail?id=600120

At one point it got merged into an other issue: https://bugs.chromium.org/p/chromium/issues/detail?id=521364

In that other issue, we got the original long reply with a solution. I asked if he could check if the Leaflet case is indeed fixed by his work and he replied:

I'm sorry the leaflet issue is in fact a different problem. I will re-open issue 600120 and share my analysis.

So the Leaflet bug is now reopened and we got the 2nd reply in the Leaflet bug one. BTW, the latest update right now is:

I took a look at mobile Google map to see how they handled it. It turns out they are plagued by this problem too, but they did some UI trick to make it less obvious.

The first thing is they used the viewport tag to disallow browser pinch zoom, and implemented pinch zoom in JS. During pinch zoom, you can still see seams between tiles (very subtle on high-res displays). After the zoom gesture finish, they snap the scale factor to the closest tile set they have. So basically tiles are always displayed in native resolution except during zoom gesture.

Honestly we don't have a solid plan yet. I'm going to discuss with other paint people on Monday.

BTW, it seems that mobile Google Maps (I guess the website one) is using exactly the same technique as us.

hyperknot commented 7 years ago

@mourner @IvanSanchez some hard-core question with a possible solution using will-change: https://bugs.chromium.org/p/chromium/issues/detail?id=600120#c15

Is the current layerization important? I see each tile is forced into its own layer (translate3d), and these layers are scaled by some fractional value independently.

As long as we scale each map tile independently (either using layers or via drawImageRect + fractional CTM), for fractional scale values we get edges which are not pixel-aligned. This triggers the ole coincident edge anti-aliasing problem where discrete AA causes background color bleed.

The way I think this could work instead is to rasterize the map with a non-fractional scale (say 1.0) and then downscale atomically:

1) avoid individual tile layerization (use "transform: translate()" instead of "transform: translate3d()") 2) force container (whole map) layerization (use "transform: scale3d()" on the container) 3) force container layer rasterization with a scale == 1.0

With these changes, map tile images should be rasterized with a scale of 1.0 (=> edges are pixel-aligned => no seaming artifacts), then the whole map layer should be down-scaled atomically.

Unfortunately #3 is not trivial, and the only hack I can think of is

e.g. http://jsbin.com/yaqeru/10/embed?output

chrishtr/danakj did I get the layer raster scale semantics right, and is there a better way to lock the layer rasterization scale to a particular value?

Can you help answer it?

ericsoco commented 7 years ago

I know a fix is in progress (thanks all for pushing on that!), but in the meantime...@cmulders' monkeypatch works best for me. Here's an version that brings in Leaflet as an npm module, uses ES6, and ensures no double-patch, in case anyone is looking for some copypasta goodness.

https://gist.github.com/ericsoco/5712076f69f9068b11d41262b9e93666

import leaflet from 'leaflet';
// ...
patchMapTileGapBug();
// ...
function patchMapTileGapBug () {

    // Workaround for 1px lines appearing in some browsers due to fractional transforms
    // and resulting anti-aliasing. adapted from @cmulders' solution:
    // https://github.com/Leaflet/Leaflet/issues/3575#issuecomment-150544739
    let originalInitTile = leaflet.GridLayer.prototype._initTile;
    if (originalInitTile.isPatched) return;

    leaflet.GridLayer.include({
        _initTile: function (tile) {
            originalInitTile.call(this, tile);

            var tileSize = this.getTileSize();

            tile.style.width = tileSize.x + 1 + 'px';
            tile.style.height = tileSize.y + 1 + 'px';
        }
    });

    leaflet.GridLayer.prototype._initTile.isPatched = true;

}
briantjacobs commented 7 years ago

I'm glad people are looking into this. In case useful to anyone as a less than ideal workaround: if you're willing to sacrifice inertia, setting intertia:false on the map init eliminates the gaps, and possibly zoomAnimation:false

IvanSanchez commented 7 years ago

I think I've found an (overcomplicated) solution for this, I've put it up at https://github.com/Leaflet/Leaflet.TileLayer.NoGap - comments are welcome, as I'm not sure of the performance of that thing on old computers/phones.

cc @Eschon @hyperknot

IvanSanchez commented 7 years ago

BTW, the bug also happens when using Chrome on a high-DPI (2560x1140) screen, without the need to use leaflet fractional zoom nor browser zooming.

Eschon commented 7 years ago

Sorry for the late reply.

I just tested your solution. The Demo worked fine for me so I tried to add the fix to my application. Since I was still using an old 1.0-dev version of Leaflet I updated it to 1.0.0-rc3 from http://leafletjs.com/download.html

The first thing that I noticed is that there seems to be an error here. It shows an error that map undefined so I changed it to this._map.

After that it worked but it is really slow on my work computer. Especially in Chrome but Firefox was also slow.

Surprisingly it didn't have that big of an impact on the phones that I tested. Firefox on Android seems to be the same as before, Chrome is a bit slower but not too bad. Safari on iOS also seems to be about the same as before.

Another strange thing that I noticed is that on mobile Chrome and Safari pinch-zooming seems to snap to fixed levels

IvanSanchez commented 7 years ago

@Eschon Thanks for testing this!

I always make the mistake of typing map instead of this._map :-(

After that it worked but it is really slow on my work computer. Especially in Chrome but Firefox was also slow.

Could you run some profiler on that computer? It'd be nice to know if the slowdown is related to canvas operations. I noticed that was a bit performance hit on IE9-IE11.

Based on whether this solution makes the map slower on some platforms, it might make sense to make it completely optional and turned off by default. Some feedback here is important!

I haven't really looked at how the OL3 folks solve this issue, or whether they do some performance detection on the browser before enabling canvas compositing. Might be worth to have a look.

Another strange thing that I noticed is that on mobile Chrome and Safari pinch-zooming seems to snap to fixed levels

I bet that's just fiddling with the zoomDelta and zoomSnap options, and not related to this bug.

Eschon commented 7 years ago

Could you run some profiler on that computer? It'd be nice to know if the slowdown is related to canvas operations. I noticed that was a bit performance hit on IE9-IE11.

Should I just record a JavaScript CPU Profile with the Chrome dev-tools? I don't have much experience with that performance testing stuff so I don't really know what to make of the data it shows.

IvanSanchez commented 7 years ago

Yeah, that's the thing. Then look for wide things on the flame chart.

Eschon commented 7 years ago

I just did a profile with the fix and another one without it. I tried to do about the same for both profiles.

The first thing I noticed that the version with the fix had an additional "peak" (I don't know if this is the right word) caused by _onZoomTransitionEnd

In the version without the fix _onZoomTransitionEnd took 1.8ms and I didn't even see it in the chart. In the version with the fix it took 6ms and caused a "peak" in activity.

The other two "peaks" look pretty similar in the two profiles things just seem to happen a little different but the gap between them is bigger in the version with the fix.

I hope that helps you somehow. I also saved the two profiles so if you need them I could upload them somewhere.

AlexanderUhl commented 7 years ago

As a dirty workaround for the time being, I just add a scale factor of .002 to the original function that sets the translate3d property to each tile which eliminates the gaps almost entirely. ".002" was the lowest scaling factor in my test cases to make the gaps fully disappear. I didn't notice any side effects.

Tested in latest Chrome, Firefox and IE 11. Tested on a 40" 4k Monitor with windows scaling set to 150% with various zoom values within the browsers.

themre commented 7 years ago

any progress on this bug? @AlexanderUhl does your solution causes any modification of images when placed or is this only affected while moving/zooming?

themre commented 7 years ago

It seems will-change CSS property was added to resolve this issue, only need to check FF performance warnings, described here: https://github.com/Leaflet/Leaflet/issues/4686

AlexanderUhl commented 7 years ago

@themre : the scale property applies to all image tiles when added and are permanently. The style attr will look like this:
style="width: 256px; height: 256px; transform: translate3d(545px, 745px, 0px) scale(1.002); opacity: 1;"

themre commented 7 years ago

But that would cause to have image stretched a bit which is an unwanted effect. I tried 1.0.3 version which has will-change CSS property but it still causes white gap. Will try to look some more.

AlexanderUhl commented 7 years ago

Ofc it stretches the img, thats the approach ;-) I agree that this workaround is not ideal, but tbh that is usually the nature of a workaround, isn't it? Nonetheless, this approach is working best for me with negligible side effects (for my use case which is a mapping application with tiles from mapbox, here, osm etc.)

themre commented 7 years ago

well, if we use normal translate, the problem is gone. Don't really know if translate3d CSS property is so much faster than normal translate, perhaps it would be best to add this flag as an option.I think I'm gonna replace translate3d with translate.