[Thanatos9404]

Fixes #1085

When a layer has no type, Neuroglancer creates an AutoUserLayer and swaps it for the detected type once the data source loads. changeLayerType rebuilt the new layer from AutoUserLayer.toJSON(), which only has the generic layer state, so type-specific settings were lost. In the issue's example the image layer started with the default opacity.

• AutoUserLayer keeps the specification it was restored from and passes it to changeLayerType as an optional baseSpecification, used for properties the auto layer doesn't set. If an invalid value makes the new layer's restoreState throw, the new layer is disposed and the auto layer stays.
• The multi-channel image setup now only applies its opacity, blend and volumeRenderingDepthSamples defaults when the specification doesn't set them. A shader in the specification is kept, and the multi-channel color and contrast defaults are skipped in that case.

Testing: the new src/layer/index.browser_test.ts simulates type detection with stub data sources. Its checks fail on master, except the one for the multi-channel defaults, and pass with this change.

Not changed: the OMERO part of the issue (explicit "type": "image") is left for a separate change.

[seankmartin]

This looks good thanks, I've left one comment as it's fine with me but could affect someone using it so in case they run into the issue they can see it here

[seankmartin]

Previously since these settings were ignored the auto layer creation would have continued even with incorrect additional state. Now these layers would get rejected instead of creating. I'm ok with the rejection, but if it ends up affecting users we may want to instead drop parts of the state which are not compatible.

[Thanatos9404]

Thanks, that makes sense. If the rejection does end up affecting anyone, dropping only the parts of the state that don't fit the detected type would be a small follow-up, and I'm happy to do it.

[chrisj]

thanks for this fix! looks good other than my one comment about keeping user shader

[chrisj]

if a custom user shader is set, this line overrides it
visit https://neuroglancer--pr1135-hqp87jqf.web.app/
load this state:

{
  "dimensions": {
    "x": [8e-9, "m"],
    "y": [8e-9, "m"],
    "z": [8e-9, "m"]
  },
  "position": [128, 128, 128],
  "crossSectionScale": 8,
  "layers": [
    {
      "name": "shader-repro",
      "source": "precomputed://gs://neuroglancer-public-data/flyem_fib-25/image",
      "opacity": 1,
      "shader": "void main() { emitRGB(vec3(1.0, 0.0, 0.0)); }"
    }
  ],
  "layout": "xy"
}

[Thanatos9404]

Good catch, thanks. In 6c16903 a shader given in the layer state is kept, and the color and contrast defaults are skipped in that case, since a custom shader may not have those controls. I added a test with the shader from your repro. For a source with several channels, each channel layer gets the same shader.

[jbms]

I'm not sure if this is a good idea --- I think if we go with this we'd want to explicitly guarantee that all layer types assign the same meaning to a given attribute that applies to more than one layer type. Just allowing specific attributes may be better. Or an explicit auto image layer type.

[Thanatos9404]

Fair point. shader and shaderControls already mean different things on image, annotation and single mesh layers, so passing the whole spec through could hand an image shader to an annotation layer. I could switch to an allowlist instead: carry over only image settings such as opacity, blend, shader and shaderControls, and only when the detected type is image, which still covers the OME-Zarr case in #1085. Would that work for you, or would you rather have an explicit auto image type?