scijava-common icon indicating copy to clipboard operation
scijava-common copied to clipboard

Version 2.84.0 does not open tif images any more

Open sunsear opened this issue 4 years ago • 7 comments

With version 2.85.0 it is no longer possible to successfully open tif images. Larger images put the CPU at 100% for minutes, usually takes a second or so. Smaller images result in:

Schermafbeelding 2020-11-01 om 14 27 32

Steps to reproduce:

Succesful opening of images

Make a project with:

	<parent>
		<groupId>org.scijava</groupId>
		<artifactId>pom-scijava</artifactId>
		<version>29.2.1</version>
		<relativePath/>
	</parent>

It is now possible to use IJLauncher to open an image and do stuff with it.

Failed opening of images

  1. Add the property: <scijava-common.version>2.85.0</scijava-common.version>
  2. Start up the IJLauncher.
  3. Open an image
  4. CPU goes to 100% and image is not opened

Full project

This project, branch and pom reproduces the problem: https://github.com/sunsear/colour_deconvolution_IJ2/blob/feature/more_decimals/pom.xml

sunsear avatar Nov 01 '20 13:11 sunsear

Thanks @sunsear for reporting, I can reproduce the issue. For the record, here's what changed between scijava-common-2.83.3 (pinned in pom-scijava-29.2.1) and scijava-common-2.85.0:

https://github.com/scijava/scijava-common/compare/scijava-common-2.83.3...scijava-common-2.85.0

I suspect it might be the changes to (Default)IOService that make use of the new Location-based API in SCIFIO... @ctrueden @hinerm any insights?

imagejan avatar Nov 01 '20 15:11 imagejan

I did a little compare there, but there was so much change in IO related stuff that I couldn’t see what the relevant part would be. Hope @ctrueden knows!

sunsear avatar Nov 01 '20 18:11 sunsear

The reason is this commit added between 2.83.4 and 2.84.0:

Use Location, not String, in the I/O API (https://github.com/scijava/scijava-common/commit/b7285c5f807891097b5405b270952fcd32f2253c)

... and the fact that DatasetIOPlugin in SCIFIO wasn't adapted to these changes yet.

imagejan avatar Nov 01 '20 20:11 imagejan

@ctrueden , @imagejan , any progress on this? We are stuck with https://github.com/scijava/scijava-ui-swing/pull/54 because of this issue.

Look forward to hearing from you!

sunsear avatar Nov 12 '20 08:11 sunsear

@sunsear if I understand correctly, we have a binary version incompatibility issue here. Because AbstractIOPlugin changed its generic type from String to Location, the jars compiled against the old version of scijava-common will not work with newer versions. All we need is to recompile SCIFIO pinned to a newer dependency version of scijava-common. I did this in https://github.com/scifio/scifio/pull/444.

Can you test whether a scifio.jar file compiled from that branch works for you?

imagejan avatar Nov 13 '20 20:11 imagejan

@imagejan I just updated my project to SciJava pom 30.0.0 and after one build this problem poped up. Indeed if I simply replaced scijava-common version 2.85.0 with 2.83.3 everything worked again.

For me with scijava-common version 2.85.0, Open... no longer uses SCIFIO with scifio turned on. Also, drag and drop of extended file types on the toolbar doesn't allow for opening them.

My scifio is 0.41.1 so it should have your pull request @Scifio #444 but still I have this problem. Any tips? Anything I can test?

Will my custom types created by extending AbstractIOPlugin still work or will they need an update? Let me know if I should move some of these questions elsewhere.

karlduderstadt avatar Feb 03 '21 21:02 karlduderstadt

Actually, I am not sure it is a problem with SCIFIO. I set some breakpoints and found the problem seems to start in the DefaultLegacyOpener. it seems that the call to ioService.getOpener(path) always returns org.scijava.text.io.TextIOPlugin but for images it should return io.scif.io.DatasetIOPlugin (and in other cases, it should return other handlers).

The way things are written in scijava-common all handlers that do not implement supportsOpen(Location source) will always return false and org.scijava.text.io.TextIOPlugin is the only handler that actually implements that method for Locations.

So it seems to me there are two options

  1. The implementation of supportsOpen(Location source) needs to be enforced for all handlers.
  2. IOService needs to be updated so that getOpener(final String source) directly calls the string versions of supportsOpen methods

The above commit make the change in the IOService and appears to fix the problem for in my tests.

karlduderstadt avatar Feb 26 '21 15:02 karlduderstadt