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

Support uint16 color layers #4152

Merged
merged 6 commits into from Jul 15, 2019

Conversation

@daniel-wer
Copy link
Contributor

commented Jun 25, 2019

This PR adds uint16 color layer support. Histograms are not working correctly yet, very related to #4095.

Backend ToDo (highest priority first):

  • the backend doesn't send enough data for uint16 color layer buckets, looks like it automatically assumes uint8
  • Histogram data for uint16 color layers is not correct yet. Can probably be done as part of #4095.
  • the backend doesn't detect uint16 color layers correctly when importing the dataset (detects uint8 instead)

URL of deployed dev instance (used for testing):

Steps to test:

  • Download the data_types dataset from our wiki which contains a uint16 layer.

Issues:


daniel-wer added 2 commits Jun 25, 2019

@daniel-wer daniel-wer requested a review from philippotto Jul 11, 2019

@daniel-wer daniel-wer changed the title [WIP] Support uint16 color layers Support uint16 color layers Jul 11, 2019

@philippotto
Copy link
Member

left a comment

Awesome refactoring ❤️ Cool to see how the previous float integration enabled this lightweight implementation!

Regarding the histogram: Maybe disable the histogram rendering for uint16 layer (like it's done for float currently). Or do you want to wait with merging until the histogram support is completed?

@daniel-wer

This comment has been minimized.

Copy link
Contributor Author

commented Jul 15, 2019

I think I'll disable the histogram for now :)

@daniel-wer

This comment has been minimized.

Copy link
Contributor Author

commented Jul 15, 2019

I've disabled the histogram for all layers but (u)int8 for now. Although it was enabled for uint24 layers before, what was displayed was definitely not correct.

@@ -135,7 +135,7 @@ class DatasetSettings extends React.PureComponent<DatasetSettingsProps, State> {
const { alpha, color, intensityRange, isDisabled } = layer;
let histogram = new Array(256).fill(0);
if (this.state.histograms && this.state.histograms[layerName]) {
histogram = this.state.histograms[layerName].histogram;
({ histogram } = this.state.histograms[layerName]);

This comment has been minimized.

Copy link
@philippotto

philippotto Jul 15, 2019

Member

Interesting, didn't know that syntax for already defined variables!

@daniel-wer daniel-wer merged commit f2cc469 into master Jul 15, 2019

2 checks passed

ci/circleci: build_test_deploy Your tests passed on CircleCI!
Details
license/cla Contributor License Agreement is signed.
Details

@daniel-wer daniel-wer deleted the support-uint16-color branch Jul 15, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.