From 5993deb4c8e2a4270f606484ca7b873c0e4cc5f5 Mon Sep 17 00:00:00 2001 From: ghybs Date: Fri, 22 Jun 2018 12:09:42 +0400 Subject: [PATCH 1/3] test(GridLayer): fix graphical browser zoom in test on number of pruned tiles during zoom-in in "graphical browsers" (i.e. not PhantomJS) was failing due to missing some rAF to let the animation process fully go through and be able to prune the remaining 4 tiles. --- spec/suites/layer/tile/GridLayerSpec.js | 92 +++++++++++++++++-------- 1 file changed, 65 insertions(+), 27 deletions(-) diff --git a/spec/suites/layer/tile/GridLayerSpec.js b/spec/suites/layer/tile/GridLayerSpec.js index c8e8e243..273c50f1 100644 --- a/spec/suites/layer/tile/GridLayerSpec.js +++ b/spec/suites/layer/tile/GridLayerSpec.js @@ -599,46 +599,84 @@ describe('GridLayer', function () { // browsers due to CSS animations! it.skipInPhantom("Loads 32, unloads 16 tiles zooming in 10-11", function (done) { - // grid.on('tileload tileunload tileloadstart load', logTiles); + // Advance the time to !== 0 otherwise `tile.loaded` timestamp will appear to be falsy. + clock.tick(1); + // Date.now() is 1. - grid.on('load', function () { - expect(counts.tileloadstart).to.be(16); + // grid.on('tileload tileunload tileloadstart load loading', logTiles); + + // Use "once" to automatically detach the listener, + // and avoid removing the above logTiles + // (which would happen when calling "grid.off('load')"). + grid.once('load', function () { expect(counts.tileload).to.be(16); expect(counts.tileunload).to.be(0); - grid.off('load'); - - // grid.on('load', logTiles); - grid.on('load', function () { - - // We're one frame into the zoom animation, there are - // 16 tiles for z10 plus 16 tiles for z11 covering the - // bounds at the *end* of the zoom-*in* anim - expect(counts.tileloadstart).to.be(32); - expect(counts.tileload).to.be(32); - expect(counts.tileunload).to.be(0); + // Wait for a frame to let _updateOpacity starting. + L.Util.requestAnimFrame(function () { // Wait > 250msec for the tile fade-in animation to complete, // which triggers the tile pruning clock.tick(300); + // At 251ms, the pruneTile from the end of the z10 tiles fade-in animation executes. + // Date.now() is 301. - // After the zoom-in, the 'outside' 12 tiles (not the 4 - // at the center, still in bounds) have been unloaded. - expect(counts.tileunload).to.be(12); - - L.Util.requestAnimFrame(function () { - expect(counts.tileloadstart).to.be(32); + grid.once('load', function () { expect(counts.tileload).to.be(32); - expect(counts.tileunload).to.be(16); - done(); - }); - }); - map.setZoom(11, {animate: true}); - clock.tick(250); + // We're one frame into the zoom animation, + // so GridLayer._setView with noPrune === undefined is not called yet + // No tile should be unloaded yet. + expect(counts.tileunload).to.be(0); + + // Wait > 250msec for the zoom animation to complete, + // which triggers the tile pruning + // Animated zoom takes 1 frame + 250ms before firing "zoom" event + // => GridLayer._resetView => GridLayer._setView => _pruneTiles + // However, this "load" event callback executes synchronously, + // i.e. _tileReady still did not have a chance to prepare the + // setTimeout(250) for pruning after the end of the fade-in animation. + clock.tick(300); + // At 301 + 250 = 551ms, the pruneTile from the end of the zoom animation executes. + // It unloads the 'outside' 12 tiles from z10, but not the 4 tiles in the center, + // since _updateOpacity did not have a chance yet to flag the 16 new z11 tiles as "active". + expect(counts.tileunload).to.be(12); + // Date.now() is 601. + + // Wait for a frame to let _updateOpacity starting + // + _tileReady to be able to prepare its setTimeout(250) + // for pruning after the end of the fade-in animation. + // Since we are already > 200ms since the 'load' event fired, + // _updateOpacity should directly set the current tiles as "active", + // so the remaining 4 tiles from z10 can then be pruned. + // However we have skipped any pruning from _updateOpacity, + // so we will have to rely on the setTimeout from _tileReady. + L.Util.requestAnimFrame(function () { + // Wait > 250msec for the tile fade-in animation to complete, + // which triggers the tile pruning + clock.tick(300); + // At 851ms, the pruneTile from the end of the z11 tiles fade-in animation executes. + // It unloads the remaining 4 tiles from z10. + expect(counts.tileunload).to.be(16); + // Date.now() is 901. + done(); + }); + }); + + map.setZoom(11, {animate: true}); + // Animation (and new tiles loading) starts after 1 frame. + L.Util.requestAnimFrame(function () { + // 16 extra tiles from z11 being loaded. Total 16 + 16 = 32. + expect(counts.tileloadstart).to.be(32); + }); + + }); }); map.addLayer(grid).setView([0, 0], 10); - clock.tick(250); + // The first setView does not animated, therefore it starts loading tiles immediately. + // 16 tiles from z10 being loaded. + expect(counts.tileloadstart).to.be(16); + // At 1ms, first pruneTile (map fires "viewreset" event => GridLayer._resetView => GridLayer._setView => _pruneTiles). }); it("Loads 32, unloads 16 tiles zooming in 10-18", function (done) { From c2d12d0521f93402d9b437991aeb5a6c8495ad62 Mon Sep 17 00:00:00 2001 From: ghybs Date: Fri, 22 Jun 2018 12:15:22 +0400 Subject: [PATCH 2/3] test(GridLayer): fix graphical browser zoom out test on number of pruned tiles during zoom-out in "graphical browsers" (i.e. not PhantomJS) was failing due to incorrect interleaving of rAF and grid.once('load') event listener, not matching the current animation algorithm. --- spec/suites/layer/tile/GridLayerSpec.js | 79 ++++++++++++++++--------- 1 file changed, 52 insertions(+), 27 deletions(-) diff --git a/spec/suites/layer/tile/GridLayerSpec.js b/spec/suites/layer/tile/GridLayerSpec.js index 273c50f1..61e6b513 100644 --- a/spec/suites/layer/tile/GridLayerSpec.js +++ b/spec/suites/layer/tile/GridLayerSpec.js @@ -710,51 +710,76 @@ describe('GridLayer', function () { // browsers due to CSS animations! it.skipInPhantom("Loads 32, unloads 16 tiles zooming out 11-10", function (done) { + // Advance the time to !== 0 otherwise `tile.loaded` timestamp will appear to be falsy. + clock.tick(1); + // Date.now() is 1. + // grid.on('tileload tileunload load', logTiles); - grid.on('load', function () { - expect(counts.tileloadstart).to.be(16); + grid.once('load', function () { expect(counts.tileload).to.be(16); expect(counts.tileunload).to.be(0); - grid.off('load'); - - // grid.on('load', logTiles); - grid.on('load', function () { - - grid.off('load'); - // grid.on('load', logTiles); - - // We're one frame into the zoom animation, there are - // 16 tiles for z11 plus 4 tiles for z10 covering the - // bounds at the *beginning* of the zoom-*out* anim - expect(counts.tileloadstart).to.be(20); - expect(counts.tileload).to.be(20); - expect(counts.tileunload).to.be(0); - + // Wait for a frame to let _updateOpacity starting. + L.Util.requestAnimFrame(function () { // Wait > 250msec for the tile fade-in animation to complete, // which triggers the tile pruning clock.tick(300); - L.Util.requestAnimFrame(function () { - expect(counts.tileunload).to.be(16); + // At 251ms, the pruneTile from the end of the z11 tiles fade-in animation executes. + // Date.now() is 301. - // The next 'load' event happens when the zoom anim is - // complete, and triggers loading of all the z10 tiles. - grid.on('load', function () { - expect(counts.tileloadstart).to.be(32); + grid.once('load', function () { + expect(counts.tileload).to.be(20); + // No tile should be unloaded yet. + expect(counts.tileunload).to.be(0); + + // Wait > 250msec for the zoom animation to complete, + // which triggers the tile pruning, but there are no + // tiles to prune yet (z11 tiles are all in bounds). + clock.tick(300); + // Date.now() is 601. + + // At the end of the animation, all 16 tiles from z10 + // are loading. + expect(counts.tileloadstart).to.be(32); + expect(counts.tileload).to.be(20); + + // Now that the zoom animation is complete, + // the grid is ready to fire a new "load" event + // on next frame, so prepare its listener now. + // During that frame, _updateOpacity will flag the 4 + // central tiles from z10 as "active", since we are now + // > 200ms after the first "load" event fired. + grid.once('load', function () { expect(counts.tileload).to.be(32); - done(); + // No tile should be unloaded yet. + expect(counts.tileunload).to.be(0); + + // Wait for a frame for next _updateOpacity to prune + // all 16 tiles from z11 which are now covered by the + // 4 central active tiles of z10. + L.Util.requestAnimFrame(function () { + expect(counts.tileunload).to.be(16); + done(); + }); }); - }); }); map.setZoom(10, {animate: true}); - clock.tick(250); + // Animation (and new tiles loading) starts after 1 frame. + L.Util.requestAnimFrame(function () { + // We're one frame into the zoom animation, there are + // 16 tiles for z11 plus 4 tiles for z10 covering the + // bounds at the *beginning* of the zoom-*out* anim + expect(counts.tileloadstart).to.be(20); + }); }); map.addLayer(grid).setView([0, 0], 11); - clock.tick(250); + // The first setView does not animated, therefore it starts loading tiles immediately. + // 16 tiles from z10 being loaded. + expect(counts.tileloadstart).to.be(16); }); it("Loads 32, unloads 16 tiles zooming out 18-10", function (done) { From d557247d165cb9b821c9484654ff5a4e5a5928c9 Mon Sep 17 00:00:00 2001 From: ghybs Date: Fri, 22 Jun 2018 12:35:04 +0400 Subject: [PATCH 3/3] style(GridLayerSpec): remove trailing tabs --- spec/suites/layer/tile/GridLayerSpec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/suites/layer/tile/GridLayerSpec.js b/spec/suites/layer/tile/GridLayerSpec.js index 61e6b513..e93b4527 100644 --- a/spec/suites/layer/tile/GridLayerSpec.js +++ b/spec/suites/layer/tile/GridLayerSpec.js @@ -754,7 +754,7 @@ describe('GridLayer', function () { expect(counts.tileload).to.be(32); // No tile should be unloaded yet. expect(counts.tileunload).to.be(0); - + // Wait for a frame for next _updateOpacity to prune // all 16 tiles from z11 which are now covered by the // 4 central active tiles of z10.