From b315b148c238a3a5944f5409ef351625dc56f578 Mon Sep 17 00:00:00 2001 From: Benjamin Gilbert Date: Wed, 24 Apr 2013 23:57:35 -0400 Subject: [PATCH 1/6] Drop remains of code to show "Loading..." message It calls a method that no longer exists, but only when an impossible condition is true. --- src/viewer.js | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/viewer.js b/src/viewer.js index b6026148..f0b698b8 100644 --- a/src/viewer.js +++ b/src/viewer.js @@ -115,8 +115,6 @@ $.Viewer = function( options ) { THIS[ this.hash ] = { "fsBoundsDelta": new $.Point( 1, 1 ), "prevContainerSize": null, - "lastOpenStartTime": 0, - "lastOpenEndTime": 0, "animating": false, "forceRedraw": false, "mouseInside": false, @@ -956,16 +954,6 @@ function openTileSource( viewer, source ) { _this.close( ); } - // to ignore earlier opens - THIS[ _this.hash ].lastOpenStartTime = +new Date(); - - window.setTimeout( function () { - if ( THIS[ _this.hash ].lastOpenStartTime > THIS[ _this.hash ].lastOpenEndTime ) { - THIS[ _this.hash ].setMessage( $.getString( "Messages.Loading" ) ); - } - }, 2000); - - THIS[ _this.hash ].lastOpenEndTime = +new Date(); _this.canvas.innerHTML = ""; THIS[ _this.hash ].prevContainerSize = $.getElementSize( _this.container ); From 9eb19d6c9c7c1e4e037162db5332a8fc2acb2a18 Mon Sep 17 00:00:00 2001 From: Benjamin Gilbert Date: Thu, 25 Apr 2013 00:23:02 -0400 Subject: [PATCH 2/6] Garbage-collect strings --- src/strings.js | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/strings.js b/src/strings.js index 16f2df8a..4d98c1e2 100644 --- a/src/strings.js +++ b/src/strings.js @@ -6,21 +6,13 @@ // pythons gettext might be a reasonable approach. var I18N = { Errors: { - Failure: "Sorry, but Seadragon Ajax can't run on your browser!\n" + - "Please try using IE 7 or Firefox 3.\n", Dzc: "Sorry, we don't support Deep Zoom Collections!", Dzi: "Hmm, this doesn't appear to be a valid Deep Zoom Image.", Xml: "Hmm, this doesn't appear to be a valid Deep Zoom Image.", - Empty: "You asked us to open nothing, so we did just that.", ImageFormat: "Sorry, we don't support {0}-based Deep Zoom Images.", Security: "It looks like a security restriction stopped us from " + "loading this Deep Zoom Image.", - Status: "This space unintentionally left blank ({0} {1}).", - Unknown: "Whoops, something inexplicably went wrong. Sorry!" - }, - - Messages: { - Loading: "Loading..." + Status: "This space unintentionally left blank ({0} {1})." }, Tooltips: { From 47aba609403d090b04b144b1e0097e0ea969f7a8 Mon Sep 17 00:00:00 2001 From: Benjamin Gilbert Date: Thu, 25 Apr 2013 00:37:36 -0400 Subject: [PATCH 3/6] Fix timer leak after multiple Viewer.open() calls Keep a counter of Viewer.close() calls in private state and a copy in the timer's closure, and stop the timer when they no longer match. Fixes #76. --- src/viewer.js | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/src/viewer.js b/src/viewer.js index f0b698b8..bd907f1b 100644 --- a/src/viewer.js +++ b/src/viewer.js @@ -115,6 +115,7 @@ $.Viewer = function( options ) { THIS[ this.hash ] = { "fsBoundsDelta": new $.Point( 1, 1 ), "prevContainerSize": null, + "closeCount": 0, "animating": false, "forceRedraw": false, "mouseInside": false, @@ -368,6 +369,8 @@ $.extend( $.Viewer.prototype, $.EventHandler.prototype, $.ControlDock.prototype, VIEWERS[ this.hash ] = null; delete VIEWERS[ this.hash ]; + THIS[ this.hash ].closeCount++; + this.raiseEvent( 'close', { viewer: this } ); return this; @@ -948,6 +951,7 @@ $.extend( $.Viewer.prototype, $.EventHandler.prototype, $.ControlDock.prototype, function openTileSource( viewer, source ) { var _this = viewer, overlay, + closeCount, i; if ( _this.source ) { @@ -1064,7 +1068,12 @@ function openTileSource( viewer, source ) { THIS[ _this.hash ].animating = false; THIS[ _this.hash ].forceRedraw = true; - scheduleUpdate( _this, updateMulti ); + + // Use local copy in closure + closeCount = THIS[ _this.hash ].closeCount; + scheduleUpdate( _this, function(){ + updateMulti( _this, closeCount ); + } ); //Assuming you had programatically created a bunch of overlays //and added them via configuration @@ -1121,9 +1130,7 @@ function scheduleUpdate( viewer, updateFunc, prevUpdateTime ){ deltaTime; if ( THIS[ viewer.hash ].animating ) { - return $.requestAnimationFrame( function(){ - updateFunc( viewer ); - } ); + return $.requestAnimationFrame( updateFunc ); } currentTime = +new Date(); @@ -1132,9 +1139,7 @@ function scheduleUpdate( viewer, updateFunc, prevUpdateTime ){ targetTime = prevUpdateTime + 1000 / 60; deltaTime = Math.max( 1, targetTime - currentTime ); - return $.requestAnimationFrame( function(){ - updateFunc( viewer ); - } ); + return $.requestAnimationFrame( updateFunc ); } @@ -1334,17 +1339,19 @@ function onContainerEnter( tracker, position, buttonDownElement, buttonDownAny ) // Page update routines ( aka Views - for future reference ) /////////////////////////////////////////////////////////////////////////////// -function updateMulti( viewer ) { +function updateMulti( viewer, closeCount ) { var beginTime; - if ( !viewer.source ) { + if ( closeCount !== THIS[ viewer.hash ].closeCount ) { return; } beginTime = +new Date(); updateOnce( viewer ); - scheduleUpdate( viewer, arguments.callee, beginTime ); + scheduleUpdate( viewer, function(){ + updateMulti( viewer, closeCount ); + }, beginTime ); } function updateOnce( viewer ) { From 23c20e3d5aaa147bdb50b2a358f453d1e3496d27 Mon Sep 17 00:00:00 2001 From: Benjamin Gilbert Date: Fri, 26 Apr 2013 21:48:48 -0400 Subject: [PATCH 4/6] Revert "Fix timer leak after multiple Viewer.open() calls" This should be done with cancelAnimationFrame() instead. This reverts commit 47aba609403d090b04b144b1e0097e0ea969f7a8. --- src/viewer.js | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/src/viewer.js b/src/viewer.js index bd907f1b..f0b698b8 100644 --- a/src/viewer.js +++ b/src/viewer.js @@ -115,7 +115,6 @@ $.Viewer = function( options ) { THIS[ this.hash ] = { "fsBoundsDelta": new $.Point( 1, 1 ), "prevContainerSize": null, - "closeCount": 0, "animating": false, "forceRedraw": false, "mouseInside": false, @@ -369,8 +368,6 @@ $.extend( $.Viewer.prototype, $.EventHandler.prototype, $.ControlDock.prototype, VIEWERS[ this.hash ] = null; delete VIEWERS[ this.hash ]; - THIS[ this.hash ].closeCount++; - this.raiseEvent( 'close', { viewer: this } ); return this; @@ -951,7 +948,6 @@ $.extend( $.Viewer.prototype, $.EventHandler.prototype, $.ControlDock.prototype, function openTileSource( viewer, source ) { var _this = viewer, overlay, - closeCount, i; if ( _this.source ) { @@ -1068,12 +1064,7 @@ function openTileSource( viewer, source ) { THIS[ _this.hash ].animating = false; THIS[ _this.hash ].forceRedraw = true; - - // Use local copy in closure - closeCount = THIS[ _this.hash ].closeCount; - scheduleUpdate( _this, function(){ - updateMulti( _this, closeCount ); - } ); + scheduleUpdate( _this, updateMulti ); //Assuming you had programatically created a bunch of overlays //and added them via configuration @@ -1130,7 +1121,9 @@ function scheduleUpdate( viewer, updateFunc, prevUpdateTime ){ deltaTime; if ( THIS[ viewer.hash ].animating ) { - return $.requestAnimationFrame( updateFunc ); + return $.requestAnimationFrame( function(){ + updateFunc( viewer ); + } ); } currentTime = +new Date(); @@ -1139,7 +1132,9 @@ function scheduleUpdate( viewer, updateFunc, prevUpdateTime ){ targetTime = prevUpdateTime + 1000 / 60; deltaTime = Math.max( 1, targetTime - currentTime ); - return $.requestAnimationFrame( updateFunc ); + return $.requestAnimationFrame( function(){ + updateFunc( viewer ); + } ); } @@ -1339,19 +1334,17 @@ function onContainerEnter( tracker, position, buttonDownElement, buttonDownAny ) // Page update routines ( aka Views - for future reference ) /////////////////////////////////////////////////////////////////////////////// -function updateMulti( viewer, closeCount ) { +function updateMulti( viewer ) { var beginTime; - if ( closeCount !== THIS[ viewer.hash ].closeCount ) { + if ( !viewer.source ) { return; } beginTime = +new Date(); updateOnce( viewer ); - scheduleUpdate( viewer, function(){ - updateMulti( viewer, closeCount ); - }, beginTime ); + scheduleUpdate( viewer, arguments.callee, beginTime ); } function updateOnce( viewer ) { From 8cb2714daea991c78823afc1bb1899e22f36c2b8 Mon Sep 17 00:00:00 2001 From: Benjamin Gilbert Date: Fri, 26 Apr 2013 22:32:51 -0400 Subject: [PATCH 5/6] Fix timer leak after multiple Viewer.open() calls, take 2 Track the request ID of the outstanding animation frame and cancel it on Viewer.close(). Fixes #76. --- src/viewer.js | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/viewer.js b/src/viewer.js index f0b698b8..af4b1822 100644 --- a/src/viewer.js +++ b/src/viewer.js @@ -115,6 +115,7 @@ $.Viewer = function( options ) { THIS[ this.hash ] = { "fsBoundsDelta": new $.Point( 1, 1 ), "prevContainerSize": null, + "updateRequestId": null, "animating": false, "forceRedraw": false, "mouseInside": false, @@ -353,7 +354,11 @@ $.extend( $.Viewer.prototype, $.EventHandler.prototype, $.ControlDock.prototype, * @return {OpenSeadragon.Viewer} Chainable. */ close: function ( ) { - + if ( THIS[ this.hash ].updateRequestId !== null ){ + $.cancelAnimationFrame( THIS[ this.hash ].updateRequestId ); + THIS[ this.hash ].updateRequestId = null; + } + if( this.drawer ){ this.drawer.clearOverlays(); } @@ -1064,7 +1069,7 @@ function openTileSource( viewer, source ) { THIS[ _this.hash ].animating = false; THIS[ _this.hash ].forceRedraw = true; - scheduleUpdate( _this, updateMulti ); + THIS[ _this.hash ].updateRequestId = scheduleUpdate( _this, updateMulti ); //Assuming you had programatically created a bunch of overlays //and added them via configuration @@ -1338,13 +1343,11 @@ function updateMulti( viewer ) { var beginTime; - if ( !viewer.source ) { - return; - } - beginTime = +new Date(); updateOnce( viewer ); - scheduleUpdate( viewer, arguments.callee, beginTime ); + + THIS[ viewer.hash ].updateRequestId = scheduleUpdate( viewer, + arguments.callee, beginTime ); } function updateOnce( viewer ) { From 3652c7066c5521aa1381f1982cbf3dacb0fb29aa Mon Sep 17 00:00:00 2001 From: Benjamin Gilbert Date: Mon, 29 Apr 2013 13:17:37 -0400 Subject: [PATCH 6/6] Re-add viewer.source check in updateMulti() It should be redundant in normal operation, but may prevent a timer leak in case of a bug in the open/close path. --- src/viewer.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/viewer.js b/src/viewer.js index af4b1822..a47d46d0 100644 --- a/src/viewer.js +++ b/src/viewer.js @@ -1343,6 +1343,11 @@ function updateMulti( viewer ) { var beginTime; + if ( !viewer.source ) { + THIS[ viewer.hash ].updateRequestId = null; + return; + } + beginTime = +new Date(); updateOnce( viewer );