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

[=] Wrap fix for #555 #987

Merged
merged 4 commits into from Jul 29, 2016
Merged

[=] Wrap fix for #555 #987

merged 4 commits into from Jul 29, 2016

Conversation

VoidVolker
Copy link
Contributor

@VoidVolker VoidVolker commented Jul 28, 2016

  1. Fix for horizontal and vertical wrap. Problem was in
    getTileAtPoint: it was working only for points inside viewer - thanks
    to @avandecreme for finding this.
  2. Was small bug in not rendering top row and left column - after scroll
    there are empty space and need some time for rendering.

Issue #555

1. Fix for horizontal and vertical wrap. Problem was in
`getTileAtPoint`: it was working only for points inside viewer - thanks
to @avandecreme for finding this.
2. Was small bug in not rendering top row and left column - after scroll
there are empty space and need some time for rendering.
@iangilman
Copy link
Member

I've kicked it around a bit and it's looking pretty solid! A big improvement.

One thing I discovered while testing with various tile sources is that ImageTileSource and LegacyTileSource don't wrap when you turn on wrapHorizontal and/or wrapVertical. Without this patch they don't wrap at all, and with this patch they wrap just once in each direction, giving you 4 copies of the image.

Would you be up for taking a look at that issue? Here's an example tile source:

var tileSource = {
    type: "image",
    url: "/test/data/A.png"
};

getTileAtPoint: function (level, point) {
return new $.Point(0, 0);
},
getTileAtPoint: $.TileSource.prototype.getTileAtPoint,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually you can just get rid of this line entirely; these subclasses inherit everything from TileSource unless overridden. Don't forget to remove the doc comment as well.

@iangilman
Copy link
Member

Awesome, looking good! Thank you for making this fix :)

@iangilman iangilman merged commit 6f62074 into openseadragon:master Jul 29, 2016
iangilman added a commit that referenced this pull request Jul 29, 2016
@iangilman iangilman mentioned this pull request Aug 26, 2016
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

2 participants