Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Version 2.84.0 does not open tif images any more #406

Open
sunsear opened this issue Nov 1, 2020 · 7 comments
Open

Version 2.84.0 does not open tif images any more #406

sunsear opened this issue Nov 1, 2020 · 7 comments

Comments

@sunsear
Copy link

sunsear commented Nov 1, 2020

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

@imagejan
Copy link
Member

imagejan commented Nov 1, 2020

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:

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?

@sunsear
Copy link
Author

sunsear commented Nov 1, 2020

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!

@imagejan
Copy link
Member

imagejan commented Nov 1, 2020

The reason is this commit added between 2.83.4 and 2.84.0:

Use Location, not String, in the I/O API (b7285c5)

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

@imagejan imagejan changed the title Version 2.85.0 does not open tif images any more Version 2.84.0 does not open tif images any more Nov 1, 2020
@sunsear
Copy link
Author

sunsear commented Nov 12, 2020

@ctrueden , @imagejan , any progress on this? We are stuck with scijava/scijava-ui-swing#54 because of this issue.

Look forward to hearing from you!

imagejan added a commit to scifio/scifio that referenced this issue Nov 13, 2020
See scijava/scijava-common#406.

With scijava-common 2.84.0, the generic typing of AbstractIOPlugin has changed from String to Location. While the String-based methods were kept for compatibility, we have to re-compile SCIFIO with a newer dependency to restore binary compatibility.
@imagejan
Copy link
Member

@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 scifio/scifio#444.

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

@karlduderstadt
Copy link

karlduderstadt commented Feb 3, 2021

@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 added a commit to karlduderstadt/scijava-common that referenced this issue Feb 26, 2021
…ring)

This commit fixes an issue caused by calling the Location version of the
method supportsOpen() in all cases when many IOPlugin do not yet
implement that Location version of that method and therefore always
return false. Instead now IOService getOpener(string) directly calls the
string version of supportsOpen. This ensures older IOPlugin that can
support the file provided are discovered and used.
@karlduderstadt
Copy link

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.

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

No branches or pull requests

3 participants