-
Notifications
You must be signed in to change notification settings - Fork 45
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
Mccalluc/switch demos v2 #74
Conversation
dc0edfb
to
53d202f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ilan-gold : I had thought that we couldn't assume not zarr implies tiff? but if we can, great!
@@ -18,7 +17,7 @@ const defaultProps = { | |||
} = props.tile; | |||
const { sliderValues, data, colorValues } = props; | |||
const xrl = new XRLayer(props, { | |||
id: `XR-Layer-${west}-${south}-${east}-${north}`, | |||
id: `XR-Layer-${west}-${south}-${east}-${north}-${props.useTiff}`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Include useZarr
as well? I remember you saying we couldn't reduce it to a single boolean, because we may have sources which are neither tiff nor zarr.
@@ -18,7 +17,7 @@ const defaultProps = { | |||
} = props.tile; | |||
const { sliderValues, data, colorValues } = props; | |||
const xrl = new XRLayer(props, { | |||
id: `XR-Layer-${west}-${south}-${east}-${north}`, | |||
id: `XR-Layer-${west}-${south}-${east}-${north}-${props.useTiff}`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
id: `XR-Layer-${west}-${south}-${east}-${north}-${props.useTiff}`, | |
id: `XR-Layer-${west}-${south}-${east}-${north}-tiff_${props.useTiff}-zarr_${props.useZarr}`, |
}); | ||
} | ||
} | ||
} | ||
|
||
renderLayers() { | ||
// const useTiff = !(this.state.pool ^ this.state.isTiff); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// const useTiff = !(this.state.pool ^ this.state.isTiff); |
return new MicroscopyViewerLayer({ | ||
id: `MicroscopyViewerLayer-${useTiff ? 'tiff' : 'zarr'}`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, I don't think we're assuming not tiff implies zarr?
pool: new Pool(), | ||
isTiff: true, | ||
isZarr: false | ||
}); | ||
}); | ||
} else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
} else { | |
} else if (this.props.useZarr) { |
@@ -19,25 +21,37 @@ export class MicroscopyViewerLayer extends CompositeLayer { | |||
} | |||
|
|||
updateState() { | |||
if (!this.state.connections) { | |||
/* eslint-disable no-bitwise */ | |||
if ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me see if this can be more clear... I'll post a follow-up comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is what we're really interested in a discrepancy between state and props? (Also wondering about the necessity of having both state and props here, but set that to the side.) Would something like this work?:
if (this.props.useTiff && !this.state.isTiff) {
...
} else if (this.props.useZarr && !this.state.isZarr) {
...
}
Replaces #70. WIP: State toggling works, and the source is passed down, but I don't think Deck.gl is rerendering. @ilan-gold : Does something in MicroscopyViewerLayer need to change? Wanted to ping you before going too deep.