Skip to content

Fix zoom tool aspect ratio #2055

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Kantha2004
Copy link
Contributor

@Kantha2004 Kantha2004 commented May 8, 2025

Context

When a minScale is set to ZoomTool the image with greater height works fine but image with greater width stop zooming out when the image height reaches the height of the canvas

Changes & Results

  1. Changed Zoom tool take account the image ratio and canvas ratio

Both had the same configuration:
image

Before:

  1. ZoomTool will not properly work when the image resolution is smaller than the canvas size like it not allow you to zoom in.
  2. ZoomTool will stop zooming out when the image height reaches the canvas height even when the images sides are hidden
zoom-before.mp4

After:

  1. ZoomTool will now work as indented zoom tool will zoom till the minScale or maxScale is reached
  2. ZoomTool will zooming out when the image height reaches the canvas height if the image has greater height else zoomout till the image width matches the canvas width
zoom-after.mp4

Testing

Checklist

PR

fix(CobbAngleTool - touch): Add touch event listeners for CobbAngleTool

  • My Pull Request title is descriptive, accurate and follows the
    semantic-release format and guidelines.

Code

  • My code has been well-documented (function documentation, inline comments,
    etc.)

Public Documentation Updates

  • The documentation page has been updated as necessary for any public API
    additions or removals.

Tested Environment

  • "OS: Ubuntu 24.04.2
  • "Node version: 22.14.0
  • "Browser: chrome 133.0.6943.126 (Official Build) (64-bit)

@Kantha2004 Kantha2004 marked this pull request as draft May 8, 2025 10:28
@Kantha2004 Kantha2004 marked this pull request as ready for review May 8, 2025 10:38
@Kantha2004 Kantha2004 marked this pull request as draft May 8, 2025 12:05
@daker
Copy link
Contributor

daker commented May 8, 2025

Can you check if does also fix #1963 ?

@Kantha2004 Kantha2004 marked this pull request as ready for review May 24, 2025 09:28
@Kantha2004 Kantha2004 marked this pull request as draft May 24, 2025 09:28
@Kantha2004
Copy link
Contributor Author

Kantha2004 commented May 24, 2025

Can you check if does also fix #1963 ?

@daker Hi, sorry for the late reply. The reason this PR is still in draft is that, when zooming out, it does not account for the padding around the image.

In the default behavior, the displayArea looks like this:
{
"imageArea": [1.1, 1.1],
"imageCanvasPoint": {
"imagePoint": [0.5, 0.5],
"canvasPoint": [0.5, 0.5]
}
}
Currently, my implementation only checks whether the image's smallest dimension reaches the canvas border.

If you can guide me on how to find and implement the padding handling in this logic, I’d be happy to help.

And we can talk about in slack Kanthakumar K

@Kantha2004 Kantha2004 marked this pull request as ready for review May 25, 2025 06:22
@Kantha2004
Copy link
Contributor Author

Can you check if does also fix #1963 ?

@daker Hi, sorry for the late reply. The reason this PR is still in draft is that, when zooming out, it does not account for the padding around the image.

In the default behavior, the displayArea looks like this: { "imageArea": [1.1, 1.1], "imageCanvasPoint": { "imagePoint": [0.5, 0.5], "canvasPoint": [0.5, 0.5] } } Currently, my implementation only checks whether the image's smallest dimension reaches the canvas border.

If you can guide me on how to find and implement the padding handling in this logic, I’d be happy to help.

And we can talk about in slack Kanthakumar K

@daker I figured out how get the display area so that sorted but the Issue #1963 it sort of fixed because i don't why but the values are slightly off.

image

@Kantha2004
Copy link
Contributor Author

@sedghi can you check this because we need this in our project

let thresholdExceeded = false;
// Get display area, if available
const displayArea = viewport.options?.displayArea;
const imageAreaScaleX = displayArea?.imageArea?.[0] ?? 1.1;
Copy link
Member

Choose a reason for hiding this comment

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

what is this magic number 1.1?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it is not a magic number it is the default padding set to image area

Copy link
Contributor Author

Choose a reason for hiding this comment

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

tmp_a15d87f2-f558-4582-beb1-0601df1354ab.png

I got the value from here

@sedghi sedghi requested a review from wayfarer3130 May 28, 2025 13:26
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.

3 participants