Added DeepZoom support #146

mvictoras · 2015-01-07T17:41:12Z

Added DZI as supported output format. Openslide is a prerequisite for this to work.

lovell · 2015-01-07T20:08:19Z

Hi Victor, thanks for adding this useful new feature.

npm test is currently failing at the jshint check. Please can you fix this and add tests to test/unit/io.js to cover your new options and code.

Please can you rename the slightly generic sounding overlap method to be the more-specific tileOverlap, which would also then match tileSize.

The pre-compiled libvips-dev packages for the latest stable Debian/Ubuntu releases don't include the libgsf-1-dev dependency so it's probably best to wrap the use of the vips_dzsave method in resize.cc with #ifdef HAVE_GSF (if that's available) or similar.

Are you able to update the preinstall.sh script with the new dependencies on libgsf-1-dev for Debian-based and libgsf-devel for RHEL-based Linux?

Once you're happy with the above, please can you change the destination of your PR to the intake branch (a limitation of Github means you may have to close and open a new PR to do this) as that's where the new features for the next major release v0.9.0 are assembling themselves. Sorry for not making the branching tactics clear - I need to improve making contribution as easy as possible.

Finally, and most importantly, please can you add yourself to the list of contributors in package,json as you deserve it.

Cheers,
Lovell

lovell · 2015-01-07T20:12:29Z

src/resize.cc

@@ -679,6 +684,15 @@ class ResizeWorker : public NanAsyncWorker {
          return Error(baton, hook);
        }
        baton->outputFormat = "tiff";
+      } else if (outputDzi || (matchInput && inputImageType == ImageType::DZI)) {
+#if (VIPS_MAJOR_VERSION >= 7 && VIPS_MINOR_VERSION >= 41)


sharp currently requires libvips 7.40.0+, which I believe provides all the API needed for vips_dzsave, so this #if directive can probably be removed.

lovell · 2015-01-09T15:30:29Z

"Sorry for not making the branching tactics clear - I need to improve making contribution as easy as possible."

I've added the first version of a guide to help contributors.

mvictoras · 2015-01-09T16:06:24Z

Great, thank you for your feedback.

I 'll send you a new pull request once I have everything ironed out,
probably end of next week.

Victor

On Fri, Jan 9, 2015 at 10:30 AM, Lovell Fuller notifications@github.com
wrote:

"Sorry for not making the branching tactics clear - I need to improve
making contribution as easy as possible."

I've added the first version of a guide to help contributors
https://github.com/lovell/sharp/blob/intake/CONTRIBUTING.md.

—
Reply to this email directly or view it on GitHub
#146 (comment).

Victor Mateevitsi
PhD Candidate - University of Illinois at Chicago http://www.uic.edu/
Research Assistant - Electronic Visualization Laboratory
http://www.evl.uic.edu/
mvictoras@gmail.com
www http://www.vmateevitsi.com | LinkedIn
http://www.linkedin.com/pub/5/919/845 | Twitter
http://twitter.com/mvictoras | UIC http://www.uic.edu | EVL
http://www.evl.uic.edu

coveralls · 2015-01-12T19:10:23Z

Coverage decreased (-3.68%) when pulling 92ccbcb on mvictoras:master into 850c2ec on lovell:master.

lovell · 2015-01-23T10:50:19Z

I'm about to merge all the v0.9.0 code into master ahead of the next major release. I'll create a judgement branch for new v0.10.0 features, including your sterling work, once that's done.

mvictoras · 2015-01-23T18:19:39Z

Great, thank you.

It is almost done, I just got tangled up in some other things I need to get
done.

Victor

On Fri, Jan 23, 2015 at 4:50 AM, Lovell Fuller notifications@github.com
wrote:

I'm about to merge all the v0.9.0 code into master ahead of the next major
release. I'll create a judgement branch for new v0.10.0 features,
including your sterling work, once that's done.

—
Reply to this email directly or view it on GitHub
#146 (comment).

Victor Mateevitsi
PhD Candidate - University of Illinois at Chicago http://www.uic.edu/
Research Assistant - Electronic Visualization Laboratory
http://www.evl.uic.edu/
mvictoras@gmail.com
www http://www.vmateevitsi.com | LinkedIn
http://www.linkedin.com/pub/5/919/845 | Twitter
http://twitter.com/mvictoras | UIC http://www.uic.edu | EVL
http://www.evl.uic.edu

jcupitt · 2015-02-12T10:53:17Z

Hi, tiny idea: instead of testing HAVE_GSF or equivalent, you could search for the dzsave operation, perhaps:

if (vips_type_find("VipsOperation", "dzsave")) {
   ... we have vips_dzsave() available
}

vips_type_find() searches below a named vips class for a class with a nickname. If vips was built without dzsave support, it won't be there.

I notice vips_type_find() is missing a gtk-doc comment, I'll add one.

http://www.vips.ecs.soton.ac.uk/supported/7.42/doc/html/libvips/VipsObject.html#vips-type-find

lovell · 2015-02-12T12:10:09Z

Thanks @jcupitt, that's a very useful method.

mvictoras · 2015-02-12T18:20:56Z

Thank you @jcupitt, I was a little busy with
other stuff, will get back to this next week.

On Thu, Feb 12, 2015 at 7:10 AM, Lovell Fuller notifications@github.com
wrote:

Thanks @jcupitt https://github.com/jcupitt, that's a very useful method.

—
Reply to this email directly or view it on GitHub
#146 (comment).

Victor Mateevitsi
PhD Candidate - University of Illinois at Chicago http://www.uic.edu/
Research Assistant - Electronic Visualization Laboratory
http://www.evl.uic.edu/
mvictoras@gmail.com
www http://www.vmateevitsi.com | LinkedIn
http://www.linkedin.com/pub/5/919/845 | Twitter
http://twitter.com/mvictoras | UIC http://www.uic.edu | EVL
http://www.evl.uic.edu

mvictoras · 2015-02-25T00:59:58Z

Just sent a new pull request.

On Thu, Feb 12, 2015 at 12:20 PM, Victor Mateevitsi mvictoras@gmail.com
wrote:

Thank you @jcupitt https://github.com/jcupitt, I was a little busy with
other stuff, will get back to this next week.

On Thu, Feb 12, 2015 at 7:10 AM, Lovell Fuller notifications@github.com
wrote:

Thanks @jcupitt https://github.com/jcupitt, that's a very useful
method.

—
Reply to this email directly or view it on GitHub
#146 (comment).

Victor Mateevitsi
PhD Candidate - University of Illinois at Chicago http://www.uic.edu/
Research Assistant - Electronic Visualization Laboratory
http://www.evl.uic.edu/
mvictoras@gmail.com
www http://www.vmateevitsi.com | LinkedIn
http://www.linkedin.com/pub/5/919/845 | Twitter
http://twitter.com/mvictoras | UIC http://www.uic.edu | EVL
http://www.evl.uic.edu

Victor Mateevitsi
PhD Candidate - University of Illinois at Chicago http://www.uic.edu/
Research Assistant - Electronic Visualization Laboratory
http://www.evl.uic.edu/
mvictoras@gmail.com
www http://www.vmateevitsi.com | LinkedIn
http://www.linkedin.com/pub/5/919/845 | Twitter
http://twitter.com/mvictoras | UIC http://www.uic.edu | EVL
http://www.evl.uic.edu

mvictoras · 2015-02-25T20:10:56Z

@jcupitt, is there a way that I can find out if vips was compiled with openslide support?
I tried vips list format, but cannot see anything related.

What I am trying to do is extend the preinstall.sh a script so that it checks whether vips was compiled with openslide support. If not, it downloads and recompiles vips.

jcupitt · 2015-02-25T21:28:00Z

@mvictoras yes, the operator is called openslideload. Use vips list classes to see everything under VipsObject. For example:

$ vips list classes | grep -i opensli
        VipsForeignLoadOpenslide (openslideload), load file with OpenSlide (.svs, .vms, .vmu, .ndpi, .scn, .mrxs, .svslide, .tif, .bif), priority=100, is_a, get_flags, get_flags_filename, header, load

or from C:

if (vips_type_find("VipsOperation", "openslideload")) {
   ... we have vips_openslideload() available
}

mvictoras · 2015-03-06T03:34:15Z

Ok I squahed into a single commit and updated with everything that we talked about.
Let me know what you think.

mvictoras · 2015-03-06T03:37:50Z

Regarding the preinstall script I have added and tested the openslide support for all the Operating Systems, except from the Mac OSX with Macports and Amazon cloud, since I don't have access to any of those. It would be great, if you can check if it is working on your machines!

lovell · 2015-03-06T08:59:12Z

index.js

@@ -501,6 +534,14 @@ Sharp.prototype.toFormat = function(format) {
 };

 /*
+  Force DZI output
+*/
+Sharp.prototype.dzi = function() {


Do we need dzi() given dz() exists?

I probably missed that with the merge and squash. Thanks!

lovell · 2015-03-06T09:05:38Z

Looking great, thanks Victor. I've added a few comments inline - now to verify the much-improved script on a few different OSs.

lovell · 2015-03-06T10:39:29Z

README.md

@@ -57,6 +60,14 @@ or run the following as `root`:

 	curl -s https://raw.githubusercontent.com/lovell/sharp/master/preinstall.sh | bash -

+To enable OpenSlide, add the --with-openslide argument:
+
+> curl -s https://raw.githubusercontent.com/lovell/sharp/master/preinstall.sh --with-openslide | sudo bash -


I had to run this as:

curl -s https://raw.githubusercontent.com/mvictoras/sharp/master/preinstall.sh | sudo bash -s -- --with-openslide

to pass the --with-openslide parameter.

Good catch, I will fix that.

lovell · 2015-03-06T11:13:11Z

I was expecting to see the use of vips_openslideload in common.cc - am I missing something?

mvictoras · 2015-03-06T17:11:50Z

preinstall.sh

+        brew install homebrew/science/vips --with-webp --with-graphicsmagick --with-openslide
+      else
+        brew install homebrew/science/vips --with-webp --with-graphicsmagick
+      fi
    elif type "port" > /dev/null; then
      echo "Installing libvips via MacPorts"
      port install vips


Can you check what happens if openslide is enabled? Does vips gets installed with openslide support or not? I don't have Macports so I cannot test.

Looks like the version of libvips installed via MacPorts already includes openslide support. (But it does seem to be missing WebP!)

mvictoras · 2015-03-06T18:00:25Z

Since we are not dealing with openslide as a buffer, I believe vips_openslideload is not needed.
When an openslide input is selected, the vips_*save is called, and vips knows internally what to do with openslide images.

So for example, if we are converting from openslide to png:
vips_pngsave is called.

Added OpenSuse 13.1 and 13.2 support in preinstall.sh script. Added OpenSlide support in preinstall script. Added unit tests for Deep Zoom and OpenSlide.

mvictoras · 2015-03-06T18:04:01Z

I also committed all the changes and fixes, so now you have a clean pull request.

lovell · 2015-03-07T15:44:03Z

With or without openslide on any given machine, these new Aperio SVS test files are accepted/parsed by libvips' existing TIFF file loader, so the current version of sharp already supports them; bonus!

Do you have a requirement to process files in formats only openslide can understand (but libvips' existing tiff/jpeg etc. loaders cannot), perhaps MIRAX? I suspect these won't work without the addition of vips_openslideload.

jcupitt · 2015-03-07T16:39:33Z

Hmm, that shouldn't happen, you must test for openslide before tiff. Most of the openslide formats use tiff as the wrapper, but opening with libtiff will miss most of the image.

There should be a priority system. Openslide is highest priority, then vips's libtiff loader, then the imagemagick loader (which can also load tiff, but very badly) right at the bottom of the pile.

Openslide will refuse to load regular tiffs. It checks the VENDOR tag.

lovell · 2015-03-10T09:54:24Z

Thanks @jcupitt , sharp uses libvips' per-format loaders so currently has it's own priority. vips_openslideload is not present and vips_tiffload is, hence the incorrect use of the latter for the sample SVS image. This was probably for "historic reasons" 🏰 . I will switch the loading logic, which is now separate from the what-format logic, to use vips_image_new_from_file.

@mvictoras I will merge in your sterling work to a new WIP branch locally and make this loading-priority change later today. I found a MIRAX sample to test with too. Hopefully everything will... slide... into place very soon.

mvictoras · 2015-03-10T16:10:02Z

Thanks @jcupitt!
@lovell great, thank you! I actually had this implemented, but deleted it because I thought it is not being used. Let me know if you need any help.

Also, I will start working on a windows build, will let you know how it goes.

lovell · 2015-03-12T15:41:59Z

This PR is merged to the judgement branch as 2d1e6f2

lovell · 2015-03-12T15:50:38Z

@mvictoras Are you able to help test the new judgement v0.10.0 work-in-progress branch containing your Openslide and Deep Zoom additions?

npm install lovell/sharp#judgement

In commit 5781a23 I made a small public API change to combine tileSize and tileOverlap into a single tile method to slightly future-proof if/when someone wants to expose other tile-related settings, e.g. container, format.

Please open a new issue for any problems you discover. Thanks!

mvictoras · 2015-03-18T21:59:23Z

I have been busy, but am testing and have found some bugs.
I will get back to you by Friday with what I have found.

Victor

On Thu, Mar 12, 2015 at 10:50 AM, Lovell Fuller notifications@github.com
wrote:

@mvictoras https://github.com/mvictoras Are you able to help test the
new judgement v0.10.0 work-in-progress branch containing your Openslide
and Deep Zoom additions?

npm install lovell/sharp#judgement

In commit 5781a23
5781a23
I made a small public API change to combine tileSize and tileOverlap into
a single tile
https://github.com/lovell/sharp/tree/judgement#tilesize-overlap method
to slightly future-proof if/when someone wants to expose other tile-related
settings, e.g. container, format.

Please open a new issue for any problems you discover. Thanks!

—
Reply to this email directly or view it on GitHub
#146 (comment).

Victor Mateevitsi
PhD Candidate - University of Illinois at Chicago http://www.uic.edu/
Research Assistant - Electronic Visualization Laboratory
http://www.evl.uic.edu/
mvictoras@gmail.com
www http://www.vmateevitsi.com | LinkedIn
http://www.linkedin.com/pub/5/919/845 | Twitter
http://twitter.com/mvictoras | UIC http://www.uic.edu | EVL
http://www.evl.uic.edu

mvictoras · 2015-04-07T21:46:07Z

I have been testing the judgement branch and has been working for me just fine until now.
I have only found two issues:

For some files this line 208 at resize.cc:
if (image->Xsize * image->Ysize > baton->limitInputPixels)

returns false and therefore cannot open the file. It happens in general with smaller .ndpi files. Did not have a chance to investigate, so don't know why this is happening.

The other issue I had was with file naming. @jcupitt will probably know more about it.
In resize.cc we use
vips_dzsave(image, filename_no_extension.c_str(), ...

to save the image to DeepZoom file format and pass the filename without the extension as the "name" argument. What I found out is that if the filename contains periods, then vips_dzsave breaks. What happens is easier to explain with an example:

Let's say I am converting "this_is_a_test_file.03.08.14.ndpi" and I am passing as the name argument "this_is_a_test_file.03.08.14". Directory this_is_a_test_file.03.08.14 gets correctly created however inside the directory ".14" gets clipped from the filenames, so we only have:
this_is_a_test_file.03.08.dzi
this_is_a_test_file.03.08_files

When conversion is done, I get the error:
rename: unable to rename file "this_is_a_test_file.03.08/this_is_a_test_file.03.08.dzi" as "this_is_a_test_file.03.08.dzi", No such file or directory

So obviously, it thinks that ".40" is the extension, which is not! Any ideas?

lovell · 2015-04-08T11:14:49Z

The default value of limitInputPixels is set at https://github.com/lovell/sharp/blob/master/index.js#L17 with a 14-bit int maximum along each dimension. We'd need to remove the && limit <= maximum.pixels conditional from the end of https://github.com/lovell/sharp/blob/master/index.js#L407 to allow bigger images than this.

https://github.com/lovell/sharp/blob/judgement/src/resize.cc#L807 is removing the .dzi extension (might need to tighten that logic to ensure (length - 4) > 0) then it looks like https://github.com/jcupitt/libvips/blob/master/libvips/foreign/dzsave.c#L1585-L1586 is removing the last dot and anything after it from the "basename", i.e. the path-less filename.

Default limit of 14-bit dimensions remains

lovell · 2015-04-12T13:25:16Z

ae96814 should alleviate the limitInputPixels problem. You'll need to manually call that method with a new maximum value, but it no longer has a hard upper limit.

lovell · 2015-04-19T20:11:55Z

a065580 should fix the problem with filenames that contain dots.

sharp v0.10.0 will be published in the next few days, thanks again for your help @mvictoras.

lovell reviewed Jan 7, 2015
View reviewed changes

lovell added this to the v0.10.0 milestone Jan 23, 2015

lovell mentioned this pull request Feb 25, 2015

Intake #171

Closed

mvictoras force-pushed the master branch 5 times, most recently from 1e2d6c7 to a7615ab Compare March 6, 2015 03:33

lovell reviewed Mar 6, 2015
View reviewed changes

mvictoras reviewed Mar 6, 2015
View reviewed changes

Added Deep Zoom support.

506546f

Added OpenSuse 13.1 and 13.2 support in preinstall.sh script. Added OpenSlide support in preinstall script. Added unit tests for Deep Zoom and OpenSlide.

mvictoras force-pushed the master branch from 51daf36 to 506546f Compare March 6, 2015 18:02

lovell mentioned this pull request Mar 12, 2015

resize width is limited to 16383 #176

Closed

lovell closed this Mar 12, 2015

lovell added a commit that referenced this pull request Apr 12, 2015

Soften limitInputPixels upper limit #146

ae96814

Default limit of 14-bit dimensions remains

tlestang mentioned this pull request Jul 12, 2019

Convert slides to DeepZoom OxfordRSE/Electron-Sight#4

Closed

Added DeepZoom support #146

Added DeepZoom support #146

Conversation

mvictoras commented Jan 7, 2015

lovell commented Jan 7, 2015

lovell Jan 7, 2015

Choose a reason for hiding this comment

lovell commented Jan 9, 2015

mvictoras commented Jan 9, 2015

coveralls commented Jan 12, 2015

lovell commented Jan 23, 2015

mvictoras commented Jan 23, 2015

jcupitt commented Feb 12, 2015

lovell commented Feb 12, 2015

mvictoras commented Feb 12, 2015

mvictoras commented Feb 25, 2015

mvictoras commented Feb 25, 2015

jcupitt commented Feb 25, 2015

mvictoras commented Mar 6, 2015

mvictoras commented Mar 6, 2015

lovell Mar 6, 2015

Choose a reason for hiding this comment

mvictoras Mar 6, 2015

Choose a reason for hiding this comment

lovell commented Mar 6, 2015

lovell Mar 6, 2015

Choose a reason for hiding this comment

mvictoras Mar 6, 2015

Choose a reason for hiding this comment

lovell commented Mar 6, 2015

mvictoras Mar 6, 2015

Choose a reason for hiding this comment

lovell Mar 7, 2015

Choose a reason for hiding this comment

mvictoras commented Mar 6, 2015

mvictoras commented Mar 6, 2015

lovell commented Mar 7, 2015

jcupitt commented Mar 7, 2015

lovell commented Mar 10, 2015

mvictoras commented Mar 10, 2015

lovell commented Mar 12, 2015

lovell commented Mar 12, 2015

mvictoras commented Mar 18, 2015

mvictoras commented Apr 7, 2015

lovell commented Apr 8, 2015

lovell commented Apr 12, 2015

lovell commented Apr 19, 2015