Skip to content

Commit bb4e33d

Browse files
committed
Fix issue with rasters disappearing
The getImageData(callback) function as previously written would invoke the callback immediately if image data was already loaded, or asynchronously if image data was not. (I knew this was weird so I noted this in the function's comment.) This turns out to be a problem with Leaflet 1.x because it expects createTile's callback (done) function to only be invoked after createTile has returned. If done is called too soon, then it's ignored, because done first checks the GridLayer's tile cache to see if the tile that's done is even still wanted. Since createTile hasn't returned the tile isn't in the cache yet, and done becomes a no-op.
1 parent a807435 commit bb4e33d

File tree

2 files changed

+26
-8
lines changed

2 files changed

+26
-8
lines changed

inst/htmlwidgets/leaflet.js

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2059,11 +2059,15 @@ methods.addRasterImage = function (uri, bounds, opacity, attribution, layerId, g
20592059
var imgDataCallbacks = [];
20602060

20612061
// Consumers of imgData, w, and h can call this to be notified when data
2062-
// is available. Unlike most async/promise-based APIs, the callback will
2063-
// be invoked immediately/synchronously if the data is already available.
2062+
// is available.
20642063
function getImageData(callback) {
20652064
if (imgData != null) {
2066-
callback(imgData, w, h, imgDataMipMapper);
2065+
// Must not invoke the callback immediately; it's too confusing and
2066+
// fragile to have a function invoke the callback *either* immediately
2067+
// or in the future. Better to be consistent here.
2068+
setTimeout(function () {
2069+
callback(imgData, w, h, imgDataMipMapper);
2070+
}, 0);
20672071
} else {
20682072
imgDataCallbacks.push(callback);
20692073
}
@@ -2107,9 +2111,12 @@ methods.addRasterImage = function (uri, bounds, opacity, attribution, layerId, g
21072111
async: true
21082112
});
21092113

2114+
// NOTE: The done() function MUST NOT be invoked until after the current
2115+
// tick; done() looks in Leaflet's tile cache for the current tile, and
2116+
// since it's still being constructed, it won't be found.
21102117
canvasTiles.createTile = function (tilePoint, done) {
21112118
var zoom = tilePoint.z;
2112-
var canvas = _leaflet2.default.DomUtil.create("canvas", "leaflet-tile");
2119+
var canvas = _leaflet2.default.DomUtil.create("canvas");
21132120
var error = void 0;
21142121

21152122
// setup tile width and height according to the options
@@ -2244,6 +2251,8 @@ methods.addRasterImage = function (uri, bounds, opacity, attribution, layerId, g
22442251
}();
22452252

22462253
if ((typeof _ret7 === "undefined" ? "undefined" : _typeof(_ret7)) === "object") return _ret7.v;
2254+
} catch (e) {
2255+
error = e;
22472256
} finally {
22482257
done(error, canvas);
22492258
}

javascript/src/methods.js

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -993,11 +993,15 @@ methods.addRasterImage = function(uri, bounds, opacity, attribution, layerId, gr
993993
let imgDataCallbacks = [];
994994

995995
// Consumers of imgData, w, and h can call this to be notified when data
996-
// is available. Unlike most async/promise-based APIs, the callback will
997-
// be invoked immediately/synchronously if the data is already available.
996+
// is available.
998997
function getImageData(callback) {
999998
if (imgData != null) {
1000-
callback(imgData, w, h, imgDataMipMapper);
999+
// Must not invoke the callback immediately; it's too confusing and
1000+
// fragile to have a function invoke the callback *either* immediately
1001+
// or in the future. Better to be consistent here.
1002+
setTimeout(() => {
1003+
callback(imgData, w, h, imgDataMipMapper);
1004+
}, 0);
10011005
} else {
10021006
imgDataCallbacks.push(callback);
10031007
}
@@ -1041,9 +1045,12 @@ methods.addRasterImage = function(uri, bounds, opacity, attribution, layerId, gr
10411045
async: true
10421046
});
10431047

1048+
// NOTE: The done() function MUST NOT be invoked until after the current
1049+
// tick; done() looks in Leaflet's tile cache for the current tile, and
1050+
// since it's still being constructed, it won't be found.
10441051
canvasTiles.createTile = function(tilePoint, done) {
10451052
let zoom = tilePoint.z;
1046-
let canvas = L.DomUtil.create("canvas", "leaflet-tile");
1053+
let canvas = L.DomUtil.create("canvas");
10471054
let error;
10481055

10491056
// setup tile width and height according to the options
@@ -1179,6 +1186,8 @@ methods.addRasterImage = function(uri, bounds, opacity, attribution, layerId, gr
11791186
}
11801187
}
11811188
}
1189+
} catch(e) {
1190+
error = e;
11821191
} finally {
11831192
done(error, canvas);
11841193
}

0 commit comments

Comments
 (0)