Skip to content

Conversation

@e-platini
Copy link
Contributor

@e-platini e-platini commented Nov 14, 2024

Bug description:
#73

I changed the css and added code to scale height while keeping aspect ratio using "object-fit: contain". For height I picked 85vh arbitrarily because it works on all reasonable computer screen resolutions. Not perfect but seems to work better than current.

Keeping aspect ratio with "object-fit: contain" creates a margin within the canvas on the smallest side of the image. Therefore I had to adapt the function that calculates the mouse position, since it was based on the whole canvas and now need to account for the difference between the canvas and the displayed image.

Tested on chrome + firefox using all available resolutions from windows settings. It works on classification, bbox, landmarks, could not test it on other tasks because I don't have working examples. @smistad can you check for yourself, or provide me with files to create test datasets with?

@jpdefrutos told me maybe I should have modified get_image_as_http_response() from utility instead:
get_image_as_http_response

Please review and tell me whether this is a good fix.

Before:
image

After:
image

@smistad
Copy link
Owner

smistad commented Dec 16, 2024

What if you only have this css for #canvas?

#canvas {
    height: 85vh;
}

Wouldn't this achieve the same thing without needing a change in mousePos?

@e-platini
Copy link
Contributor Author

e-platini commented Dec 17, 2024

@smistad
It does work in most cases, except if the image is long (when width is more than ~1.8x the height). I guess it's a trade off about how much of an edge case a long image is considered to be.
image

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