These are chat archives for openseadragon/openseadragon

14th
Jul 2015
Conner Wingard
@ConnerMan
Jul 14 2015 14:51
Hey @iangilman, I tested the image @azaroth42 provided for #673. It seems to work, I posted a screenshot in the comments of the PR.
Screen Shot 2015-07-14 at 10.44.02 AM.PNG
Added it here too for reference. :beers:
Ian Gilman
@iangilman
Jul 14 2015 16:17
Looking good!
@ConnerMan ^
Looks like that patch is ready to land now; would you agree?
Conner Wingard
@ConnerMan
Jul 14 2015 16:32
I think so, but I think thats up to you :)
Ian Gilman
@iangilman
Jul 14 2015 16:32
:) Just making sure you're happy with it
Looks good to me. I'll land it today or tomorrow
Conner Wingard
@ConnerMan
Jul 14 2015 16:34
Okay cool. One thing I did notice now that I think about it. It seems the TileSources set everything provided in options on the object. which results in the object having both tileWidth/tileHeight and _tileWidth/_tileHeight set when provided. Don't know if that might confuse developers?
Conner Wingard
@ConnerMan
Jul 14 2015 16:41
No effect on functionality, just ease of use for users and devs of the lib, might be worth thinking about down the road. I like the leading underscore convention for 'private' internal variables though
Ian Gilman
@iangilman
Jul 14 2015 16:43
Yeah, I'm trying to move toward keeping all properties "private" and providing accessor methods only when needed; gives us more flexibility for the future.
Maybe it's worth doing a delete options.tileWidth after you grab the tileWidth so it's not also a property, to keep things clean
Btw, have you given any thought to how one might add a test for the non-square tile code?
Conner Wingard
@ConnerMan
Jul 14 2015 16:50
I like the delete idea, that would clean things up a bit. I'll add that. As for test, I might need some guidance. I looked for a test of TileSource and it descendants and didn't seem to find one. I guess they are tested indirectly through test/modules/formats?
Ian Gilman
@iangilman
Jul 14 2015 16:52
Indeed. We probably haven't gotten around to testing the TileSource core.
Do you want to take a whack at creating a general tileSource test file? It doesn't have to have more than needed to test for non-square tile support for now.
Conner Wingard
@ConnerMan
Jul 14 2015 16:54
Sure, I could try adding a basic one that tests the non-square tile support.
That, and the delete, and I think we would be golden.
Ian Gilman
@iangilman
Jul 14 2015 16:55
Awesome :)
Actually, while you're at it, you might as well put in the changelog entry (including deprecation notice) for this feature.
Conner Wingard
@ConnerMan
Jul 14 2015 16:56
Just modify changelog.txt to do that?
Ian Gilman
@iangilman
Jul 14 2015 16:58
Yup. Often I do that for small patches, but this is a big one, and you might be best to write it
Conner Wingard
@ConnerMan
Jul 14 2015 16:58
Okay, cool, I'll take a shot at it.
Ian Gilman
@iangilman
Jul 14 2015 16:59
Awesome :)
Conner Wingard
@ConnerMan
Jul 14 2015 19:07
  • Added support for non-square tiles (#673)
    • TileSource.Options objects can now optionally provide tileWidth/tileWidth instead of tileSize for non-square tile support.
    • IIIFTileSources will now respect non-square tiles if available.
    • DEPRECATION: TileSource.getTileSize is deprecated use TileSource.getTileWidth and TileSource.getTileHeight instead.
What about this for the addition to the changelog?
Ian Gilman
@iangilman
Jul 14 2015 20:04
@ConnerMan Looks good.
Conner Wingard
@ConnerMan
Jul 14 2015 20:18
okay PR updated, with tests, TileSource private cleanup, and changelog. Unfortunately it says there is a merge conflict :worried:
Ian Gilman
@iangilman
Jul 14 2015 20:25
That's probably the changelog. Can you grab the latest from OSD master?
(that's why I often do the changelogs)
@ConnerMan ^
Grant Echols
@gechols
Jul 14 2015 20:37
I have a custom canvas-scroll handler that pans the viewport up and down using the viewport.panBy method. This handles the scroll button nicely. However, the click-drag gesture that results in a 'pan' event isn't quite so nice because it allows the user to pan the image too far down the page so the viewport ends up showing too much black space above the image and the image can't be found. I'm struggling with a way to prevent the pan from going too far and figured there must already be something simple in place to prevent this. Any pointers?
Ian Gilman
@iangilman
Jul 14 2015 20:39
@gechols Well, there's the constraint system. It's not terribly flexible though. Another approach is to catch pan events and adjust the viewport's pan target at that time.
Grant Echols
@gechols
Jul 14 2015 20:46
@iangilman I am capturing the pan events, but it doesn't look like there's anything useful in the pan event itself (maybe I'm missing it). So far I'm getting myself either in an endless loop or I'm jumping even further down or up the view. Here's what I've got currently in the 'pan handler':
var viewBounds = viewport.getBounds();
var delta = null;
if (viewBounds.y < 0) {
delta = openseadragon.viewport.deltaPointsFromPixels(new OpenSeadragon.Point(0, Math.abs(viewBounds.y)));
openseadragon.viewport.panBy(delta, true);
}
I think I'm getting lost in the coordinate systems again.
Conner Wingard
@ConnerMan
Jul 14 2015 20:48
@iangilman fixed merge conflict, build running :)
Ian Gilman
@iangilman
Jul 14 2015 20:48
@ConnerMan Awesome. You're on a roll :)
@gechols Let me see...
@gechols Something like this should work:
var inConstraintCode = false;
viewer.addHandler('pan', function() {
    if (inConstraintCode) {
        return;
    }

    inConstraintCode = true;

    var viewBounds = viewer.viewport.getBounds();
    if (viewBounds.y < 0) {
        viewBounds.y = 0;
        viewer.viewport.fitBounds(viewBounds);
    }

    inConstraintCode = false;
});
(untested code)
Conner Wingard
@ConnerMan
Jul 14 2015 21:00
Looks like #673 passed. Might be ready for prime time, let me know if you have any concerns @iangilman cheers!
Ian Gilman
@iangilman
Jul 14 2015 21:01
@ConnerMan Will do!
Grant Echols
@gechols
Jul 14 2015 21:30
@iangilman The fitBounds is a great help! Thanks a ton!
Ian Gilman
@iangilman
Jul 14 2015 21:40
Excellent :)