Layers support #298

avandecreme · 2013-11-30T23:13:38Z

Following the discussion on #13 I just merged this work josh1093/openseadragon@master...layers into the current version, and it is still working :)

You can get an example here: http://avandecreme.github.io/openseadragon-debug-site/
Not the best dataset for this feature but I had nothing better at hand.

Obviously, a lot of work remains to do before merging but I wanted to share that first step in order to discuss how we want to neatly integrate the layers support.

I think the addLayer method should have an options parameter like this:

var options = {
   tileSource: tileURL or tileSource object,
   opacity: opacity of the layer's canvas (default to 1)
}

This method should return something allowing to later remove the layer via a Viewer.removeLayer but I am not sure what that thing would be. The drawer associated with the layer?

Also, it might be useful to have some sort of hierarchy to set layers order. Should we use something similar to css property z-index? Maybe even use it.

iangilman · 2013-12-02T19:09:02Z

src/viewer.js

@@ -1998,6 +2014,18 @@ function resizeViewportAndRecenter( viewer, containerSize, oldBounds, oldCenter
    viewport.fitBounds( newBounds, true );
 }

+function updateDrawers( viewer ) {
+    viewer.drawers.forEach( function( drawer ) {


We can't use forEach' andsome`, as we support IE8 and IE8 does not have them.

Ok, I will make sure to change that.

iangilman · 2013-12-02T19:20:48Z

Yes, opacity and z-order are both important. Ideally we'd be able to modify them as needed (not just at initialization). And yes, we'll also want the remove. I guess if the layers are just drawers, the opacity can be built into the drawer API (might be handy even for the base layer). The drawer could be returned by addLayer (rather than the drawer's canvas), and that can be passed into the viewer's removeLayer. Setting z-order should probably happen at the viewer level as well. I think we should have JavaScript control over that, so it wouldn't necessarily be literally the CSS z-index.

avandecreme · 2013-12-02T19:44:03Z

Great, thanks for the feedback!

avandecreme · 2013-12-09T14:26:32Z

New commit. It is not finalized yet as it is not extensively tested. Also adding a layer to a collection need to be handle (maybe just throw an error).

avandecreme · 2013-12-09T14:33:15Z

I made 2 comments there: avandecreme@0c2af65

While commenting, I also realized that I need to clean the DOM (remove layers) when viewer.close is call.

iangilman · 2013-12-09T18:34:51Z

When you're happy with the code, please add some tests. Also it would be great to have a "demo" page (like the current basic.html) to be able to see it in action.

avandecreme · 2013-12-09T18:36:46Z

Sure!

PetterRanefall · 2013-12-20T12:01:57Z

I have now beean able to try it and It works fine. Thanks!

iangilman · 2013-12-31T17:41:15Z

The recent commit looks good (just getting back from Christmas break).

Add the original options in addLayer events Add layers demo page

avandecreme · 2014-01-06T00:25:08Z

New commit fixing bugs and adding the demo page (back from vacations too :) )
I still want to add unit tests and check what happens with collections.

@PetterRanefall, if you are already using it, you should use that new version as I fixed a major bug (getLayerLevel was not working on most browsers).

iangilman · 2014-01-06T17:53:24Z

The demo is great! A couple suggestions:

It took me a little bit to figure out what I was supposed to do with the control on the right. It would probably be clearer if there were already a couple of layers in use to begin with. Also, that's most likely what people want to be seeing anyway, so it's a good starting state.
Maybe instead of left and right arrows, those buttons could be called "add" and "remove"; or maybe in addition to the arrows (since the arrows help make it clear what the lists mean).
The top layer appears at the bottom in the list; this is counter-intuitive... can you flip the order?

Otherwise looking lovely. :-)

avandecreme · 2014-01-06T18:17:24Z

I agree with all 3 points.
I also would like to add a button to test rotation.

iangilman · 2014-01-07T19:21:12Z

Sounds good. :-)

layers.html test file: -display 2 layers after opening -fix order -add opacity demo -add rotation demo

…lOfLayer. Add layers unit test.

avandecreme · 2014-01-12T00:28:42Z

All right, I think I wrote all I wanted to :)
Comments are still welcome though!

iangilman · 2014-01-15T17:32:08Z

test/demo/layers.html

+
+        <div>
+            <div id="availables">
+                Availables layers<br>


Should be "available layers".

iangilman · 2014-01-15T17:36:42Z

The demo looks great! Just a couple minor comments (above) and that question about getNumberOfLayers().

Oh, also that method name could be shorter: getLayerCount().

avandecreme · 2014-01-16T20:42:25Z

Other thoughts quite related to this work:

When working with sequences, it could be nice to have an option to cache the previous and next images in drawers to avoid the blinking effect when you change images.
When specifying a tile source, we somehow get the width and height of the entire image. We could have options to specify what part of the image we actually want OSD to render. It would be practical for layers if you want to add a layer bigger than the base image. It is also related in some way to Padded Image tiles are shrinking at the edges of the dzi image and retains its size on zoom #234 (I reproduced the same kind of side effects with layers).

iangilman · 2014-01-17T17:15:29Z

Good points. I think the first one could be filed as a new issue to tackle after this patch lands. The second one as well, except what happens now if you add a layer that's not the same size as the base layer? Perhaps we should detect that state and not allow it for the time being.

avandecreme · 2014-01-23T18:13:16Z

I looked in the overlay compatibility. It is causing some problems.
Currently, there are 2 ways to add overlays:

from the viewer's constructor. In that case, anytime a drawer is created (currently, only when opening a tile source), this drawer get the overlay added via drawer.addOverlay.
by manually calling drawer.addOverlay. In that case, when the viewer open a new image, the overlays of the current drawer is removed.

Now, as one viewer can have multiple drawers with different overlays at the same time, we need to clarify what is the expected behavior.
In the case of the viewer's overlay, I think the overlay should always be displayed above all layers. Also, if the base layer change, I think that the overlay should be attached to the new base layer.

However, I am not sure what is best for a drawer's overlay. Should they be behind any layer above the drawer they are attached to? Should that be an option? Also, as the overlay is directly attached to the drawer, I think they should be removed whenever the layer is removed.

iangilman · 2014-01-24T17:10:21Z

Good point. I agree that the viewer's overlay should appear above all of the layers. As for drawer overlays, I'm not sure why we even support them. Having them exist above that drawer's layer but below the layers on top sounds reasonable, and I agree they should be removed when the drawer goes.

avandecreme · 2014-01-24T18:48:54Z

As for drawer overlays, I'm not sure why we even support them.

It is the only way to add overlays after having created the Viewer object.
But maybe all that overlay code should be moved from the drawer to the viewer (or viewport?).

iangilman · 2014-01-27T17:21:33Z

Maybe I'm missing some part of the picture, but it seems like it makes sense for the overlay code (or at least the overlay API) to reside in the viewer.

avandecreme · 2014-01-27T21:53:10Z

Ok, I will look into this in another pull request and see if it is feasible.
If so, it will be a lot easier to make layers and overlays work together.

iangilman · 2014-01-29T01:13:39Z

Makes sense. What's the plan for this patch? Wait for the overlay refactor, or put some guards in for overlays? Also there's still the aspect ratio check to do, right? Just making sure we're on the same page.

avandecreme · 2014-01-29T02:23:43Z

I will wait for the overlay refactor.
And yes there is still the aspect ratio thing.

PetterRanefall · 2014-03-04T10:28:14Z

I have an original image where the size (31744x41472) is not a multiple of the tile size (2048x2048). When generating tiles for this image I get some tiles at the end of the rows/columns with tile sizes smaller than 2048x2048. So far so good, this works fine. Then I have done some image analysis on the highest resolution tiles, but only on the tiles of size 2048x2048, and produced a new resolution pyramid bottom up from these results. This means that at the highest resolution I only have 30720x40960 pixels. When I use this new resolution pyramid as an overlay to the original image the overlay gets stretched and I get a mismatch between the image and the overlay.

PetterRanefall · 2014-03-04T11:42:40Z

When I cut the original to a multiple of the tile size then the alignment is perfect. I can live with this solution, but I still think the behavior is strange.

iangilman · 2014-03-04T17:35:51Z

@PetterRanefall Is that using this patch?

I'm pretty sure the suggested use with this patch is that all of your layers be the same size.

avandecreme · 2014-03-04T18:42:30Z

Yep, actually the aspect-ratio should be (almost) the same (see layersAspectRatio).
@PetterRanefall What behavior would you expect if the ratio is not the same?

PetterRanefall · 2014-03-05T07:27:33Z

I expected (i.e. wanted) that the (0, 0) corners would be aligned and the tiles would have the same pixel size. This was since I don't create overlay tiles where the original tiles don't have full size. My current solution is to crop the original image before I create any tiles.

avandecreme · 2014-03-05T13:53:41Z

Ok, it is somewhat related with the comment I made above

When specifying a tile source, we somehow get the width and height of the entire image. We could have options to specify what part of the image we actually want OSD to render. It would be practical for layers if you want to add a layer bigger than the base image. It is also related in some way to #234 (I reproduced the same kind of side effects with layers).

This will probably won't happen for the next release though.

iangilman · 2014-03-05T17:11:20Z

Yes, I agree it's a bit much to tack onto this patch, but a nice feature request for the future. Perhaps file a new issue for it?

@PetterRanefall Meanwhile, couldn't you also make your second layer the same size as the original? Why chop off the partial tiles?

PetterRanefall · 2014-03-06T06:46:39Z

One problem is that the image analysis pipeline demands that all tiles have the same size. That is why I don't get any overlay tiles from the partial tiles. Of course I could pad the original image instead of chopping it, but the edge tiles are basically empty anyway. As I said, I can live with this solution.

iangilman · 2014-03-06T17:31:44Z

Good to know! I'm always interested to hear how people are using the tech. :-)

Conflicts: src/drawer.js src/viewer.js test/test.html

…p of layers.

avandecreme · 2014-03-20T14:05:12Z

Merge with master and fix for overlays done.

This last commit is somewhat a breaking change because 2 new divs are added.
So if one was accessing the drawer element via viewer.canvas.firstChild he now needs to do viewer.drawersContainer.firstChild.
In any case the best solution is probably viewer.drawer.canvas which stay the same before and after this commit.

iangilman · 2014-03-20T16:19:22Z

Looks good. I think that means this patch is ready to land, right? Can you add a changelog item for it? Please mention the breaking change (though I imagine it's not the sort of thing anyone will actually encounter, still good to have it documented).

avandecreme · 2014-03-20T20:02:32Z

Yes ready for merging! Changelog above :)

iangilman · 2014-03-21T16:13:08Z

Beautiful... great to have this feature!

Layers support

Add basic layers support

58da998

avandecreme mentioned this pull request Nov 30, 2013

Add support for layers of tiles that can be turned off and on at runtime #13

Closed

iangilman reviewed Dec 2, 2013
View reviewed changes

Add support to add/remove layers and change their levels.

0c2af65

Add layer-level-changed event and add $.indexOf polyfill

c90a1ed

Fix $.indexOf pollyfill

b542b90

Add the original options in addLayer events Add layers demo page

Uncomment version

63af1e7

avandecreme added 4 commits January 11, 2014 12:52

Add viewer.getNumberOfLayers method

56707bd

layers.html test file: -display 2 layers after opening -fix order -add opacity demo -add rotation demo

Add viewer.getLayerAtLevel method and rename getLayerLevel to getLeve…

f0f76b8

…lOfLayer. Add layers unit test.

Prevent adding collections as layers.

cd62ba7

Throw an error when trying to add a layer in collection mode.

facccf7

iangilman reviewed Jan 15, 2014
View reviewed changes

test/demo/layers.html

<div>

<div id="availables">

Availables layers<br>

Copy link

Member

iangilman Jan 15, 2014

Choose a reason for hiding this comment

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

Should be "available layers".

avandecreme mentioned this pull request Jan 28, 2014

Drawing rectangles on an image #329

Closed

Antoine Vandecreme added 2 commits January 30, 2014 16:43

Add layersAspectRatioEpsilon check

d43b6d8

Update the source when the base layer change.

695df87

avandecreme added 2 commits March 19, 2014 16:24

Merge branch 'master' into layers

ba10c39

Conflicts: src/drawer.js src/viewer.js test/test.html

Separate layers and overlays to ensure that overlays are always on to…

73eaf2c

…p of layers.

Add changelog for openseadragon#298

447a7a2

iangilman added a commit that referenced this pull request Mar 21, 2014

Merge pull request #298 from avandecreme/layers

93096fe

Layers support

iangilman merged commit 93096fe into openseadragon:master Mar 21, 2014

avandecreme deleted the layers branch March 21, 2014 17:27

avandecreme mentioned this pull request Jun 20, 2014

Why are aspect ratios of layers required to match? #425

Closed

Layers support #298

Layers support #298

Conversation

avandecreme commented Nov 30, 2013

iangilman Dec 2, 2013

Choose a reason for hiding this comment

avandecreme Dec 2, 2013

Choose a reason for hiding this comment

iangilman commented Dec 2, 2013

avandecreme commented Dec 2, 2013

avandecreme commented Dec 9, 2013

avandecreme commented Dec 9, 2013

iangilman commented Dec 9, 2013

avandecreme commented Dec 9, 2013

PetterRanefall commented Dec 20, 2013

iangilman commented Dec 31, 2013

avandecreme commented Jan 6, 2014

iangilman commented Jan 6, 2014

avandecreme commented Jan 6, 2014

iangilman commented Jan 7, 2014

avandecreme commented Jan 12, 2014

iangilman Jan 15, 2014

Choose a reason for hiding this comment

iangilman commented Jan 15, 2014

avandecreme commented Jan 16, 2014

iangilman commented Jan 17, 2014

avandecreme commented Jan 23, 2014

iangilman commented Jan 24, 2014

avandecreme commented Jan 24, 2014

iangilman commented Jan 27, 2014

avandecreme commented Jan 27, 2014

iangilman commented Jan 29, 2014

avandecreme commented Jan 29, 2014

PetterRanefall commented Mar 4, 2014

PetterRanefall commented Mar 4, 2014

iangilman commented Mar 4, 2014

avandecreme commented Mar 4, 2014

PetterRanefall commented Mar 5, 2014

avandecreme commented Mar 5, 2014

iangilman commented Mar 5, 2014

PetterRanefall commented Mar 6, 2014

iangilman commented Mar 6, 2014

avandecreme commented Mar 20, 2014

iangilman commented Mar 20, 2014

avandecreme commented Mar 20, 2014

iangilman commented Mar 21, 2014