Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix error when opening a tiled image - highest zoom level could not be determined #2493

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

stefanheinze
Copy link

In our configuration while loading a tiled image the following error was thrown. This happen when OSD was loading the first tiled image.

global-error-handler.service.ts:15 RangeError: Invalid array length
    at __webpack_modules__.7446.$.TiledImage._updateLevelsForViewport (openseadragon.js:25539:1)
    at __webpack_modules__.7446.$.TiledImage.update (openseadragon.js:24507:1)
    at __webpack_modules__.7446.$.World.update (openseadragon.js:27064:1)
    at updateOnce (openseadragon.js:11711:1)
    at updateMulti (openseadragon.js:11655:1)
    at openseadragon.js:10687:1

Reason for that is that the highest level could not be determinded (its NaN) in _updateLevelsForViewport that is calculated in _getLevelsInterval.

this.source

{
    "_levelRects": {},
    "tilesUrl": "/api/service/curie/v1/tile/5fce4ecc5a7633115825923d/dzi_files/",
    "fileFormat": "png",
    "displayRects": [],
    "events": {},
    "_rejectedEventList": {},
    "width": 3916,
    "height": 3918,
    "tileOverlap": 0,
    "minLevel": 0,
    "maxLevel": 12,
    "Image": {
        "xmlns": "http://schemas.microsoft.com/deepzoom/2008",
        "Url": null,
        "Format": "png",
        "DisplayRect": null,
        "Overlap": 0,
        "TileSize": 256,
        "Size": {
            "Height": 3918,
            "Width": 3916
        }
    },
    "queryParams": "",
    "ajaxWithCredentials": false,
    "ready": true,
    "aspectRatio": 0.9994895354772844,
    "dimensions": {
        "x": 3916,
        "y": 3918
    },
    "_tileHeight": 256,
    "_tileWidth": 256
}

currentZeroRatio -564.8212461695608
this.minPixelRatio 0.5

@iangilman
Copy link
Member

The change seems like a decent bit of defensive code, but before we go with it I'd like to know a little more about why this is happening. It looks like highestLevel is NaN because it's doing a Math.log of a negative number (currentZeroRatio). Looking at the code, it seems currentZeroRatio should never be negative. Could you please log the numbers that go into its calculation?

Also, with your fix, does everything seem to be working properly? Do you see any side effects?

My concern is something else is going wrong that's causing currentZeroRatio to be negative, and that we should fix the source of the problem.

Thank you for doing this!

@pearcetm
Copy link
Contributor

Looking at the code, it seems currentZeroRatio should never be negative.

It looks like this could potentially happen if a negative number was given to one of setWidth/setHeight/setScale (or a negative width was passed in the options for this TiledImage). Perhaps it would be better to guard against negative numbers there. I suspect that allowing negative scaling could mess up the tile positioning and drawing code. setRotation(180) should give equivalent results to making the scale negative, I think?

@stefanheinze
Copy link
Author

i agree on your both comments.

i investigated a bit more. Turn out that we are using the OSD api maybe a bit wrong. After opening an image we want to have the image displayed in fit to size, thats like the user is pressing the home button. As far as i can remember we had in the past some issues that this was not always working. Therefore we did the following:

      this.imageViewer.viewport.goHome(true);
      this.imageViewer.viewport.zoomBy(0 - this.imageViewer.zoomPerScroll); // zoomPerScroll = 1.2

The result is at the end a negativ zoom level and therefore the highest level in NaN. I am currently on it to run all our automated tests and check manually to check whether we can remove the zoomBy call.

But maybe its also good to prevent the usage of zoomBy if the result is a negative value.

What do you think about it? Does it make sense? @iangilman @pearcetm

@pearcetm
Copy link
Contributor

I agree that passing a negative number to zoomBy is the likely culprit here; instead of zoomBy(0 - this.imageViewer.zoomPerScroll) you probably want zoomBy(1 / this.imageViewer.zoomPerScroll).

Another question is why your image isn't filling the viewer after you call goHome(true). This could happen if the viewer is resized immediately after goHome - if all of this happens right at initialization or on image load, you might try putting that call in a timeout: setTimeout(()=>this.imageViewer.viewport.goHome(true), 0).

@iangilman
Copy link
Member

Yeah, the open call is asynchronous, so you need to wait until it's done before doing goHome. Typically I do viewer.addHandler('open' ... to take care of that.

And yeah, some guards against making the zoom negative (or even 0) in zoomBy and zoomTo (ideally including an error message) could be good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants