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

959 Use image dimension properties in IIIF Manifest if they exist… #969

Merged
merged 2 commits into from
Aug 16, 2023

Conversation

alxp
Copy link

@alxp alxp commented Aug 7, 2023

GitHub Issue: (link)

#959

What does this Pull Request do?

When generating IIIF manifests, get with and height from the image field properties if they exist.

What's new?

If the site builder has chosen to use Service Files or otherwise picks a field that is an image type for the source of the image tiles, we have direct access to the image's dimensions and don't need to get them from Cantaloupe or by examining the file.
This greatly speeds up IIIF generation and takes a lot of load off of the Cantaloupe IIIF server.

  • Does this change add any new dependencies?
  • Does this change require any other modifications to be made to the repository
    (i.e. Regeneration activity, etc.)? No
  • Could this change impact execution of existing code? No

How should this be tested?

If using the Starter Site:

  1. Go to Admin -> Structure -> Views and edit IIIF Manifest
  2. For the default view, with path /node/%node/book-manifest, edit the settings for the IIIF Manifest in the Format section.
  3. Uncheck 'File, leaving 'Image' checked.
    image
  4. Clear Drupal's cache
  5. Add a repository item with model Paged Content.
  6. Add at least one child page Reposity Item with model Page.
  7. Add a media to the page object(s). It can be a TIFF or JPEG, we are going be using the Service File regardless.
  8. Tail the cantaloupe log, if running in Docker, it's likely: docker compose logs -f cantaloupe
  9. Clear Drupal's cache.
  10. Go to /node/[nid]/book-manifest where nid is the node id of the Paged Content item.
  11. You should not see any activity in the Cantaloupe logs for the manifest request.

Documentation Status

  • Does this change existing behaviour that's currently documented? No
  • Does this change require new pages or sections of documentation? No
  • Who does this need to be documented for? N/A

Additional Notes:

Any additional information that you think would be helpful when reviewing this
PR.

Interested parties

Tag (@ mention) interested parties or, if unsure, @Islandora/committers

@alxp alxp changed the title 959-use-image-dimensions Use image dimension properties if they exist… 959 Use image dimension properties in IIIF Manifest if they exist… Aug 7, 2023
@rosiel rosiel self-assigned this Aug 9, 2023
… when generating IIIF manifests.

959-use-image-dimensions Address PHPCS error.

959-use-image-dimensions Address PHPCS error.
@alxp
Copy link
Author

alxp commented Aug 11, 2023

some basic numbers for performance differences:: loading an 850 page item (not cached) took 16 seconds - then 6 seconds after cache https://digital.wolfsonian.org/node/66588 980 page item (pre-cache) 21 sec. - 6.6 seconds after cache https://digital.wolfsonian.org/node/66591

before the fix - often these extra large items would crash or time out - if they did load it was measured in minutes not seconds

Copy link
Member

@rosiel rosiel left a comment

Choose a reason for hiding this comment

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

With this PR, the manifest includes height and width values as strings not integers, e.g.

height : "5500" rather than height: 5500

From the IIIF Manifest docs,

Every canvas must have a label to display, and a height and a width as integers.

So I think maybe this does matter, at least from a strict-correctness position.

@alxp
Copy link
Author

alxp commented Aug 16, 2023

Image dimensions are now numeric in the JSON:

image

@rosiel rosiel merged commit 71f0945 into 2.x Aug 16, 2023
9 checks passed
@rosiel rosiel deleted the 959-use-image-dimensions branch August 16, 2023 14:09
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.

2 participants