diff --git a/changelog.txt b/changelog.txt index 2e4d94a3..46f935b4 100644 --- a/changelog.txt +++ b/changelog.txt @@ -4,6 +4,7 @@ OPENSEADRAGON CHANGELOG 0.9.129: (In Progress) * Fixed an error when using navPrevNextWrap on single images (#135) +* Various fixes to our timer handling (#133) 0.9.128: diff --git a/src/openseadragon.js b/src/openseadragon.js index dad43393..5b7b52a2 100644 --- a/src/openseadragon.js +++ b/src/openseadragon.js @@ -1689,6 +1689,7 @@ window.OpenSeadragon = window.OpenSeadragon || function( options ){ }; } else { var aAnimQueue = [], + processing = [], iRequestId = 0, iIntervalId; @@ -1699,7 +1700,18 @@ window.OpenSeadragon = window.OpenSeadragon || function( options ){ if ( !iIntervalId ) { iIntervalId = setInterval( function() { if ( aAnimQueue.length ) { - aAnimQueue.shift( )[ 1 ](+new Date()); + var time = new Date().getTime(); + // Process all of the currently outstanding frame + // requests, but none that get added during the + // processing. + // Swap the arrays so we don't have to create a new + // array every frame. + var temp = processing; + processing = aAnimQueue; + aAnimQueue = temp; + while ( processing.length ) { + processing.shift()[ 1 ]( time ); + } } else { // don't continue the interval, if unnecessary clearInterval( iIntervalId ); @@ -1714,12 +1726,23 @@ window.OpenSeadragon = window.OpenSeadragon || function( options ){ // create a mock cancelAnimationFrame function $.cancelAnimationFrame = function( requestId ) { // find the request ID and remove it - for ( var i = 0, j = aAnimQueue.length; i < j; i += 1 ) { + var i, j; + for ( i = 0, j = aAnimQueue.length; i < j; i += 1 ) { if ( aAnimQueue[ i ][ 0 ] === requestId ) { aAnimQueue.splice( i, 1 ); return; } } + + // If it's not in the queue, it may be in the set we're currently + // processing (if cancelAnimationFrame is called from within a + // requestAnimationFrame callback). + for ( i = 0, j = processing.length; i < j; i += 1 ) { + if ( processing[ i ][ 0 ] === requestId ) { + processing.splice( i, 1 ); + return; + } + } }; } })( window ); diff --git a/src/viewer.js b/src/viewer.js index 75b4e33b..47cb9072 100644 --- a/src/viewer.js +++ b/src/viewer.js @@ -151,7 +151,6 @@ $.Viewer = function( options ) { THIS[ this.hash ] = { "fsBoundsDelta": new $.Point( 1, 1 ), "prevContainerSize": null, - "updateRequestId": null, "animating": false, "forceRedraw": false, "mouseInside": false, @@ -168,6 +167,8 @@ $.Viewer = function( options ) { "onfullscreenchange": null }; + this._updateRequestId = null; + //Inherit some behaviors and properties $.EventHandler.call( this ); $.ControlDock.call( this, options ); @@ -461,12 +462,16 @@ $.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._updateRequestId !== null ) { + $.cancelAnimationFrame( this._updateRequestId ); + this._updateRequestId = null; } - if( this.drawer ){ + if ( this.navigator ) { + this.navigator.close(); + } + + if ( this.drawer ) { this.drawer.clearOverlays(); } @@ -1147,22 +1152,25 @@ function openTileSource( viewer, source ) { }); //Instantiate a navigator if configured - if ( _this.showNavigator && ! _this.navigator && !_this.collectionMode ){ - _this.navigator = new $.Navigator({ - id: _this.navigatorId, - position: _this.navigatorPosition, - sizeRatio: _this.navigatorSizeRatio, - height: _this.navigatorHeight, - width: _this.navigatorWidth, - // By passing the fully parsed source here, the navigator doesn't - // have to load it again. Additionally, we don't have to call - // navigator.open, as it's implicitly called in the ctor. - tileSources: source, - tileHost: _this.tileHost, - prefixUrl: _this.prefixUrl, - overlays: _this.overlays, - viewer: _this - }); + if ( _this.showNavigator && !_this.collectionMode ){ + // Note: By passing the fully parsed source, the navigator doesn't + // have to load it again. + if ( _this.navigator ) { + _this.navigator.open( source ); + } else { + _this.navigator = new $.Navigator({ + id: _this.navigatorId, + position: _this.navigatorPosition, + sizeRatio: _this.navigatorSizeRatio, + height: _this.navigatorHeight, + width: _this.navigatorWidth, + tileSources: source, + tileHost: _this.tileHost, + prefixUrl: _this.prefixUrl, + overlays: _this.overlays, + viewer: _this + }); + } } //Instantiate a referencestrip if configured @@ -1186,7 +1194,7 @@ function openTileSource( viewer, source ) { THIS[ _this.hash ].animating = false; THIS[ _this.hash ].forceRedraw = true; - THIS[ _this.hash ].updateRequestId = scheduleUpdate( _this, updateMulti ); + _this._updateRequestId = scheduleUpdate( _this, updateMulti ); //Assuming you had programatically created a bunch of overlays //and added them via configuration @@ -1233,23 +1241,7 @@ function openTileSource( viewer, source ) { /////////////////////////////////////////////////////////////////////////////// // Schedulers provide the general engine for animation /////////////////////////////////////////////////////////////////////////////// -function scheduleUpdate( viewer, updateFunc, prevUpdateTime ){ - var currentTime, - targetTime, - deltaTime; - - if ( THIS[ viewer.hash ].animating ) { - return $.requestAnimationFrame( function(){ - updateFunc( viewer ); - } ); - } - - currentTime = +new Date(); - prevUpdateTime = prevUpdateTime ? prevUpdateTime : currentTime; - // 60 frames per second is ideal - targetTime = prevUpdateTime + 1000 / 60; - deltaTime = Math.max( 1, targetTime - currentTime ); - +function scheduleUpdate( viewer, updateFunc ){ return $.requestAnimationFrame( function(){ updateFunc( viewer ); } ); @@ -1455,19 +1447,17 @@ function onContainerEnter( tracker, position, buttonDownElement, buttonDownAny ) /////////////////////////////////////////////////////////////////////////////// function updateMulti( viewer ) { - - var beginTime; - if ( !viewer.source ) { - THIS[ viewer.hash ].updateRequestId = null; + viewer._updateRequestId = null; return; } - beginTime = +new Date(); updateOnce( viewer ); - THIS[ viewer.hash ].updateRequestId = scheduleUpdate( viewer, - arguments.callee, beginTime ); + // Request the next frame, unless we've been closed during the updateOnce() + if ( viewer.source ) { + viewer._updateRequestId = scheduleUpdate( viewer, updateMulti ); + } } function updateOnce( viewer ) { diff --git a/test/basic.js b/test/basic.js index 786938bb..0d13e844 100644 --- a/test/basic.js +++ b/test/basic.js @@ -13,8 +13,7 @@ id: 'example', prefixUrl: '/build/openseadragon/images/', tileSources: '/test/data/testpattern.dzi', - springStiffness: 100, // Faster animation = faster tests - showNavigator: true + springStiffness: 100 // Faster animation = faster tests }); ok(viewer, 'Viewer exists'); @@ -25,6 +24,8 @@ equal(eventSender, viewer, 'Sender of open event was viewer'); ok(eventData, 'Handler also received event data'); ok(viewer.viewport, 'Viewport exists'); + ok(viewer.source, 'source exists'); + ok(viewer._updateRequestId, 'timer is on'); start(); }; @@ -123,9 +124,14 @@ asyncTest('Close', function() { var closeHandler = function() { viewer.removeHandler('close', closeHandler); + ok(!viewer.source, 'no source'); $('#example').empty(); ok(true, 'Close event was sent'); - start(); + ok(!viewer._updateRequestId, 'timer is off'); + setTimeout(function() { + ok(!viewer._updateRequestId, 'timer is still off'); + start(); + }, 100); }; viewer.addHandler('close', closeHandler); diff --git a/test/navigator.js b/test/navigator.js index bb4c06e3..547e39a3 100644 --- a/test/navigator.js +++ b/test/navigator.js @@ -461,4 +461,59 @@ QUnit.config.autostart = false; }); }); + asyncTest('Viewer closing one image and opening another', function() { + var timeWatcher = Util.timeWatcher(); + + viewer = OpenSeadragon({ + id: 'example', + prefixUrl: '/build/openseadragon/images/', + tileSources: '/test/data/testpattern.dzi', + springStiffness: 100, // Faster animation = faster tests + showNavigator: true + }); + + var openHandler1 = function(eventSender, eventData) { + viewer.removeHandler('open', openHandler1); + ok(viewer.navigator, 'navigator exists'); + viewer.navigator.addHandler('open', navOpenHandler1); + }; + + var navOpenHandler1 = function(eventSender, eventData) { + viewer.navigator.removeHandler('open', navOpenHandler1); + equal(viewer.navigator.source, viewer.source, 'viewer and navigator have the same source'); + ok(viewer.navigator._updateRequestId, 'navigator timer is on'); + viewer.addHandler('close', closeHandler1); + viewer.addHandler('open', openHandler2); + viewer.open('/test/data/tall.dzi'); + }; + + var closeHandler1 = function() { + viewer.removeHandler('close', closeHandler1); + ok(true, 'calling open closes the old one'); + equal(viewer.navigator.source, null, 'navigator source has been cleared'); + }; + + var openHandler2 = function(eventSender, eventData) { + viewer.removeHandler('open', openHandler2); + viewer.navigator.addHandler('open', navOpenHandler2); + }; + + var navOpenHandler2 = function(eventSender, eventData) { + viewer.navigator.removeHandler('open', navOpenHandler2); + equal(viewer.navigator.source, viewer.source, 'viewer and navigator have the same source'); + viewer.addHandler('close', closeHandler2); + viewer.close(); + }; + + var closeHandler2 = function() { + viewer.removeHandler('close', closeHandler2); + ok(!viewer.navigator._updateRequestId, 'navigator timer is off'); + setTimeout(function() { + ok(!viewer.navigator._updateRequestId, 'navigator timer is still off'); + timeWatcher.done(); + }, 100); + }; + + viewer.addHandler('open', openHandler1); + }); })();