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

Ensure SourceCache reuses tiles across the dateline #1834

Closed
buchanae opened this issue Dec 14, 2015 · 10 comments
Closed

Ensure SourceCache reuses tiles across the dateline #1834

buchanae opened this issue Dec 14, 2015 · 10 comments
Labels
performance ⚡ Speed, stability, CPU usage, memory usage, or power usage

Comments

@buchanae
Copy link
Contributor

This started when I was animating a flight path across the Pacific, and setting the map center to follow the flight. The flight path was generated using arc.js. I would get a frame or two where the map would turn black (satellite style background), so I started investigating.

I realized that the blackout happened as the path crossed the line from positive longitude to negative longitude (anti-meridian), i.e. as the flight passed that line, the center of the map goes from 179 longitude to -180 longitude.

Transforming the longitude into a positive value (i.e. -185 --> 175) fixed the blackout, and I think it's a slight improvement to tile rendering performance.

I tried to demonstrate that here: http://jsfiddle.net/wodvrjj0/3/

The transformation function I used is:

if (lng < 0) {
  lng = 360 + (lng % -360);
}

During my debugging, I modified Transform#center to do this fix and things in Mapbox still seemed to work ok under light testing. I have no idea if that's the correct place for a fix, I don't know the code well enough to decide. TileCoord#cover is another possible place for a fix; again, I don't know.

@lucaswoj lucaswoj self-assigned this Dec 17, 2015
@lucaswoj
Copy link
Contributor

I have identified the underlying cause of this issue. We aspire to use the same tiles when rendering the same region across the dateline but fail to do so in some cases. This test currently fails but should pass.

    t.test('reuses wrapped tile (when first tile was w != 0)', function(t) {
        var load = 0;
        var add = 0;

        var pyramid = createPyramid({
            load: function(tile) { tile.loaded = true; load++; },
            add: function() { add++; }
        });

        var t1 = pyramid.addTile(new TileCoord(0, 0, 0, 1));
        var t2 = pyramid.addTile(new TileCoord(0, 0, 0, 0));

        t.equal(load, 1);
        t.equal(add, 2);
        t.equal(t1, t2);

        t.end();
    });

@lucaswoj
Copy link
Contributor

Hm. While it is a real bug, fixing the tile retention problem doesn't seem to fix the flickering.

Commenting out the body of TilePyramid#removeTile does fix the flickering.

@mourner
Copy link
Member

mourner commented Jan 12, 2016

@lucaswoj did you figure it out eventually?

@lucaswoj
Copy link
Contributor

@mourner I fixed something but the bug persists. @kelvinabrokwa is looking at this bug this week.

@lucaswoj lucaswoj assigned kelvinabrokwa and unassigned lucaswoj Jan 12, 2016
@lucaswoj
Copy link
Contributor

@kelvinabrokwa The "something" I fixed is in this branch https://github.com/mapbox/mapbox-gl-js/tree/pyramid-1834

@lucaswoj lucaswoj assigned lucaswoj and unassigned kelvinabrokwa Jan 20, 2016
@lucaswoj lucaswoj removed their assignment Feb 12, 2016
@lucaswoj lucaswoj added performance ⚡ Speed, stability, CPU usage, memory usage, or power usage and removed bug 🐞 labels Jul 29, 2016
@lucaswoj lucaswoj changed the title Tile rendering performance improved when normalizing longitude Ensure SourceCache reuses tiles across the dateline Jul 29, 2016
@mourner
Copy link
Member

mourner commented Nov 2, 2016

Trying to replicate the issue with http://jsfiddle.net/wodvrjj0/3/ and I don't currently see any blackouts in Chrome or Firefox on the provided JSFiddle. Am I looking at a wrong place?

@lucaswoj
Copy link
Contributor

lucaswoj commented Nov 2, 2016

I’m also unable to reproduce this bug. I think it’s safe to close this issue.

On Nov 1, 2016, at 8:29 PM, Vladimir Agafonkin notifications@github.com wrote:

Trying to replicate the issue with http://jsfiddle.net/wodvrjj0/3/ http://jsfiddle.net/wodvrjj0/3/ and I don't currently see any blackouts in Chrome or Firefox on the provided JSFiddle. Am I looking at a wrong place?


You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub #1834 (comment), or mute the thread https://github.com/notifications/unsubscribe-auth/AARK2mwIYQXF_fu2PKhwcuEKcdjaAwaMks5q6AOWgaJpZM4G0qW5.

@lucaswoj lucaswoj closed this as completed Nov 2, 2016
@bfrengley
Copy link
Contributor

This issue appears again in the latest version of mapbox-gl-js (0.44.1, at the time of writing--it has also been present for several versions before that), as can be seen in http://jsfiddle.net/wodvrjj0/8/

@jfirebaugh
Copy link
Contributor

Yes, this is intended behavior. Changes to the label placement algorithm mean tiles can no longer be reused across the antimeridian.

@jfirebaugh
Copy link
Contributor

I should have looked at the fiddle before replying. 🤦‍♂️ The flickering is not intended. I'll open a fresh issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance ⚡ Speed, stability, CPU usage, memory usage, or power usage
Projects
None yet
Development

No branches or pull requests

6 participants