Kitware / minerva

Minerva: client/server/services for analysis and visualization
Apache License 2.0
36 stars 14 forks source link

Large image #488

Closed dorukozturk closed 6 years ago

dorukozturk commented 6 years ago

Replaces girder_ktile plugin with large_image.

Depends on https://github.com/girder/large_image/pull/252 and https://github.com/girder/large_image/pull/253

dorukozturk commented 6 years ago

@matthewma7 Can you review this?

matthewma7 commented 6 years ago

@dorukozturk Do you have the elevation.tif that you sent me at hand? This PR works fine with a small geotiff, but it seems large_image doesn't recognize elevation.tif after upload. So, I encountered an error after uploading the elevation.tif.

dorukozturk commented 6 years ago

@matthewma7 I made the changes that you requested. This is ready for another round of review.

matthewma7 commented 6 years ago

@dorukozturk Thanks for the changes and the new datasets. But with the elevation-medium.tif, when I give a custom style with a min value clamping, it will throw some error. Changing max works fine, but can't change min. image

dorukozturk commented 6 years ago

@matthewma7 Yes, I was able to reproduce the bug. https://github.com/Kitware/minerva/pull/488/commits/c9879add66ace2bd3414d5c03fc80d16f8e1ac46 fixes it.

matthewma7 commented 6 years ago

Hi @dorukozturk Thanks for the quick update. The previous issue is fixed. I found something else, but don't know if it's an easy fix. When I zoom out on the map with the elevation_medium.tif Clicking in the circle area will throw an error, but the rectangle area works fine. This probably has something to do with the value ranges. image

dorukozturk commented 6 years ago

@matthewma7 https://github.com/Kitware/minerva/pull/488/commits/94df4bfc69870831e86d33056a46abe6c9414515 addresses it. Let me know if that does not work for you.

matthewma7 commented 6 years ago

Hi @dorukozturk, I think it didn't fix it. It seems now clicking anywhere will throw an error.

dorukozturk commented 6 years ago

@matthewma7 I will try again but it worked for me. I will let you know on monday.

matthewma7 commented 6 years ago

@dorukozturk Thanks for pointing out. I didn't work with the latest large image. The code works great with the latest large master of large_image. And the changes looks great. Approving. 👍