Merge branch 'master' into zarr-sink

CHANGELOG.md

@@ -1,5 +1,17 @@
 # Change Log
 
+## 1.27.2
+
+### Improvements
+- Support range requests when downloading DICOMweb files ([#1444](../../pull/1444))
+- Bypass some scaling code when compositing multi sources ([#1447](../../pull/1447), [#1449](../../pull/1449))
+- Do not create needless alpha bands in the multi source ([#1451](../../pull/1451))
+- Infer DICOM file size, when possible ([#1448](../../pull/1448))
+- Swap styles faster in the frame viewer ([#1452](../../pull/1452))
+
+### Bug Fixes
+- Fix an issue with compositing sources in the multi source caused by alpha range ([#1453](../../pull/1453))
+
 ## 1.27.1
 
 ### Improvements

girder/girder_large_image/web_client/views/imageViewerWidget/geojs.js

@@ -175,10 +175,25 @@ var GeojsImageViewerWidget = ImageViewerWidget.extend({
         frame = frame || 0;
         this._nextframe = frame;
         this._nextstyle = style;
-        if ((frame !== this._frame || style !== this._style) && !this._updating) {
+        if ((frame !== this._frame || style !== this._style)) {
             this._frame = frame;
             this._style = style;
             this.trigger('g:imageFrameChanging', this, frame);
+            if (this._updating) {
+                this._layer2.url(this.getFrameAndUrl().url);
+                if (this._style === undefined) {
+                    if (this._layer2.setFrameQuad) {
+                        this._layer2.setFrameQuad(frame);
+                    }
+                    this._layer2.frame = frame;
+                } else {
+                    if (this._layer2.setFrameQuad) {
+                        this._layer2.setFrameQuad(undefined);
+                    }
+                    this._layer2.frame = undefined;
+                }
+                return;
+            }
             const quadLoaded = ((this._layer.setFrameQuad || {}).status || {}).loaded;
             if (quadLoaded && this._style === undefined) {
                 this._layer.url(this.getFrameAndUrl().url);
@@ -194,7 +209,7 @@ var GeojsImageViewerWidget = ImageViewerWidget.extend({
                 this._layer.frame = undefined;
             }
             this._updating = true;
-            this.viewer.onIdle(() => {
+            this._layer.onIdle(() => {
                 this._layer2.url(this.getFrameAndUrl().url);
                 if (this._style === undefined) {
                     if (this._layer2.setFrameQuad) {
@@ -207,7 +222,7 @@ var GeojsImageViewerWidget = ImageViewerWidget.extend({
                     }
                     this._layer2.frame = undefined;
                 }
-                this.viewer.onIdle(() => {
+                this._layer2.onIdle(() => {
                     if (this._layer.zIndex() > this._layer2.zIndex()) {
                         this._layer.moveDown();
                         if (!this._layer.options.keepLower) {

large_image/tilesource/base.py

@@ -1036,7 +1036,10 @@ def _applyStyle(  # noqa
                            np.full(image.shape[:2], 255, np.uint8))
                 sc.composite = entry.get('composite', 'multiply')
             if sc.band is None:
-                sc.band = image[:, :, sc.bandidx]
+                sc.band = image[
+                    :, :, sc.bandidx  # type: ignore[index]
+                    if sc.bandidx is not None and sc.bandidx < image.shape[2]  # type: ignore[misc]
+                    else 0]
             sc.band = self._applyStyleFunction(sc.band, sc, 'preband')
             sc.palette = getPaletteColors(entry.get(
                 'palette', ['#000', '#FFF']

large_image/tilesource/utilities.py

@@ -8,6 +8,7 @@
 from typing import Any, Dict, List, Optional, Set, Tuple, Union, cast
 
 import numpy as np
+import numpy.typing as npt
 import PIL
 import PIL.Image
 import PIL.ImageColor
@@ -729,16 +730,17 @@ def getAvailableNamedPalettes(includeColors: bool = True, reduced: bool = False)
     return sorted(palettes)
 
 
-def fullAlphaValue(arr: np.ndarray) -> int:
+def fullAlphaValue(arr: Union[np.ndarray, npt.DTypeLike]) -> int:
     """
     Given a numpy array, return the value that should be used for a fully
     opaque alpha channel.  For uint variants, this is the max value.
 
     :param arr: a numpy array.
     :returns: the value for the alpha channel.
     """
-    if arr.dtype.kind == 'u':
-        return np.iinfo(arr.dtype).max
+    dtype = arr.dtype if isinstance(arr, np.ndarray) else arr
+    if dtype.kind == 'u':
+        return np.iinfo(dtype).max
     return 1
 
 

sources/dicom/large_image_source_dicom/assetstore/dicomweb_assetstore_adapter.py

@@ -1,3 +1,4 @@
+import cherrypy
 import requests
 from large_image_source_dicom.dicom_tags import dicom_key_to_tag
 from large_image_source_dicom.dicomweb_utils import get_dicomweb_metadata
@@ -105,59 +106,145 @@ def deleteFile(self, file):
         # We don't actually need to do anything special
         pass
 
+    def setContentHeaders(self, file, offset, endByte, contentDisposition=None):
+        """
+        Sets the Content-Length, Content-Disposition, Content-Type, and also
+        the Content-Range header if this is a partial download.
+
+        :param file: The file being downloaded.
+        :param offset: The start byte of the download.
+        :type offset: int
+        :param endByte: The end byte of the download (non-inclusive).
+        :type endByte: int or None
+        :param contentDisposition: Content-Disposition response header
+            disposition-type value, if None, Content-Disposition will
+            be set to 'attachment; filename=$filename'.
+        :type contentDisposition: str or None
+        """
+        isRangeRequest = cherrypy.request.headers.get('Range')
+        setResponseHeader('Content-Type', file['mimeType'])
+        setContentDisposition(file['name'], contentDisposition or 'attachment')
+
+        if file.get('size') is not None:
+            # Only set Content-Length and range request headers if we have a file size
+            size = file['size']
+            if endByte is None or endByte > size:
+                endByte = size
+
+            setResponseHeader('Content-Length', max(endByte - offset, 0))
+
+            if offset or endByte < size or isRangeRequest:
+                setResponseHeader('Content-Range', f'bytes {offset}-{endByte - 1}/{size}')
+
     def downloadFile(self, file, offset=0, headers=True, endByte=None,
                      contentDisposition=None, extraParameters=None, **kwargs):
 
-        if offset != 0 or endByte is not None:
-            # FIXME: implement range requests
-            msg = 'Range requests are not yet implemented'
-            raise NotImplementedError(msg)
+        if headers:
+            setResponseHeader('Accept-Ranges', 'bytes')
+            self.setContentHeaders(file, offset, endByte, contentDisposition)
+
+        def stream():
+            # Perform the request
+            # Try a single-part download first. If that doesn't work, do multipart.
+            response = self._request_retrieve_instance_prefer_singlepart(file)
+
+            bytes_read = 0
+            for chunk in self._stream_retrieve_instance_response(response):
+                if bytes_read < offset:
+                    # We haven't reached the start of the offset yet
+                    bytes_needed = offset - bytes_read
+                    if bytes_needed >= len(chunk):
+                        # Skip over the whole chunk...
+                        bytes_read += len(chunk)
+                        continue
+                    else:
+                        # Discard all bytes before the offset
+                        chunk = chunk[bytes_needed:]
+                        bytes_read += bytes_needed
+
+                if endByte is not None and bytes_read + len(chunk) >= endByte:
+                    # We have reached the end... remove all bytes after endByte
+                    chunk = chunk[:endByte - bytes_read]
+                    if chunk:
+                        yield chunk
+
+                    bytes_read += len(chunk)
+                    break
+
+                yield chunk
+                bytes_read += len(chunk)
+
+        return stream
+
+    def _request_retrieve_instance_prefer_singlepart(self, file, transfer_syntax='*'):
+        # Try to perform a singlepart request. If it fails, perform a multipart request
+        # instead.
+        response = None
+        try:
+            response = self._request_retrieve_instance(file, multipart=False,
+                                                       transfer_syntax=transfer_syntax)
+        except requests.HTTPError:
+            # If there is an HTTPError, the server might not accept single-part requests...
+            pass
+
+        if self._is_singlepart_response(response):
+            return response
+
+        # Perform the multipart request instead
+        return self._request_retrieve_instance(file, transfer_syntax=transfer_syntax)
+
+    def _request_retrieve_instance(self, file, multipart=True, transfer_syntax='*'):
+        # Multipart requests are officially supported by the DICOMweb standard.
+        # Singlepart requests are not officially supported, but they are easier
+        # to work with.
+        # Google Healthcare API support it.
+        # See here: https://cloud.google.com/healthcare-api/docs/dicom#dicom_instances
 
+        # Create the URL
+        client = _create_dicomweb_client(self.assetstore_meta)
+        url = self._create_retrieve_instance_url(client, file)
+
+        # Build the headers
+        headers = {}
+        if multipart:
+            # This is officially supported by the DICOMweb standard.
+            headers['Accept'] = '; '.join((
+                'multipart/related',
+                'type="application/dicom"',
+                f'transfer-syntax={transfer_syntax}',
+            ))
+        else:
+            # This is not officially supported by the DICOMweb standard,
+            # but it is easier to work with, and some servers such as
+            # Google Healthcare API support it.
+            # See here: https://cloud.google.com/healthcare-api/docs/dicom#dicom_instances
+            headers['Accept'] = f'application/dicom; transfer-syntax={transfer_syntax}'
+
+        return client._http_get(url, headers=headers, stream=True)
+
+    def _create_retrieve_instance_url(self, client, file):
         from dicomweb_client.web import _Transaction
 
         dicom_uids = file['dicom_uids']
         study_uid = dicom_uids['study_uid']
         series_uid = dicom_uids['series_uid']
         instance_uid = dicom_uids['instance_uid']
 
-        client = _create_dicomweb_client(self.assetstore_meta)
-
-        if headers:
-            setResponseHeader('Content-Type', file['mimeType'])
-            setContentDisposition(file['name'], contentDisposition or 'attachment')
-
-            # The filesystem assetstore calls the following function, which sets
-            # the above and also sets the range and content-length headers:
-            # `self.setContentHeaders(file, offset, endByte, contentDisposition)`
-            # However, we can't call that since we don't have a great way of
-            # determining the DICOM file size without downloading the whole thing.
-            # FIXME: call that function if we find a way to determine file size.
-
-        # Create the URL
-        url = client._get_instances_url(
+        return client._get_instances_url(
             _Transaction.RETRIEVE,
             study_uid,
             series_uid,
             instance_uid,
         )
 
-        # Build the headers
-        transfer_syntax = '*'
-        accept_parts = [
-            'multipart/related',
-            'type="application/dicom"',
-            f'transfer-syntax={transfer_syntax}',
-        ]
-        headers = {
-            'Accept': '; '.join(accept_parts),
-        }
-
-        def stream():
-            # Perform the request
-            response = client._http_get(url, headers=headers, stream=True)
-            yield from self._stream_retrieve_instance_response(response)
-
-        return stream
+    def _stream_retrieve_instance_response(self, response):
+        # Check if the original request asked for multipart data
+        if 'multipart/related' in response.request.headers.get('Accept', ''):
+            yield from self._stream_dicom_multipart_response(response)
+        else:
+            # The content should *only* contain the DICOM file
+            with response:
+                yield from response.iter_content(BUF_SIZE)
 
     def _extract_media_type_and_boundary(self, response):
         content_type = response.headers['content-type']
@@ -171,7 +258,7 @@ def _extract_media_type_and_boundary(self, response):
 
         return media_type, boundary
 
-    def _stream_retrieve_instance_response(self, response):
+    def _stream_dicom_multipart_response(self, response):
         # The first part of this function was largely copied from dicomweb-client's
         # _decode_multipart_message() function. But we can't use that function here
         # because it relies on reading the whole DICOM file into memory. We want to
@@ -263,6 +350,50 @@ def _stream_retrieve_instance_response(self, response):
             msg = 'Failed to find ending boundary in response content'
             raise ValueError(msg)
 
+    def _infer_file_size(self, file):
+        # Try various methods to infer the file size, without streaming the
+        # whole file. Returns the file size if successful, or `None` if unsuccessful.
+        if file.get('size') is not None:
+            # The file size was already determined.
+            return file['size']
+
+        # Only method currently is inferring from single-part content_length
+        return self._infer_file_size_singlepart_content_length(file)
+
+    def _is_singlepart_response(self, response):
+        if response is None:
+            return False
+
+        content_type = response.headers.get('Content-Type')
+        return (
+            response.status_code == 200 and
+            not any(x in content_type for x in ('multipart/related', 'boundary'))
+        )
+
+    def _infer_file_size_singlepart_content_length(self, file):
+        # First, try to see if single-part requests work, and if the Content-Length
+        # is returned. This works for Google Healthcare API.
+        try:
+            response = self._request_retrieve_instance(file, multipart=False)
+        except requests.HTTPError:
+            # If there is an HTTPError, the server might not accept single-part requests...
+            return
+
+        if not self._is_singlepart_response(response):
+            # Does not support single-part requests...
+            return
+
+        content_length = response.headers.get('Content-Length')
+        if not content_length:
+            # The server did not return a Content-Length
+            return
+
+        try:
+            # The DICOM file size is equal to the Content-Length
+            return int(content_length)
+        except ValueError:
+            return
+
     def importData(self, parent, parentType, params, progress, user, **kwargs):
         """
         Import DICOMweb WSI instances from a DICOMweb server.
@@ -364,7 +495,10 @@ def importData(self, parent, parentType, params, progress, user, **kwargs):
                     'instance_uid': instance_uid,
                 }
                 file['imported'] = True
-                File().save(file)
+
+                # Try to infer the file size without streaming, if possible.
+                file['size'] = self._infer_file_size(file)
+                file = File().save(file)
 
             items.append(item)
 
@@ -374,6 +508,30 @@ def importData(self, parent, parentType, params, progress, user, **kwargs):
     def auth_session(self):
         return _create_auth_session(self.assetstore_meta)
 
+    def getFileSize(self, file):
+        # This function will compute the size of the DICOM file (a potentially
+        # expensive operation, since it may have to stream the whole file).
+        # The caller is expected to cache the result in file['size'].
+        # This function is called when the size is needed, such as the girder
+        # fuse mount code, and range requests.
+        if file.get('size') is not None:
+            # It has already been computed once. Return the cached size.
+            return file['size']
+
+        # Try to infer the file size without streaming, if possible.
+        size = self._infer_file_size(file)
+        if size:
+            return size
+
+        # We must stream the whole file to get the file size...
+        size = 0
+        response = self._request_retrieve_instance_prefer_singlepart(file)
+        for chunk in self._stream_retrieve_instance_response(response):
+            size += len(chunk)
+
+        # This should get cached in file['size'] in File().updateSize().
+        return size
+
 
 def _create_auth_session(meta):
     auth_type = meta.get('auth_type')