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

ND2 scanline pad fix #2868

Merged
merged 1 commit into from
Jun 23, 2017
Merged

ND2 scanline pad fix #2868

merged 1 commit into from
Jun 23, 2017

Conversation

sbesson
Copy link
Member

@sbesson sbesson commented Jun 13, 2017

See https://trello.com/c/TaqXNGco/152-nd2-pixel-offset.

From the Nikon ND2 samples in our repository, it seems like having an odd size both alongside the X dimension and the C dimension is necessary and sufficient to have the scanline padded.

This PR simplifies the NativeND2Reader.getScanlinePad() method to use this simpler calculation. To test this PR, check the automated tests keep passing against all ND2 samples. Additionally, test the sample file provided in the Fiji forum thread and check that without this PR, the image display with some horizontal lines while with this PR, it matches the expectation from NIS Elements.

Based on our existing data samples, only ND2 files with odd values for the
dimensions alongside X and C seem to require a non-zero scanline padding.
@sbesson sbesson changed the title ND2 scanline pad investigation ND2 scanline pad fix Jun 15, 2017
@sbesson sbesson removed the breaking label Jun 15, 2017
@dgault
Copy link
Member

dgault commented Jun 19, 2017

Testing the sample file from the forum thread shows that the scanlines are present without this PR but with the PR applied the file is read and displayed correctly. I additionally tested a number of other nd2 files from our repo and all continued to read and display correctly also.

The subset jobs are green and the new config file also passes testing - https://ci.openmicroscopy.org/job/BIOFORMATS-DEV-merge-repository-subset/444/

I have been trying to locate the addition of this logic originally and although the getScanlinePad function was added back in 2014 the logic itself was being used dating further back still.

Based on the sample files and information that we have this relaxation of the getScanlinePad rules appears to work as expected. Once the merge full job is back to normal this should be good to merge.

@dgault
Copy link
Member

dgault commented Jun 20, 2017

Daily builds and tests are all back to normal and previous issues were unrelated to this PR.

@sbesson sbesson merged commit e86b42d into ome:develop Jun 23, 2017
@sbesson sbesson deleted the nd2_scanline branch June 23, 2017 08:11
@sbesson sbesson added this to the 5.5.3 milestone Jun 23, 2017
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.

2 participants