From 9e68f6c27b72d611fb8991bc568a9a6223f05612 Mon Sep 17 00:00:00 2001 From: Antoine Vandecreme Date: Tue, 5 Apr 2016 17:50:18 -0400 Subject: [PATCH 1/7] Fix home bounds with clipping. Fix #891 --- src/tiledimage.js | 20 +++++++++ src/viewport.js | 90 +++++++++++++++++++++++++++++++++----- src/world.js | 44 +++++++++++-------- test/modules/tiledimage.js | 5 +-- 4 files changed, 127 insertions(+), 32 deletions(-) diff --git a/src/tiledimage.js b/src/tiledimage.js index 248f0ece..9d4db782 100644 --- a/src/tiledimage.js +++ b/src/tiledimage.js @@ -287,6 +287,26 @@ $.extend($.TiledImage.prototype, $.EventSource.prototype, /** @lends OpenSeadrag return this.getBounds(); }, + /** + * Get the bounds of the displayed part of the tiled image. + * @param {Boolean} [current=false] Pass true for the current location, + * false for the target location. + * @returns {$.Rect} The clipped bounds in viewport coordinates. + */ + getClippedBounds: function(current) { + var bounds = this.getBounds(current); + if (this._clip) { + var ratio = this._worldWidthCurrent / this.source.dimensions.x; + var clip = this._clip.times(ratio); + bounds = new $.Rect( + bounds.x + clip.x, + bounds.y + clip.y, + clip.width, + clip.height); + } + return bounds; + }, + /** * @returns {OpenSeadragon.Point} This TiledImage's content size, in original pixels. */ diff --git a/src/viewport.js b/src/viewport.js index 63b4dcaf..6f418f13 100644 --- a/src/viewport.js +++ b/src/viewport.js @@ -1085,8 +1085,18 @@ $.Viewport.prototype = { return this.viewportToImageCoordinates(viewerX.x, viewerX.y); } - if (this.viewer && this.viewer.world.getItemCount() > 1) { - $.console.error('[Viewport.viewportToImageCoordinates] is not accurate with multi-image; use TiledImage.viewportToImageCoordinates instead.'); + if (this.viewer) { + var count = this.viewer.world.getItemCount(); + if (count > 1) { + $.console.error('[Viewport.viewportToImageCoordinates] is not accurate ' + + 'with multi-image; use TiledImage.viewportToImageCoordinates instead.'); + } else if (count === 1) { + // It is better to use TiledImage.viewportToImageCoordinates + // because this._contentBoundsNoRotate can not be relied on + // with clipping. + var item = this.viewer.world.getItemAt(0); + return item.viewportToImageCoordinates(viewerX, viewerY, true); + } } return this._viewportToImageDelta( @@ -1119,8 +1129,18 @@ $.Viewport.prototype = { return this.imageToViewportCoordinates(imageX.x, imageX.y); } - if (this.viewer && this.viewer.world.getItemCount() > 1) { - $.console.error('[Viewport.imageToViewportCoordinates] is not accurate with multi-image; use TiledImage.imageToViewportCoordinates instead.'); + if (this.viewer) { + var count = this.viewer.world.getItemCount(); + if (count > 1) { + $.console.error('[Viewport.imageToViewportCoordinates] is not accurate ' + + 'with multi-image; use TiledImage.imageToViewportCoordinates instead.'); + } else if (count === 1) { + // It is better to use TiledImage.viewportToImageCoordinates + // because this._contentBoundsNoRotate can not be relied on + // with clipping. + var item = this.viewer.world.getItemAt(0); + return item.imageToViewportCoordinates(imageX, imageY, true); + } } var point = this._imageToViewportDelta(imageX, imageY); @@ -1150,6 +1170,21 @@ $.Viewport.prototype = { rect = new $.Rect(imageX, imageY, pixelWidth, pixelHeight); } + if (this.viewer) { + var count = this.viewer.world.getItemCount(); + if (count > 1) { + $.console.error('[Viewport.imageToViewportRectangle] is not accurate ' + + 'with multi-image; use TiledImage.imageToViewportRectangle instead.'); + } else if (count === 1) { + // It is better to use TiledImage.imageToViewportRectangle + // because this._contentBoundsNoRotate can not be relied on + // with clipping. + var item = this.viewer.world.getItemAt(0); + return item.imageToViewportRectangle( + imageX, imageY, pixelWidth, pixelHeight, true); + } + } + var coordA = this.imageToViewportCoordinates(rect.x, rect.y); var coordB = this._imageToViewportDelta(rect.width, rect.height); return new $.Rect( @@ -1183,6 +1218,21 @@ $.Viewport.prototype = { rect = new $.Rect(viewerX, viewerY, pointWidth, pointHeight); } + if (this.viewer) { + var count = this.viewer.world.getItemCount(); + if (count > 1) { + $.console.error('[Viewport.viewportToImageRectangle] is not accurate ' + + 'with multi-image; use TiledImage.viewportToImageRectangle instead.'); + } else if (count === 1) { + // It is better to use TiledImage.viewportToImageCoordinates + // because this._contentBoundsNoRotate can not be relied on + // with clipping. + var item = this.viewer.world.getItemAt(0); + return item.viewportToImageRectangle( + viewerX, viewerY, pointWidth, pointHeight, true); + } + } + var coordA = this.viewportToImageCoordinates(rect.x, rect.y); var coordB = this._viewportToImageDelta(rect.width, rect.height); return new $.Rect( @@ -1296,9 +1346,19 @@ $.Viewport.prototype = { * target zoom. * @returns {Number} imageZoom The image zoom */ - viewportToImageZoom: function( viewportZoom ) { - if (this.viewer && this.viewer.world.getItemCount() > 1) { - $.console.error('[Viewport.viewportToImageZoom] is not accurate with multi-image.'); + viewportToImageZoom: function(viewportZoom) { + if (this.viewer) { + var count = this.viewer.world.getItemCount(); + if (count > 1) { + $.console.error('[Viewport.viewportToImageZoom] is not ' + + 'accurate with multi-image.'); + } else if (count === 1) { + // It is better to use TiledImage.viewportToImageZoom + // because this._contentBoundsNoRotate can not be relied on + // with clipping. + var item = this.viewer.world.getItemAt(0); + return item.viewportToImageZoom(viewportZoom); + } } var imageWidth = this._contentSizeNoRotate.x; @@ -1320,9 +1380,19 @@ $.Viewport.prototype = { * target zoom. * @returns {Number} viewportZoom The viewport zoom */ - imageToViewportZoom: function( imageZoom ) { - if (this.viewer && this.viewer.world.getItemCount() > 1) { - $.console.error('[Viewport.imageToViewportZoom] is not accurate with multi-image.'); + imageToViewportZoom: function(imageZoom) { + if (this.viewer) { + var count = this.viewer.world.getItemCount(); + if (count > 1) { + $.console.error('[Viewport.imageToViewportZoom] is not accurate ' + + 'with multi-image.'); + } else if (count === 1) { + // It is better to use TiledImage.imageToViewportZoom + // because this._contentBoundsNoRotate can not be relied on + // with clipping. + var item = this.viewer.world.getItemAt(0); + return item.imageToViewportZoom(imageZoom); + } } var imageWidth = this._contentSizeNoRotate.x; diff --git a/src/world.js b/src/world.js index 5597f0f8..07e99b81 100644 --- a/src/world.js +++ b/src/world.js @@ -375,34 +375,40 @@ $.extend( $.World.prototype, $.EventSource.prototype, /** @lends OpenSeadragon.W var oldContentSize = this._contentSize ? this._contentSize.clone() : null; var oldContentFactor = this._contentFactor || 0; - if ( !this._items.length ) { + if (!this._items.length) { this._homeBounds = new $.Rect(0, 0, 1, 1); this._contentSize = new $.Point(1, 1); this._contentFactor = 1; } else { - var bounds = this._items[0].getBounds(); - this._contentFactor = this._items[0].getContentSize().x / bounds.width; - var left = bounds.x; - var top = bounds.y; - var right = bounds.x + bounds.width; - var bottom = bounds.y + bounds.height; - var box; - for ( var i = 1; i < this._items.length; i++ ) { - box = this._items[i].getBounds(); - this._contentFactor = Math.max(this._contentFactor, this._items[i].getContentSize().x / box.width); - left = Math.min( left, box.x ); - top = Math.min( top, box.y ); - right = Math.max( right, box.x + box.width ); - bottom = Math.max( bottom, box.y + box.height ); + var item = this._items[0]; + var bounds = item.getBounds(); + this._contentFactor = item.getContentSize().x / bounds.width; + var clippedBounds = item.getClippedBounds(); + var left = clippedBounds.x; + var top = clippedBounds.y; + var right = clippedBounds.x + clippedBounds.width; + var bottom = clippedBounds.y + clippedBounds.height; + for (var i = 1; i < this._items.length; i++) { + item = this._items[i]; + bounds = item.getBounds(); + this._contentFactor = Math.max(this._contentFactor, + item.getContentSize().x / bounds.width); + clippedBounds = item.getClippedBounds(); + left = Math.min(left, clippedBounds.x); + top = Math.min(top, clippedBounds.y); + right = Math.max(right, clippedBounds.x + clippedBounds.width); + bottom = Math.max(bottom, clippedBounds.y + clippedBounds.height); } - this._homeBounds = new $.Rect( left, top, right - left, bottom - top ); - this._contentSize = new $.Point(this._homeBounds.width * this._contentFactor, + this._homeBounds = new $.Rect(left, top, right - left, bottom - top); + this._contentSize = new $.Point( + this._homeBounds.width * this._contentFactor, this._homeBounds.height * this._contentFactor); } - if (this._contentFactor !== oldContentFactor || !this._homeBounds.equals(oldHomeBounds) || - !this._contentSize.equals(oldContentSize)) { + if (this._contentFactor !== oldContentFactor || + !this._homeBounds.equals(oldHomeBounds) || + !this._contentSize.equals(oldContentSize)) { /** * Raised when the home bounds or content factor change. * @event metrics-change diff --git a/test/modules/tiledimage.js b/test/modules/tiledimage.js index f281d076..405f0dfc 100644 --- a/test/modules/tiledimage.js +++ b/test/modules/tiledimage.js @@ -206,9 +206,8 @@ propEqual(image.getClip(), clip, 'clip is set correctly'); Util.spyOnce(viewer.drawer, 'setClip', function(rect) { - ok(true, 'drawer.setClip is called'); - var pixelRatio = viewer.viewport.getContainerSize().x / image.getContentSize().x; - var canvasClip = clip.times(pixelRatio * OpenSeadragon.pixelDensityRatio); + var homeBounds = viewer.viewport.getHomeBounds(); + var canvasClip = viewer.viewport.viewportToViewerElementRectangle(homeBounds); propEqual(rect, canvasClip, 'clipping to correct rect'); start(); }); From e436fc93fd2e66c1fe2687d4c41faf7a2b4fee66 Mon Sep 17 00:00:00 2001 From: Antoine Vandecreme Date: Sat, 9 Apr 2016 11:23:59 -0400 Subject: [PATCH 2/7] Fix test. --- test/modules/tiledimage.js | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/test/modules/tiledimage.js b/test/modules/tiledimage.js index 405f0dfc..49cd71d8 100644 --- a/test/modules/tiledimage.js +++ b/test/modules/tiledimage.js @@ -207,8 +207,17 @@ Util.spyOnce(viewer.drawer, 'setClip', function(rect) { var homeBounds = viewer.viewport.getHomeBounds(); - var canvasClip = viewer.viewport.viewportToViewerElementRectangle(homeBounds); - propEqual(rect, canvasClip, 'clipping to correct rect'); + var canvasClip = viewer.viewport + .viewportToViewerElementRectangle(homeBounds); + var precision = 0.00000001; + Util.assessNumericValue(rect.x, canvasClip.x, precision, + 'clipping x should be ' + canvasClip.x); + Util.assessNumericValue(rect.y, canvasClip.y, precision, + 'clipping y should be ' + canvasClip.y); + Util.assessNumericValue(rect.width, canvasClip.width, precision, + 'clipping width should be ' + canvasClip.width); + Util.assessNumericValue(rect.height, canvasClip.height, precision, + 'clipping height should be ' + canvasClip.height); start(); }); }); From 686176f82159e44b4496b2d2f48f5d2c4957dab8 Mon Sep 17 00:00:00 2001 From: Antoine Vandecreme Date: Sat, 9 Apr 2016 11:37:05 -0400 Subject: [PATCH 3/7] Add TiledImage.getClippedBounds test. --- test/modules/tiledimage.js | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/test/modules/tiledimage.js b/test/modules/tiledimage.js index 49cd71d8..614d28fd 100644 --- a/test/modules/tiledimage.js +++ b/test/modules/tiledimage.js @@ -228,6 +228,39 @@ }); }); + asyncTest('getClipBounds', function() { + var clip = new OpenSeadragon.Rect(100, 200, 800, 500); + + viewer.addHandler('open', function() { + var image = viewer.world.getItemAt(0); + var bounds = image.getClippedBounds(); + var expectedBounds = new OpenSeadragon.Rect(1.2, 1.4, 1.6, 1); + propEqual(bounds, expectedBounds, + 'getClipBounds should take clipping into account.'); + + image = viewer.world.getItemAt(1); + bounds = image.getClippedBounds(); + expectedBounds = new OpenSeadragon.Rect(1, 2, 2, 2); + propEqual(bounds, expectedBounds, + 'getClipBounds should work when no clipping set.'); + + start(); + }); + + viewer.open([{ + tileSource: '/test/data/testpattern.dzi', + clip: clip, + x: 1, + y: 1, + width: 2 + }, { + tileSource: '/test/data/testpattern.dzi', + x: 1, + y: 2, + width: 2 + }]); + }); + // ---------- asyncTest('opacity', function() { From b8c87ddb61ea0943bc9a54c8019bc53cbcaa2db4 Mon Sep 17 00:00:00 2001 From: Antoine Vandecreme Date: Sat, 9 Apr 2016 11:56:34 -0400 Subject: [PATCH 4/7] Use Util.assertRectangleEquals --- test/modules/tiledimage.js | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/test/modules/tiledimage.js b/test/modules/tiledimage.js index 614d28fd..d7a46aef 100644 --- a/test/modules/tiledimage.js +++ b/test/modules/tiledimage.js @@ -210,14 +210,8 @@ var canvasClip = viewer.viewport .viewportToViewerElementRectangle(homeBounds); var precision = 0.00000001; - Util.assessNumericValue(rect.x, canvasClip.x, precision, - 'clipping x should be ' + canvasClip.x); - Util.assessNumericValue(rect.y, canvasClip.y, precision, - 'clipping y should be ' + canvasClip.y); - Util.assessNumericValue(rect.width, canvasClip.width, precision, - 'clipping width should be ' + canvasClip.width); - Util.assessNumericValue(rect.height, canvasClip.height, precision, - 'clipping height should be ' + canvasClip.height); + Util.assertRectangleEquals(rect, canvasClip, precision, + 'clipping should be ' + canvasClip); start(); }); }); From e0e6ce9b65662528fb743c8edc4adf0ce751599c Mon Sep 17 00:00:00 2001 From: Antoine Vandecreme Date: Sat, 9 Apr 2016 18:13:37 -0400 Subject: [PATCH 5/7] Add unit tests for home bounds with clip. --- test/modules/viewport.js | 51 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/test/modules/viewport.js b/test/modules/viewport.js index 70e980ed..0b3ca020 100644 --- a/test/modules/viewport.js +++ b/test/modules/viewport.js @@ -238,6 +238,57 @@ viewer.open(DZI_PATH); }); + asyncTest('getHomeBoundsWithMultiImages', function() { + function openHandler() { + viewer.removeHandler('open', openHandler); + var viewport = viewer.viewport; + Util.assertRectangleEquals( + new OpenSeadragon.Rect(0, 0, 4, 4), + viewport.getHomeBounds(), + 0.00000001, + "Test getHomeBoundsWithMultiImages"); + start(); + } + viewer.addHandler('open', openHandler); + viewer.open([{ + tileSource: DZI_PATH, + x: 0, + y: 0, + width: 2 + }, { + tileSource: DZI_PATH, + x: 3, + y: 3, + width: 1 + }]); + }); + + asyncTest('getHomeBoundsWithMultiImagesAndClipping', function() { + function openHandler() { + viewer.removeHandler('open', openHandler); + var viewport = viewer.viewport; + Util.assertRectangleEquals( + new OpenSeadragon.Rect(1, 1, 4, 4), + viewport.getHomeBounds(), + 0.00000001, + "Test getHomeBoundsWithMultiImagesAndClipping"); + start(); + } + viewer.addHandler('open', openHandler); + viewer.open([{ + tileSource: DZI_PATH, + x: 0, + y: 0, + width: 2, + clip: new OpenSeadragon.Rect(500, 500, 500, 500) + }, { + tileSource: DZI_PATH, + x: 4, + y: 4, + width: 1 + }]); + }); + asyncTest('getHomeZoom', function() { reopenViewerHelper({ property: 'defaultZoomLevel', From b1a0abd1041682a9d852c48139e63981f3da2aa5 Mon Sep 17 00:00:00 2001 From: Antoine Vandecreme Date: Tue, 19 Apr 2016 18:13:12 -0400 Subject: [PATCH 6/7] Add this.viewer test. --- src/viewport.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/viewport.js b/src/viewport.js index a2a9cc26..5095a9d5 100644 --- a/src/viewport.js +++ b/src/viewport.js @@ -341,7 +341,9 @@ $.Viewport.prototype = { }, margins); this._updateContainerInnerSize(); - this.viewer.forceRedraw(); + if (this.viewer) { + this.viewer.forceRedraw(); + } }, /** From 65a95d4a4988a33f9cb6f27cee5bdabec4cc4afb Mon Sep 17 00:00:00 2001 From: Antoine Vandecreme Date: Thu, 21 Apr 2016 10:57:39 -0400 Subject: [PATCH 7/7] Add asserts on this.viewer. --- src/viewport.js | 36 ++++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/src/viewport.js b/src/viewport.js index 5095a9d5..5a70de16 100644 --- a/src/viewport.js +++ b/src/viewport.js @@ -1134,7 +1134,7 @@ $.Viewport.prototype = { if (this.viewer) { var count = this.viewer.world.getItemCount(); if (count > 1) { - $.console.error('[Viewport.imageToViewportCoordinates] is not accurate ' + + $.console.error('[Viewport.imageToViewportCoordinates] is not accurate ' + 'with multi-image; use TiledImage.imageToViewportCoordinates instead.'); } else if (count === 1) { // It is better to use TiledImage.viewportToImageCoordinates @@ -1276,10 +1276,12 @@ $.Viewport.prototype = { * @param {OpenSeadragon.Point} pixel * @returns {OpenSeadragon.Point} */ - windowToImageCoordinates: function( pixel ) { + windowToImageCoordinates: function(pixel) { + $.console.assert(this.viewer, + "[Viewport.windowToImageCoordinates] the viewport must have a viewer."); var viewerCoordinates = pixel.minus( - OpenSeadragon.getElementPosition( this.viewer.element )); - return this.viewerElementToImageCoordinates( viewerCoordinates ); + $.getElementPosition(this.viewer.element)); + return this.viewerElementToImageCoordinates(viewerCoordinates); }, /** @@ -1288,10 +1290,12 @@ $.Viewport.prototype = { * @param {OpenSeadragon.Point} pixel * @returns {OpenSeadragon.Point} */ - imageToWindowCoordinates: function( pixel ) { - var viewerCoordinates = this.imageToViewerElementCoordinates( pixel ); + imageToWindowCoordinates: function(pixel) { + $.console.assert(this.viewer, + "[Viewport.imageToWindowCoordinates] the viewport must have a viewer."); + var viewerCoordinates = this.imageToViewerElementCoordinates(pixel); return viewerCoordinates.plus( - OpenSeadragon.getElementPosition( this.viewer.element )); + $.getElementPosition(this.viewer.element)); }, /** @@ -1347,10 +1351,12 @@ $.Viewport.prototype = { * @param {OpenSeadragon.Point} pixel * @returns {OpenSeadragon.Point} */ - windowToViewportCoordinates: function( pixel ) { + windowToViewportCoordinates: function(pixel) { + $.console.assert(this.viewer, + "[Viewport.windowToViewportCoordinates] the viewport must have a viewer."); var viewerCoordinates = pixel.minus( - OpenSeadragon.getElementPosition( this.viewer.element )); - return this.viewerElementToViewportCoordinates( viewerCoordinates ); + $.getElementPosition(this.viewer.element)); + return this.viewerElementToViewportCoordinates(viewerCoordinates); }, /** @@ -1358,10 +1364,12 @@ $.Viewport.prototype = { * @param {OpenSeadragon.Point} point * @returns {OpenSeadragon.Point} */ - viewportToWindowCoordinates: function( point ) { - var viewerCoordinates = this.viewportToViewerElementCoordinates( point ); + viewportToWindowCoordinates: function(point) { + $.console.assert(this.viewer, + "[Viewport.viewportToWindowCoordinates] the viewport must have a viewer."); + var viewerCoordinates = this.viewportToViewerElementCoordinates(point); return viewerCoordinates.plus( - OpenSeadragon.getElementPosition( this.viewer.element )); + $.getElementPosition(this.viewer.element)); }, /** @@ -1380,7 +1388,7 @@ $.Viewport.prototype = { if (this.viewer) { var count = this.viewer.world.getItemCount(); if (count > 1) { - $.console.error('[Viewport.viewportToImageZoom] is not ' + + $.console.error('[Viewport.viewportToImageZoom] is not ' + 'accurate with multi-image.'); } else if (count === 1) { // It is better to use TiledImage.viewportToImageZoom