Viewinghint:paged manifests containing images of varying heights are misaligned

WilsonAndrewWilson commented 4 years ago

UV version:

 universalviewer@4.0.0-pre.12

I'm submitting a:

[x] bug report
[ ] feature request => please use the user stories repo
[ ] support request => Please do not submit support requests here, use stackoverflow

Current behavior: When viewing a manifest with viewHint:paged that contains images of different heights, the images are not aligning correctly when viewed 2-up.

Example : https://collections.st-andrews.ac.uk/album-book-portfolio/orlando-furioso/762604/viewer#?#viewer&xywh=-1610%2C-1%2C12265%2C6432&cv=9&c=&m=&s=

Expected behavior: The images would scale height to align correctly. (as can be seen here in Mirador: https://collections.st-andrews.ac.uk/workspace/individual/762604

Steps to reproduce: Go here and go to the first few pages.

Other information: Related to #278 ?

edsilv commented 4 years ago

Mirador implementation. Could this be a generic util in manifesto?

https://github.com/ProjectMirador/mirador/blob/f41f7655d20ed3dcebad360c4cefdbc76c972e15/src/lib/CanvasWorld.js#L25-L81

cc @mejackreed

mejackreed commented 4 years ago

Sure thing. One of the things we found is that this changes based on the current type of view (single, paged, scroll, etc). So we end up needing a class that can determine this.

We end up using it for a few different things throughout the codebase.

cc @cbeer

edsilv commented 3 years ago

@mejackreed @cbeer

It looks like we might only need imageResources from MiradorCanvas for CanvasWorld to work in manifesto?

We're wondering what this line does: https://github.com/ProjectMirador/mirador/blob/f41f7655d20ed3dcebad360c4cefdbc76c972e15/src/lib/MiradorCanvas.js#L95

Could we move imageResources to a manifesto util too? e.g. imageResources(canvas) => resources

mejackreed commented 3 years ago

We're wondering what this line does: https://github.com/ProjectMirador/mirador/blob/f41f7655d20ed3dcebad360c4cefdbc76c972e15/src/lib/MiradorCanvas.js#L95

I think it maps things that are static images / or services that are part of layers.

Could we move imageResources to a manifesto util too? e.g. imageResources(canvas) => resources

Sure that seems like a nice move. Our class MiradorCanvas is really an extension of Manifest::Canvas and the API we would hope to be there.

edsilv commented 3 years ago

@mejackreed @stephenwf @WilsonAndrewWilson I've created a PR here to add CanvasWorld to manifesto:

https://github.com/IIIF-Commons/manifesto/pull/83

Once we're agreed that this is ok, I'll use it in the UV's openseadragon extension.

edsilv commented 3 years ago

So manifesto now has an implementation of CanvasWorld (v4.3.0-pre-4). I had to publish this in order to test it in the UV, but it's on the canvasworld branch.

It's aligning the tilesources nicely in two-up mode, but unfortunately the search functionality of the UV is now broken. I'm adding an offset to the search result annotations using CanvasWorld.offsetByCanvas, but they're still misaligned. I think the issue is because the UV uses OSD's addOverlay and Mirador is drawing rects directly onto the canvas.

Here's where the tilesources are getting initialised: https://github.com/UniversalViewer/universalviewer/blob/canvasworld/src/modules/uv-openseadragoncenterpanel-module/OpenSeadragonCenterPanel.ts#L[…]7

Here's where the offset is being applied to each annotation: https://github.com/UniversalViewer/universalviewer/blob/canvasworld/src/modules/uv-openseadragoncenterpanel-module/OpenSeadragonCenterPanel.ts#L1126

Not sure how to proceed now, as changing the way overlays work in the UV is a larger undertaking. Perhaps the UV Steering Group can discuss this on the next call (17th Dec)?

edsilv commented 3 years ago

glenrobson commented 3 years ago

After Ed flagged this up at Coco, I've been playing around with a few examples:

https://glenrobson.github.io/iiif_stuff/canvas_diff_dimensions/

I wonder if the correct solution is to not scale the smaller canvas? For Andrew's use case would it not be better to change the canvas heights to match rather than relying on the viewer behaviour?

Would this make the annotations on 2 up pages easier to work out?

glenrobson commented 2 years ago

There is a excellent blog and use case highlighting this issue here: https://blogs.bl.uk/digital-scholarship/2022/02/a-manuscript-reunited-and-a-iiif-viewer-issue.html

stephenwf commented 2 years ago

I don't think a viewer has really fixed this issue. The UV does not use the canvas dimensions when putting 2 images side by side (as this issue describes) but the approach that Mirador is taking is also removing control from the IIIF author to determine the relative heights.

I don't think it's correct to assume that "paged" behaviour also means "images should be displayed at the same height". I think that this is one of 2 things

An option provided by a viewer (e.g. a checkbox in the UI "[ ] normalise heights")
Another behaviour on the manifest, perhaps an extension

For a viewer to have enough information, it would need to know the physical dimensions, which is defined as an extension.

The only truly correct answer is to use the physical dimensions service. Otherwise you don't have enough info. The practical real world answer is to assume that within one published manifest with images from one source, the pixel canvas dimensions might be related to the real world sizes of the things. But that's just a likely scenario, you don't know. For mixing images from different sources - there is not enough information, unless you supply physical dimensions.

More on this (from @tomcrane)

I don't think the primary aim for a viewer should be to make too many assumptions on the meaning of the IIIF, instead to interpret and give alternative options. To me using the canvas height and width for displaying side-by-side images is required as part of this (as described in Uniform heights)

Allows for a page to be "taller" or "shorter" than an adjacent page - perhaps to accommodate a fold-out or partial page.
Allows for a scroll to have a variable width in a continuous view
Gives control to the author of the IIIF to change how a viewer presents this (the most important in my opinion)

By using the information in the canvas, it opens up some more questions:

Aligning pages + spacing (more behaviours? or viewer controls?)
Annotations spanning multiple pages
Over/under zooming when a small image is painted on a large canvas

I think our time would be best solving those problems - giving IIIF authors choices.

From a technical point of view, I think the way we are scaling in both the UV and Mirador is flawed. We are changing the data so that we can render it. It feels more natural to make this change at the rendering step.

paintCanvas(canvas1, {x: 0, y: 0});
paintCanvas(canvas2, {x: canvas1.width, y: 0});

paintAnnotationToCanvas(anno1, canvas);

or perhaps in JSX-like environment:

<zoom>
  <iiif-canvas canvas={canvas1} x={0} y={0}>
    <annotation x={100} y={100} width={100} height={100} />
  </iiif-canvas>
  <iiif-canvas canvas={canvas2} x={canvas2.width} y={0} />
</zoom>

The ease of the abstraction is important if you want to be able to make changes to spacing, or layout. I have some demos/examples to add to this - but I do not have the time at the moment to pull these together.

LlGC-szw commented 1 year ago

All issues will be triaged for further investigation or closure by the 28 September 2023. If your issue is still relevant and would like for it be investigated further please comment by 14 September 2023.

thattonBL commented 1 year ago

Still active

UniversalViewer / universalviewer

Viewinghint:paged manifests containing images of varying heights are misaligned #761