Repository navigation
Add scalar image layer controls to the Rendering tab - #983
Conversation
✅ Deploy Preview for volview-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
49e48ca to
6e41e8b
Compare
7223d65 to
e2cff06
Compare
| const files = | ||
| 'urls' in sample | ||
| ? urlsToDataSources(sample.urls) | ||
| : [ | ||
| remoteFileToDataSource( | ||
| await fetchFile(sample.url, sample.filename, { progress }), | ||
| sample.url | ||
| ), | ||
| ]; |
There was a problem hiding this comment.
make a helper func or something to avoid long ternary with logic in second branch
There was a problem hiding this comment.
Fixed locally: extracted getSampleDataSources to replace the long ternary.
| :disabled=" | ||
| sample.isDownloading || (sample.isLoaded && !('urls' in sample)) | ||
| " |
There was a problem hiding this comment.
in general for this file, i'm I'm confused about why there's so much thrashing in this file just to add one new option that should be pretty standard. Just default loading of a list of files. But if you think this is simpler, all these file changes in this, in this component, we can keep it, but just take a hard look at it.
There was a problem hiding this comment.
PET/CT needs both import results and the normal auto-layering path. The remaining changes track both volumes and keep progress active until import finishes.
| watchImmediate( | ||
| [() => wlConfig.config.value?.visibility ?? true, imageData], | ||
| ([visible, image]) => { | ||
| sliceRep.actor.setVisibility(visible); | ||
| if (visible && image) { | ||
| // Hidden image actors are excluded when a layout resize fits clipping. | ||
| view.renderer.resetCameraClippingRange(image.getBounds()); | ||
| } | ||
| } | ||
| ); |
There was a problem hiding this comment.
For watch effect style, where we don't have to declare dependencies, if that's appropriate here.
There was a problem hiding this comment.
Kept the explicit watch: resetting clipping invokes camera callbacks that read reactive state. watchEffect would track those incidental reads too.
| sliceRep.mapper.set({ | ||
| viewSpecificProperties: { | ||
| OpenGL: { | ||
| ShaderReplacements: replacementValue |
There was a problem hiding this comment.
neat but its it striaghtforward to avoid doing shader replaments for this?
There was a problem hiding this comment.
These preserve preset opacity during background fading and apply layer opacity to RGBA images, which vtk.js currently ignores in its RGBA shader.
| : coloringConfig.value.transferFunction.mappingRange | ||
| ); | ||
| watch( | ||
| [colorPreset, () => mappingRange.value[0], () => mappingRange.value[1]], |
There was a problem hiding this comment.
watcheffect prefered if possible.
There was a problem hiding this comment.
Fixed locally: changed to watchEffect.
| } | ||
| const draftRange = computed(() => [ |
There was a problem hiding this comment.
whats up with the no space formating between fucntions and watches in this file?
There was a problem hiding this comment.
Fixed locally: added spacing between functions and watchers.
| ); | ||
| const emit = defineEmits<{ |
There was a problem hiding this comment.
space between big defs?
There was a problem hiding this comment.
Fixed locally: added spacing between the larger definitions.
| const validRange = ( | ||
| range: readonly [number, number] | undefined | ||
| ): range is readonly [number, number] => | ||
| !!range && | ||
| range.every(Number.isFinite) && | ||
| range[0] < range[1] && | ||
| Number.isFinite(range[1] - range[0]); | ||
| function setRange(range: [number, number]) { | ||
| if (coloringUnavailableReason.value || !validRange(range)) return; | ||
| layerColoringStore.updateDisplayConfig(props.layer.id, { | ||
| mappingRange: range, | ||
| }); | ||
| } |
There was a problem hiding this comment.
is this kind of code duplicated in the code base? wonder if there is factor out resue opporutnity here?
There was a problem hiding this comment.
Fixed locally: extracted isValidDisplayRange and reused it in the layer controls, contrast presets, range inputs, and slider.
| function addView(viewInit: ViewInfoInit) { | ||
| const id = idStore.nextId(); | ||
| let id = idStore.nextId(); | ||
| while (id in viewByID) id = idStore.nextId(); |
There was a problem hiding this comment.
is this while really needed?
There was a problem hiding this comment.
Yes. Restored view IDs can overlap several upcoming generated IDs; the loop prevents overwriting those views.
There was a problem hiding this comment.
do we really this this as a screeshot test? seems like some logical assertion would be simpler
There was a problem hiding this comment.
Fixed locally: replaced the image baseline with rendered-pixel assertions at 0% and 100% opacity.
Provide shared palette selection, numeric display limits, and a draggable three-handle range bar with Tab and arrow-key interaction.
Use shared controls for scalar palettes, display ranges, contrast, visibility, and overlay opacity. Apply base image settings in ordinary and oblique views, while preserving native RGB colors and RGBA alpha composition. Preserve display settings across view changes and saved sessions, including reset appearance and removed layers. Cover the controls, rendering, and saved state with behavior tests and document the display options.
Load the PET and CT URL collection through ordinary importing and automatic same-study layering. Reuse loaded DICOM chunks when reopening the sample. Add sample attribution, a thumbnail, and checksum-pinned browser fixtures without tracking binary datasets.
107f20e to
16c80a7
Compare
Adds shared color-map and display-range controls for scalar 2D base images and overlays, with visibility, overlay opacity, and transparent-background blending. Layer settings remain editable with slices off-screen and persist through layout changes and saved sessions. Existing multicomponent overlay rendering remains available, and range edits reuse unchanged opacity data.
Includes a PET/CT Chest example that uses the existing URL importer and automatic DICOM layering.