search

emigre / openseadragon-annotations

Draw annotations over deep zoom images
BSD 3-Clause "New" or "Revised" License
97 stars 39 forks source link

The position of lines I drew is not the mouse position. #13

Closed thippo closed 7 years ago

thippo commented 7 years ago

When I drew lines displayed on openseadragon using openslide, the position of lines I drew is not the mouse position. why?

ays0110 commented 7 years ago

@Emigre @thippo

Figured out how to recreate-- the bug happens when the aspect ratio of the image is such that there is white space padding on the left and right hand side of the image when the openseadragon viewer is initialized.

This happens when your window's aspect ratio is wider than the aspect ratio of the openseadragon area, so the image is padded on the left and right to keep it "centered" in the window. There is some kind of miscalculation that doesn't account for this extra padding, so the coordinates of the mouse are calculated wrong.

You can "fix" this issue (lol) by making your window smaller/more narrow, or "break" it by dragging your window very wide.

ays0110 commented 7 years ago

@Emigre Further investigation:

Store.getWidth(); and Store.getHeight(); cannot be held constant to the initialized this.base.getBoundingClientRect(). This can change when the screen is resized, or if openseadragon centers the image in a viewer that is larger than the image aspect ratio.

This issue is fixed if I circumvent .getPercent() and just directly calculate the percent by calling for a fresh set of width and height from this.base.getBoundingClientRect().

Filing a PR for code that fixes this issue by skipping getPercent(), but you should probably decide the best way to implement this and how you want to use Store.getWidth() and Store.getHeight()

emigre commented 7 years ago

@thippo, @ays0110, many thanks for the bug report and the effort in fixing it. I have not had time lately to put into this plugin and I see several issues that have been reported. I will try to find some time to take a look at them at some point, and analyze this pull request, too. Thanks again for your contribution. 👍

ays0110 commented 7 years ago

@Emigre No problem, thanks a lot for making this plugin. It's a nice, lightweight option compared to things like Paperjs or Fabricjs, and it's very easy to work with. My lab is currently using this plugin to build a public portal for researchers to share and analyze cancer tissue, so please know that your work is definitely being put to good use by us all! 😄

emigre commented 7 years ago

Oh, that's great @ays0110! I'm really happy to hear that! 😃 I will get back to this during the weekend.

thippo commented 7 years ago

@ays0110 @Emigre Thanks a lot!

emigre commented 7 years ago

Thanks again for your contribution, this fixes the bug indeed.

As per the additional methods that you want to include to start and stop the drawing process, I am happy to add them too if you find them useful. The only drawback I see in it is that they add a new method to the Openseadragon namespace and it is recommended to add the less methods as possible to minimize the possibility of conflicts.

In any case, I'm fine for the time being. I was thinking on remaking the plugin in a way that only added one method to that namespace, I'll try to rework it a little and release it as version two this month - if I have the time! I hope so! Also I have taken a look and this definitely needs some tidying up, I have forgotten about it for a while.