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

Fix downsampling issue #69

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix downsampling issue #69

wants to merge 1 commit into from

Conversation

lguerard
Copy link

@lguerard lguerard commented Oct 27, 2022

Just changing this line fixes the issue with the TileConfiguration reported here : https://forum.image.sc/t/fusion-of-label-images-using-downsampling/73150

Closes #38.

Just changing this line fixes the issue with the TileConfiguration reported here : https://forum.image.sc/t/fusion-of-label-images-using-downsampling/73150
@lguerard
Copy link
Author

@imagejan Thanks for cleaning up a bit the PR ;)

However, I'm not sure if that closes the issue aryehw was mentioning, but it at least closes mine. (I probably should have made a different issue for this, sorry..)

@imagejan
Copy link
Member

Good point, @lguerard, sorry, I maybe was a bit fast to add the "Closes #38" to your PR description.

#38 actually talks about "Filename defined position", which is grid type 4 according to here:

"Filename defined position", // 4

So how about relaxing to gridType >= 4 even?

@lguerard
Copy link
Author

I would be fine but I don't have any files to test the other types which is why I asked if there was a reason to keep it only for 5 and 7.

@imagejan
Copy link
Member

@lguerard wrote in #38 (comment)

Was there a reason to only do the downsampling if GridType was 5 or 7 ?

The Downsampler logic was introduced with f37c98f.

There's actually two code paths, for grid type {5,7} and for all others:

  • !( gridType == 5 || gridType == 7) (note the ds arguments in the function calls):

    Downsampler ds = null;
    if ( downSample && !( gridType == 5 || gridType == 7) ) ds = new Downsampler();
    if ( gridType < 5 )
    elements = getGridLayout( grid, gridSizeX, gridSizeY, overlapX, overlapY, directory, filenames, startI, startX, startY, params.virtual, ds );
    //John Lapage modified this: copying setup for Unknown Positions
    else if ( gridType == 5 || gridType == 7)
    elements = getAllFilesInDirectory( directory, confirmFiles );
    else if ( gridType == 6 && gridOrder == 1 ) // positions from file metadata
    elements = getLayoutFromMultiSeriesFile( seriesFile, increaseOverlap,
    ignoreCalibration, invertX, invertY, ignoreZStage, ds );
    else if ( gridType == 6 ) // positions from tile configuration file
    elements = getLayoutFromFile( directory, outputFile, ds );

  • ( gridType == 5 || gridType == 7)

    if (downSample && ( gridType == 5 || gridType == 7)) {
    if (ds == null) {
    ds = new Downsampler();
    ds.getInput(imp.getWidth(), imp.getHeight());
    }
    ds.run(imp);
    }

So while the fix might work for your case, @lguerard, I believe there was some other breakage since f37c98f. It would be good to have unit test cases for this 😄

@lguerard
Copy link
Author

Indeed, sorry about this. I'll investigate a bit more and update this PR when I have found something !

@lguerard
Copy link
Author

lguerard commented Oct 27, 2022

Actually, I think I was right.

If I understand correctly what the ImageCollection corresponds to, this part

else if ( gridType == 6 ) // positions from tile configuration file
elements = getLayoutFromFile( directory, outputFile, ds );

only changes the grid elements which corresponds to the position of the tiles no ? The real downsampling of the ImagePlus is happening here and only if the GridType is 5 or 7.
if (downSample && ( gridType == 5 || gridType == 7)) {
if (ds == null) {
ds = new Downsampler();
ds.getInput(imp.getWidth(), imp.getHeight());
}
ds.run(imp);
}

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

Successfully merging this pull request may close these issues.

"downsample tiles" corrupts stitching
2 participants