Skip to content

Fix Interleaved RGB Image Handling. #399

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

Merged
merged 5 commits into from
Mar 31, 2021

Conversation

ilan-gold
Copy link
Collaborator

@ilan-gold ilan-gold commented Mar 29, 2021

Our current handling of RGB interleaved images doesn't work at all, partially because Avivator tries sending in an array of channel selections (whereas an interleaved RGB loader only has one selection available to it) and partially because isInterleaved was always passed in the PixelSource instead of its shape.

Background

After #358 I don't think we tested this (or if we did, we did not do it well enough). I will slack you a test URL @manzt - thanks @NHPatterson for bringing this to our attention.

Change List

  • Make sure isInterleaved is passed in a shape not the PixelSource itself.
  • Don't make extra selections in Avivator that an interleaved loader cannot handle.

Checklist

  • Update JSdoc types if there is any API change.
  • Make sure Avivator works as expected with your change.

Copy link
Member

@manzt manzt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! I experimented with interleaved data for zarr a couple weeks ago an ran into a similar issue. One thing I vaguely remember from that is the BitmapLayer failing with no default value... can you test this if you don't provide a photometric interpretation?

Comment on lines 159 to 161
const {
meta: { photometricInterpretation }
} = loader;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if no photometricInterpretation is provided? Is there a default? This is not included in Zarr, so it would be good to have a default:

const { photometricInterpretation = ?? } = loader.meta;

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call, I'll add a default.

if (isInterleaved(base.shape)) {
const {
meta: { photometricInterpretation }
} = base;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same question here.

* Add test for BitmapLayer

* Add more stringent requirement for layer.

* Fix slicing bug.

* Fix tests better.

* Small fixes.

* ImageLayer too.
@ilan-gold ilan-gold merged commit b912f55 into master Mar 31, 2021
@ilan-gold ilan-gold deleted the ilan-gold/fix_interleaved_rgb_bugs branch March 31, 2021 16:33
ilan-gold added a commit that referenced this pull request May 26, 2021
* Fix Interleaved RGB Image Handling. (#399)

* Fix Interleaved RGB Image Handling.

* Prettier.

* Defaults for photometricInterpretation

* Add test for Interleaved BitmapLayer (#400)

* Add test for BitmapLayer

* Add more stringent requirement for layer.

* Fix slicing bug.

* Fix tests better.

* Small fixes.

* ImageLayer too.

* 0.9.4 (#405)

* Fix type/docs for VivView(er) and Consumers. (#407)

* Fix type/docs for VivViewer and VivView (and upstream users)

* Use special jsdoc -> typescript syntax.

* Small Fixes

* Fix ImageLayer

* Try functional component

* Small fixes.

* Small fixes.

* Fix `ImageLayer` `loader` type. (#410)

* Improve emitted TS types for layers (#413)

* Create `Viv` utility type to refine emitted Layer constructor types.

* Do not export VivProps type.

* Fix Empty Loader Selection Bug. (#416)

* 0.9.5 (#417)

* Fix Bugs in `getChannelStats`. (#419)

* Fix Bugs in `getChannelStats`.

* Use > 0 instead of epsilon value.

* Check all properties in onHover to avoid crashes (#426)

* Bump url-parse from 1.4.7 to 1.5.1 in /avivator (#427)

Bumps [url-parse](https://github.com/unshiftio/url-parse) from 1.4.7 to 1.5.1.
- [Release notes](https://github.com/unshiftio/url-parse/releases)
- [Commits](unshiftio/url-parse@1.4.7...1.5.1)

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump hosted-git-info from 2.8.8 to 2.8.9 (#429)

Bumps [hosted-git-info](https://github.com/npm/hosted-git-info) from 2.8.8 to 2.8.9.
- [Release notes](https://github.com/npm/hosted-git-info/releases)
- [Changelog](https://github.com/npm/hosted-git-info/blob/v2.8.9/CHANGELOG.md)
- [Commits](npm/hosted-git-info@v2.8.8...v2.8.9)

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump lodash from 4.17.20 to 4.17.21 in /avivator (#431)

Bumps [lodash](https://github.com/lodash/lodash) from 4.17.20 to 4.17.21.
- [Release notes](https://github.com/lodash/lodash/releases)
- [Commits](lodash/lodash@4.17.20...4.17.21)

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump postcss from 8.1.4 to 8.2.15 in /avivator (#432)

Bumps [postcss](https://github.com/postcss/postcss) from 8.1.4 to 8.2.15.
- [Release notes](https://github.com/postcss/postcss/releases)
- [Changelog](https://github.com/postcss/postcss/blob/main/CHANGELOG.md)
- [Commits](postcss/postcss@8.1.4...8.2.15)

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump lodash from 4.17.20 to 4.17.21 (#428)

Bumps [lodash](https://github.com/lodash/lodash) from 4.17.20 to 4.17.21.
- [Release notes](https://github.com/lodash/lodash/releases)
- [Commits](lodash/lodash@4.17.20...4.17.21)

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Check for dependabot before running CHANGELOG checks (#433)

* New README Screenshots (#435)

* Add Screenshots for README

* Change width.

* Update URLs

* Allow for LINEAR/NEAREST Filtering Interpolation Choice (#437)

* Allow for LINEAR/NEAREST Filtering Interpolation Choice

* Change variable name

* Change filtering options.

* Bump browserslist from 4.16.1 to 4.16.6 (#441)

Bumps [browserslist](https://github.com/browserslist/browserslist) from 4.16.1 to 4.16.6.
- [Release notes](https://github.com/browserslist/browserslist/releases)
- [Changelog](https://github.com/browserslist/browserslist/blob/main/CHANGELOG.md)
- [Commits](browserslist/browserslist@4.16.1...4.16.6)

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump browserslist from 4.14.6 to 4.16.6 in /avivator (#442)

Bumps [browserslist](https://github.com/browserslist/browserslist) from 4.14.6 to 4.16.6.
- [Release notes](https://github.com/browserslist/browserslist/releases)
- [Changelog](https://github.com/browserslist/browserslist/blob/main/CHANGELOG.md)
- [Commits](browserslist/browserslist@4.14.6...4.16.6)

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

Co-authored-by: Trevor Manz <[email protected]>
Co-authored-by: Andreas Girgensohn <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
ilan-gold added a commit that referenced this pull request May 26, 2021
* Fix Interleaved RGB Image Handling. (#399)

* Fix Interleaved RGB Image Handling.

* Prettier.

* Defaults for photometricInterpretation

* Add test for Interleaved BitmapLayer (#400)

* Add test for BitmapLayer

* Add more stringent requirement for layer.

* Fix slicing bug.

* Fix tests better.

* Small fixes.

* ImageLayer too.

* 0.9.4 (#405)

* Fix type/docs for VivView(er) and Consumers. (#407)

* Fix type/docs for VivViewer and VivView (and upstream users)

* Use special jsdoc -> typescript syntax.

* Small Fixes

* Fix ImageLayer

* Try functional component

* Small fixes.

* Small fixes.

* Fix `ImageLayer` `loader` type. (#410)

* Improve emitted TS types for layers (#413)

* Create `Viv` utility type to refine emitted Layer constructor types.

* Do not export VivProps type.

* Fix Empty Loader Selection Bug. (#416)

* 0.9.5 (#417)

* Fix Bugs in `getChannelStats`. (#419)

* Fix Bugs in `getChannelStats`.

* Use > 0 instead of epsilon value.

* Check all properties in onHover to avoid crashes (#426)

* Bump url-parse from 1.4.7 to 1.5.1 in /avivator (#427)

Bumps [url-parse](https://github.com/unshiftio/url-parse) from 1.4.7 to 1.5.1.
- [Release notes](https://github.com/unshiftio/url-parse/releases)
- [Commits](unshiftio/url-parse@1.4.7...1.5.1)

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump hosted-git-info from 2.8.8 to 2.8.9 (#429)

Bumps [hosted-git-info](https://github.com/npm/hosted-git-info) from 2.8.8 to 2.8.9.
- [Release notes](https://github.com/npm/hosted-git-info/releases)
- [Changelog](https://github.com/npm/hosted-git-info/blob/v2.8.9/CHANGELOG.md)
- [Commits](npm/hosted-git-info@v2.8.8...v2.8.9)

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump lodash from 4.17.20 to 4.17.21 in /avivator (#431)

Bumps [lodash](https://github.com/lodash/lodash) from 4.17.20 to 4.17.21.
- [Release notes](https://github.com/lodash/lodash/releases)
- [Commits](lodash/lodash@4.17.20...4.17.21)

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump postcss from 8.1.4 to 8.2.15 in /avivator (#432)

Bumps [postcss](https://github.com/postcss/postcss) from 8.1.4 to 8.2.15.
- [Release notes](https://github.com/postcss/postcss/releases)
- [Changelog](https://github.com/postcss/postcss/blob/main/CHANGELOG.md)
- [Commits](postcss/postcss@8.1.4...8.2.15)

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump lodash from 4.17.20 to 4.17.21 (#428)

Bumps [lodash](https://github.com/lodash/lodash) from 4.17.20 to 4.17.21.
- [Release notes](https://github.com/lodash/lodash/releases)
- [Commits](lodash/lodash@4.17.20...4.17.21)

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Check for dependabot before running CHANGELOG checks (#433)

* New README Screenshots (#435)

* Add Screenshots for README

* Change width.

* Update URLs

* Allow for LINEAR/NEAREST Filtering Interpolation Choice (#437)

* Allow for LINEAR/NEAREST Filtering Interpolation Choice

* Change variable name

* Change filtering options.

* Bump browserslist from 4.16.1 to 4.16.6 (#441)

Bumps [browserslist](https://github.com/browserslist/browserslist) from 4.16.1 to 4.16.6.
- [Release notes](https://github.com/browserslist/browserslist/releases)
- [Changelog](https://github.com/browserslist/browserslist/blob/main/CHANGELOG.md)
- [Commits](browserslist/browserslist@4.16.1...4.16.6)

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump browserslist from 4.14.6 to 4.16.6 in /avivator (#442)

Bumps [browserslist](https://github.com/browserslist/browserslist) from 4.14.6 to 4.16.6.
- [Release notes](https://github.com/browserslist/browserslist/releases)
- [Changelog](https://github.com/browserslist/browserslist/blob/main/CHANGELOG.md)
- [Commits](browserslist/browserslist@4.14.6...4.16.6)

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

Co-authored-by: Trevor Manz <[email protected]>
Co-authored-by: Andreas Girgensohn <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
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