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

New rendering settings specification #55

Open
dominikl opened this issue Oct 7, 2021 · 69 comments
Open

New rendering settings specification #55

dominikl opened this issue Oct 7, 2021 · 69 comments

Comments

@dominikl
Copy link
Member

dominikl commented Oct 7, 2021

The render plugin is a good starting point to play around with different ideas. Wonder if we should have an 'experimental' branch maybe? /cc @jburel @dgault @francesw

Here's a first attempt, kind of a hybrid of the current render plugin output and the current NGFF draft specification
(code see draft PR #54 ):

{
    "channels": {
        "0": {
            "active": true,
            "color": "00FF00",
            "color-format": "hex",
            "color-type": "rgb",
            "label": "CEP192-M",
            "window": {
                "end": 16633.45703125,
                "max": 16633.45703125,
                "min": 0.0,
                "start": 0.0
            }
        },
        "1": {
            "active": true,
            "color": "FF0000",
            "color-format": "hex",
            "color-type": "rgb",
            "label": "CENT2",
            "window": {
                "end": 103353.1796875,
                "max": 103353.1796875,
                "min": 0.0,
                "start": 0.0
            }
        }
    },
    "color-model": "color",
    "default-dimension-0": 0,
    "default-dimension-1": 0,
    "dimensions": "zt",
    "plane": "xy",
    "version": 3
}

Instead of defaultZ/T it's more generic default-dimension- and what the dimensions actually mean is specified in dimensions, what a plane actually means likewise in plane. Also the color can be more generic. See above for the common html color specification, but it could also hold lookup tables (e.g. specified by a name or maybe url, etc.), e.g.

    "channels": {
        "0": {
            "active": true,
            "color": "glasbey",
            "color-format": "name",
            "color-type": "lut",
            "label": "CEP192-M",
            "window": {
                "end": 23900.765625,
                "max": 23900.765625,
                "min": 0.0,
                "start": 0.0
            }
        },
        ...
    }

Edit: Actually default-dimension- should better be a list, not several keys.

@jburel
Copy link
Member

jburel commented Oct 7, 2021

other elements like mapping family needs to be added: this assumes linear mapping
This could also be an opportunity to re-activate the "co-domain" transformations i.e. changes in the device space

Introducing an experimental branch will make sense otherwise we will have a long open PR

@sbesson
Copy link
Member

sbesson commented Oct 7, 2021

A few high-level thoughts, I think the development workflow will come across several repositories (ome/omero-cli-zarr#88 is another example). Completely fine from my side with having an experimental/development branch where PRs get integrated more quickly.

A few additional considerations:

  • wherever appropriate we might still want to be upfront about the in-progres nature of the work. For instance, suffixing the spec version with a dev suffix or similar
  • even if working from an integration branch, I think we might want to go for pre-releases/dev releases that we can start consuming internally

@will-moore
Copy link
Member

Maybe for default dimensions, this might be nicer:

"default_dimensions" : {
  "0": 10,
  "2": 2
}

Also, note the discussion of naming style at ome/ngff#54
Strong preference for color_format over color-format etc.

@jburel
Copy link
Member

jburel commented Oct 8, 2021

for the naming discussion, I am wondering if we should not follow the schema.org rule i.e. colorFormat

@jburel
Copy link
Member

jburel commented Oct 8, 2021

concept of group should be supported. We will need support for multiple groups

group : {
  "name":  "group1",
  "visible": True,
  "indexes": "1, 2, 5",
  "type": "channel" (could be angle)
}

@jburel
Copy link
Member

jburel commented Oct 8, 2021

Looking at the BDV xml files, they need support for AffineTransform for each viewSetup. A viewSetup is either a channel or an angle. There is a transform per timepoint.

@jburel
Copy link
Member

jburel commented Oct 8, 2021

The physicalSizeX etc. will need to be known for scale bar but I assume that will be stored elsewhere in the file

@jburel
Copy link
Member

jburel commented Oct 8, 2021

Resolution level, viewport could be considered

@will-moore
Copy link
Member

@jburel If you're suggesting that the ngff spec uses a different naming convention than snake_case then you should suggest that on ome/ngff#54 since I don't think we want different conventions for different parts of the spec.
I don't quite understand the relevance of https://schema.org/ here. Are you thinking that we might want to extend the schema.org vocabulary with our ngff terms?

@jburel
Copy link
Member

jburel commented Oct 8, 2021

My comment is more general that this specific issue. The suggestion of following schema.org is coming from the fact that others efforts e.g. bioschema.org, ro-crate follow that convention so it might make more sense to follow similar rule instead of the snake_case convention

@joshmoore
Copy link
Member

Are you thinking that we might want to extend the schema.org vocabulary with our ngff terms?

There's a pretty good likelihood that we will eventually want to incorporate schema.org types into our metadata, but that's true for other ontologies & vocabularies as well. The likelihood of being able to find a set which follow the same naming style is basically zero.

@jburel
Copy link
Member

jburel commented Oct 8, 2021

I am currently looking at https://www.ebi.ac.uk/spot/oxo/docs/api for a project: mappingTarget is for example a field returned

@dominikl
Copy link
Member Author

With the recent suggestions added, it would look like these:

{
    "channels": {
        "0": {
            "color": "00FF00",
            "color_format": "hex",
            "color_type": "rgb",
            "label": "GFP",
            "mapping": {
                "coefficient": "1.8",
                "family": "exponential",
                "reverse": "False"
            },
            "noise_reduction": "False",
            "window": {
                "end": 1542.5789794921875,
                "max": 1542.5789794921875,
                "min": 0.0,
                "start": 0.0
            }
        },
...
    },
    "color_model": "color",
    "default_dimensions": {
        "0": 0,
        "1": 0
    },
    "dimensions": "zt",
    "group": {
        "0": {
            "indexes": [
                "0",
                "2"
            ],
            "type": "channel",
            "visible": "True"
        }
    },
    "plane": "xy",
    "version": 3
}

Not sure where to put the viewSetup.

@sbesson
Copy link
Member

sbesson commented Oct 14, 2021

A few initial thoughts while reading #55 (comment) (without having looked at the notes):

  • I see the channels index is changed from 1-based index to 0-based index. No objection to this as long as we agree this is the way forward and do not change it back again
  • does the current draft of the rendering model support sparse channel settings or should all channels be specified? In the latter case, it might be possible to choose between a dictionary (assuming the key is the channel index) and a list
  • the new color_format/color_type properties might be think we might be moving towards defining a color dictionary rather than a series of flat keys?
  • dimensions/default_dimensions: is there an expected overlap/relationship with the axes metadata? Should these default settings also be within the group objects?

@dominikl
Copy link
Member Author

You're right, color as dictionary is better. And default Z/T (dimensions/axis) should be part of the group. Also dimension should actually be axis. And the coefficient in the channel mapping is now a list (as far as I understand one could have other operations than linear, exponential which might have more than one parameter).

I struggle a bit with a further generalisation of the dimension/axis bit. In general the axis are XYZTC (or whatever order). But they are not the same kind of thing. Channel is a special dimension, and you also still need the concept of a plane, plus extra dimensions. Right? But then @jburel said instead of channels one could also have angles (sorry I still don't really get this bit).

Anyway, at the moment it would look like this:

Another example
{
    "axes": "zt",
    "channels": {
        "0": {
            "color": {
                "format": "hex",
                "type": "rgb",
                "value": "00FF00"
            },
            "label": "GFP",
            "mapping": {
                "coefficients": [
                    "1.8"
                ],
                "family": "exponential",
                "reverse": "False"
            },
            "noise_reduction": "False",
            "window": {
                "end": 1542.5789794921875,
                "max": 1542.5789794921875,
                "min": 0.0,
                "start": 0.0
            }
        },
        "1": {
            "color": {
                "format": "hex",
                "type": "rgb",
                "value": "FFFFFF"
            },
            "label": "1",
            "mapping": {
                "coefficients": [
                    "1.0"
                ],
                "family": "linear",
                "reverse": "False"
            },
            "noise_reduction": "False",
            "window": {
                "end": 1555.0537109375,
                "max": 1555.0537109375,
                "min": 0.0,
                "start": 0.0
            }
        },
        "2": {
            "color": {
                "format": "name",
                "type": "lut",
                "value": "brgbcmyw"
            },
            "label": "2",
            "mapping": {
                "coefficients": [
                    "1.0"
                ],
                "family": "linear",
                "reverse": "False"
            },
            "noise_reduction": "False",
            "window": {
                "end": 188.89093017578125,
                "max": 188.89093017578125,
                "min": 0.007875712588429451,
                "start": 0.007875712588429451
            }
        }
    },
    "color_model": "color",
    "group": {
        "0": {
            "default_axis": {
                "0": 0,
                "1": 0
            },
            "indexes": [
                "0",
                "2"
            ],
            "type": "channel",
            "visible": "True"
        }
    },
    "plane": "xy",
    "version": 3
}```

</details>

@dominikl
Copy link
Member Author

does the current draft of the rendering model support sparse channel settings or should all channels be specified?

That's a good question too. I don't think we've discussed that yet.

@will-moore
Copy link
Member

Currently, it looks like if the number of channels in the omero data is different from the size_c of the data (or if any of the channels is missing color) then the image will fail to open in napari. But it shouldn't be hard to fix this and handle missing channels / colors.

Apologies for missing any discussions on this, but what does "plane": "xy" and "axes": "zt" signify?
The proposal at ome/ngff#57 is to specify the types of the various axes, e.g.:

"axes": [
  {"name": "t", "type": "time", "unit": "millisecond"},
  {"name": "c", "type": "channel"},
  {"name": "z", "type": "space", "unit": "micrometer"},
  {"name": "y", "type": "space", "unit": "micrometer"},
  {"name": "x", "type": "space", "unit": "micrometer"}
],

@dominikl
Copy link
Member Author

dominikl commented Oct 21, 2021

You need to somehow specify that you're viewing a x,y plane and that the further dimensions are labelled as z and t (in exactly that order). This axis list looks good. But is that ordered? And how would you in that case say you want to show the x,y plane for z-axis 3 by default for example?

@will-moore
Copy link
Member

The axes list above is certainly ordered. Already for v0.3 we have "axes": ["z", "y", "x"] etc, but for v0.4 it's proposed to specify the type of each axis.
If you have "axes": ["z", "y", "x"] with "default_dimensions": {"0": 3,} I think that would do it for "z-axis 3"?
Just being picky, I wonder if default_dimensions in your examples is really specifying defaultIndices rather than dimensions?
NB: it's looking like ome/ngff#54 is heading for camelCase.

@dominikl
Copy link
Member Author

But still needs a plane. Or should one assume from default_dimensions": {"0": 3,} and axes": ["z", "y", "x"] that the plane is x,y? That would mean all default_dimensions have to specified. Yes, it should rather be defaultIndices indeed.

@will-moore
Copy link
Member

will-moore commented Oct 21, 2021

I think that plane can default to xy, so we don't need to specify it. If you choose to specify the x or y dimension in the defaultIndices then I guess you might need to know plane (only if you have size_T > 1 AND size_Z > 1), but we have no way to support that in napari or vizarr as far as I can tell (https://napari.org/docs/dev/api/napari.view_layers.Viewer.html#napari.view_layers.Viewer.add_image)
And in genuine 3D viewers (moBie, neuroglancer) I don't think you'll use it either?

@dominikl
Copy link
Member Author

I thought the point was to keep things as generic as possible, no matter what's currently supported by our or other software. But if we define the plane is always x/y and the axis are always z and t - like it is now - then this whole discussion is pointless and we can basically take the current output of the render plugin as rendering settings spec (just have to change a few names like z to default-z, defaultZ, default_z, ...)

@jburel
Copy link
Member

jburel commented Oct 26, 2021

plane cannot default to xy, in our model we have the concept of PlaneDef and support for xy, xz and yz. This was previously implemented in insight as a slice viewer.

@unidesigner
Copy link

Just a quick, perhaps unrelated question: Is the idea that tool-specific (e.g. rendering specific) metadata becomes part of the "official" ngff spec? I'm wondering because I created this issue with regards to tool-specific metadata. Sorry if this has been answered already elsewhere.

@jburel
Copy link
Member

jburel commented Oct 27, 2021

"mapping": {
                "coefficients": [
                    "1.0"
                ],
                "family": "linear",
                "reverse": "False"
            },

I don't see why coefficients is a list. if you have a mapping function that takes more than one parameter it will have to be a dictionary. The proposal is mixing 2 concepts:

  • first the mapping of the pixels data to the device space (subset of 0-255). This is what family+coefficient cover (linear/polynomial/log/exponential)
  • Second step is extra mapping within the device space, e.g. reverse intensity, binning Salt&Pepper etc. That should probably be a dictionary. Depends on the viewer used the number of "device space" mapping can vary and might not be supported by another viewer e.g. we do not support S&P but ImageJ does. This is a "mapping" extension

@jburel
Copy link
Member

jburel commented Oct 27, 2021

Looking at the BDV example,

<Timepoints type="range">
<first>0</first>
<last>2</last>
</Timepoints>

defaultIndices will probably need to support range or list of ranges and not only a value
in our case it could be "0": [0,0]. This adds some complexity in the handling but should hopefully accommodate other viewers' needs

@dominikl
Copy link
Member Author

As the axis definition is apparently already in the "general" ngff spec, we might as well use the axis names z, t for the plane and defaultIndices instead of indices. I don't know where/what kind of dictionary this mapping element should be, nor how/where to add an angle definition, please just add an example @jburel / @dgault .

@unidesigner : Yes, it's going to be an ngff specfication, but as far as I know there will be places for tool specific metadata too.

Updated example:
{
    "channels": {
        "0": {
            "coefficient": "1.8",
            "color": {
                "format": "hex",
                "type": "rgb",
                "value": "00FF00"
            },
            "family": "exponential",
            "label": "GFP",
            "noiseReduction": "False",
            "reverse": "False",
            "window": {
                "end": 1542.5789794921875,
                "max": 1542.5789794921875,
                "min": 0.0,
                "start": 0.0
            }
        },
        "1": {
            "coefficient": "1.0",
            "color": {
                "format": "hex",
                "type": "rgb",
                "value": "FFFFFF"
            },
            "family": "linear",
            "label": "1",
            "noiseReduction": "False",
            "reverse": "False",
            "window": {
                "end": 1555.0537109375,
                "max": 1555.0537109375,
                "min": 0.0,
                "start": 0.0
            }
        },
        "2": {
            "coefficient": "1.0",
            "color": {
                "format": "name",
                "type": "lut",
                "value": "brgbcmyw"
            },
            "family": "linear",
            "label": "2",
            "noiseReduction": "False",
            "reverse": "False",
            "window": {
                "end": 188.89093017578125,
                "max": 188.89093017578125,
                "min": 0.007875712588429451,
                "start": 0.007875712588429451
            }
        }
    },
    "colorModel": "color",
    "group": {
        "0": {
            "defaultIndices": {
                "t": {
                    "from": 0,
                    "to": 0
                },
                "z": {
                    "from": 0,
                    "to": 0
                }
            },
            "indices": [
                "0",
                "2"
            ],
            "type": "channel",
            "visible": "True"
        }
    },
    "plane": "xy"
}

@jburel
Copy link
Member

jburel commented Oct 28, 2021

{
    "views": {
       "type": "channel",
        "0": {
            "color": {
                "format": "hex",
                "type": "rgb",
                "value": "00FF00"
            },
            "coefficient": "1.8",
            "family": "exponential",
            "label": "GFP",
            "noiseReduction": "False",
            "deviceSpaceMapping":  [{"reverseIntensity": {"value": True}}],
            "window": {
                "end": 1542.5789794921875,
                "max": 1542.5789794921875,
                "min": 0.0,
                "start": 0.0
            }
        },
        "1": {
            "coefficient": "1.0",
            "color": {
                "format": "hex",
                "type": "rgb",
                "value": "FFFFFF"
            },
            "family": "linear",
            "label": "1",
            "noiseReduction": "False",
            "deviceSpaceMapping":  [{"reverseIntensity": {"value": True}}, {"binning": {"0-100": 100, "101-200": 200, "201-255": 255}}]
            "window": {
                "end": 1555.0537109375,
                "max": 1555.0537109375,
                "min": 0.0,
                "start": 0.0
            }
        },
        "2": {
            "coefficient": "1.0",
            "color": {
                "format": "name",
                "type": "lut",
                "value": "brgbcmyw"
            },
            "family": "linear",
            "label": "2",
            "noiseReduction": "False",
            "reverse": "False",
            "window": {
                "end": 188.89093017578125,
                "max": 188.89093017578125,
                "min": 0.007875712588429451,
                "start": 0.007875712588429451
            }
        }
    },
    "colorModel": "color",
    "group": {
        "0": {
            "defaultIndices": {
                "t": {
                    "from": 0,
                    "to": 0
                },
                "z": {
                    "from": 0,
                    "to": 0
                }
            },
            "indices": [
                "0",
                "2"
            ],
            "type": "channel",
            "visible": "True",
            "name": "Group 1"
        }
    },
    "plane": "xy"
}

As mentioned during the call, angle is a new dimension so I have replaced channels by views and we identify the views by its type. If we have settings for angles and channels, we will have multiple views entries, this is no different from groups. I have also added a new entry deviceSpaceMapping. This is a bit complicated since we need a name and the parameters for the transformation.
For reverseIntensity, the name is probably enough but it seems strange to just have the name

@jburel
Copy link
Member

jburel commented Nov 3, 2021

From discussion today it should probably be

 "views": {
        "0": {
           "channel": "0",
           "angle": "1",
            "color": {
                "format": "hex",
                "type": "rgb",
                "value": "00FF00"
            },
            "coefficient": "1.8",
            "family": "exponential",
            "label": "GFP",
            "noiseReduction": "False",
            "deviceSpaceMapping":  [{"reverseIntensity": {"value": True}}],
            "window": {
                "end": 1542.5789794921875,
                "max": 1542.5789794921875,
                "min": 0.0,
                "start": 0.0
            }
        },

@jburel
Copy link
Member

jburel commented Nov 25, 2021

   "groups: [
      {
       "views": [id1, id2],
       "name": "Cycle1",
       "visible": "True",
      },
      {
       "views": [id3],
       "name": "Cycle2",
       "visible": "True",
      },
    ],
    "views": [
             { 
                 "id": xxx,
                 "angle": "",
                 "channel": "0",
                 "color": {}... etc.
              },
              {
                 "id": xxx,
                 "angle": "",
                  "channel": "1",
                  "color": {}... etc.
              }
        ],
   "colorModel": "color",
   "defaultPlane": [3,4] 
   "defaultIndices" = [5,10:20,0:200,null],   // (t,z, x, y) view only 0-200 on x axis and all values on y axis 
   "version": 3

@will-moore
Copy link
Member

will-moore commented Nov 25, 2021

Fixing a few things:

  • If the image has a channel axis, then need to include that as null in the defaultIndices.
  • Use @id for IDs.
  • Adding visible to each view, as well as on group
  • defaultIndices can be used for defining default x and y for viewport and the range has to be a string to be valid JSON e.g. "defaultIndices": [5, null, "10:20", 250, 750]
{
    "groups": [
        {
            "views": ["view1", "view2"],
            "name": "Cycle1",
            "visible": true
        },
        {
            "views": ["view3"],
            "name": "Cycle2",
            "visible": true
        }
    ],
    "views": [
        {
            "@id": "view1",
            "channel": 0,
           "angle": 20,
            "visible": true,
            "color": {}  //... etc.
        },
        {
            "@id": "view2",
            "channel": 1,
            "angle": 30,
            "visible": false,
            "color": {}  //... etc.
        },
        {
            "@id": "view3",
            "channel": 2,
            "angle": 10,
            "visible": true,
            "color": {} //... etc.
        }
    ],
    "colorModel": "color",
    // e.g. shape is (t,c,z,y,x, angle)
    "defaultIndices": [5, null, "10:20", null, null, null],   // default view: t=5 and range 10-20 on z axis 
    // we want to view y,x plane by default
    "defaultPlane": [3, 4],
}

edit: @will-moore I have added angle so people don't think we dropped it during our brainstorming

@will-moore
Copy link
Member

I removed angle since it's not yet allowed as a dimension type by the spec v0.3 or v0.4 proposal at https://github.com/ome/ngff/pull/57/files#diff-ffe6148e5d9f47acc4337bb319ed4503a810214933e51f5f3e46a227b10e3fcdR214
Unless you want to suggest an update to that PR, or add "angle" as a specified type when we add rendering?

@jburel
Copy link
Member

jburel commented Nov 25, 2021

For the spec that we propose when ready we can omit angle if we are not ready at the ngff spec level yet.
This is to avoid confusion amongst people currently involve in this discussion

@jburel
Copy link
Member

jburel commented Nov 25, 2021

With the current specification in mind
We should have something like
renderingSettings: a collection of renderingDefinition objects

"renderingDefinition": {
   "@id": "renderingDef1"
   "groups": [
        {
            "views": ["view1", "view2"],
            "name": "Cycle1",
            "visible": "True"
        },
        {
            "views": ["view3"],
            "name": "Cycle2",
            "visible": "True"
        }
    ],
    "views": [
        {
            "@id": "view1",
            "channel": 0,
            "visible": true,
            "coefficient": "1.0",
            "color": {
                "format": "name",
                "type": "lut",
                "value": "brgbcmyw.lut"
            },
            "deviceSpaceMapping": {
                "reverseIntensity": "False"
            },
            "family": "linear",
            "label": "GFP",
            "noiseReduction": "False",
            "window": {
                "end": 188.89093017578125,
                "max": 188.89093017578125,
                "min": 0.007875712588429451,
                "start": 0.007875712588429451
            }
        },
        {
            "@id": "view2",
            "channel": 1,
            "visible": "False",
            "coefficient": "1.8",
            "color": {
                "format": "hex",
                "type": "rgb",
                "value": "00FF00"
            },
            "deviceSpaceMapping": {
                "reverseIntensity": "False"
            },
            "family": "exponential",
            "label": "DAPI",
            "noiseReduction": "False",
            "window": {
                "end": 1542.5789794921875,
                "max": 1542.5789794921875,
                "min": 0.0,
                "start": 0.0
            }
        },
        {
            "@id": "view3",
            "channel": 2,
            "visible": "True",
            "color": {
                "format": "hex",
                "type": "rgb",
                "value": "FFFFFF"
            },
            "deviceSpaceMapping": {
                "reverseIntensity": "False"
            },
            "family": "linear",
            "label": "OOOO",
            "noiseReduction": "False",
            "window": {
                "end": 1555.0537109375,
                "max": 1555.0537109375,
                "min": 0.0,
                "start": 0.0
            }
        }
    ],
    "colorModel": "color",
    // e.g. shape is (t,c,z,y,x)
    "defaultIndices": [5, null, 10:20, null, null],   // default view: t=5 and range 10-20 on z axis 
    // we want to view y,x plane by default
    "defaultPlane": [3, 4],
    "resolution": 0
}

@jburel
Copy link
Member

jburel commented Nov 25, 2021

One remaining question is about the LUT, it is currently identify by a name. Is that acceptable? or do we want to ship the binary? A viewer will still need to be able to read the LUT. I think the name of the lut e.g. a.lut will be enough for most cases
i.e.

"color": {
                "format": "name",
                "type": "lut",
                "value": "brgbcmyw"
            },

@jburel
Copy link
Member

jburel commented Nov 26, 2021

@will-moore I have added a new parameter resolution. This is a value currently missing in our model and important when browsing pyramid for example

@chris-allan
Copy link
Member

Yes, currently PathViewer's channel group specification allows for a channel to be in more than one group. There's lots of history there and @emilroz, @erindiel, and @knabar would be your authorities.

@jburel
Copy link
Member

jburel commented Nov 30, 2021

Suggestion from @chris-allan: use term palette instead LUT
see https://en.wikipedia.org/wiki/List_of_color_palettes

@jburel
Copy link
Member

jburel commented Nov 30, 2021

@jburel
Copy link
Member

jburel commented Dec 1, 2021

Discussion about RGB support, possible options:
*

"views": [
        {
            "@id": "view1",
            "channel": 0:2, or [0, 1, 2]
            "visible": true,
            "label": "RGB",
        },
  • "colorModel": "color",
  • use groups with 3 views corresponding to each "channel"

"defaultIndices": [null, 0:2, null, null, null],
"views": [
{
"@id": "view1",
"visible": true,
"label": "RGB",
},

see also AllenCellModeling/aicsimageio#165

@will-moore
Copy link
Member

will-moore commented Dec 1, 2021

napari has an rgb=True flag for an Image, in which case the Image name is used for labelling the rgb channel:
https://napari.org/#simple-example

We could support this with "colorModel": "rgb", and then we wouldn't need views at all.

@jburel
Copy link
Member

jburel commented Dec 1, 2021

Comment from @joshmoore
Some rnd thoughts:

  • (1) there was a suggestion somewhere to use “component” rather than “channel” would that help to cover angle?
  • (2) when angles are introduced, would a more complex (multidimensional) indexing than channel: 1 be useful?
  • (3) I wonder if some of the fields belong more “together” like family/noiseReduction/… in an object like “window”
  • (4) I’m unsure how the window plays together with the defaultIndices? (perhaps because I generally don’t follow defaultIndices).
  • (5) did “zoom” get discussed rather than “resolution”?
  • (6) something feels a bit off about the “defaultPlane” representation but I haven’t pinned it down, yet.

@jburel
Copy link
Member

jburel commented Dec 2, 2021

for (1)&(2) I think we need to review how we can extend when it comes into play. Initially it was channel and angle but that will not scale when we extend
(3) yes
(4) "defaultIndices": [5, null, 10:20, 0:300, 100:200] // e.g. shape is (t,c,z,y,x) will indicate to display the specified rectangle [100, 0, 100 (200-100), 300]
(5) zoom is for me a "preferences". Happy to rethink that if people think we should include it
(6) this was previously "plane": "xy",. This matches what we currently have in OMERO (planeDef)

@jburel
Copy link
Member

jburel commented Dec 2, 2021

(3)
something like:

"mapping": {
  "family": "polynomial",
  "coefficient": "1.5",
  "noiseReduction": "False",
}

@will-moore
Copy link
Member

I've just noticed that "model": "greyscale" is ignored in vizarr, so you end up seeing actual channel colour (e.g. grey) https://hms-dbmi.github.io/vizarr/?source=https://uk1s3.embassy.ebi.ac.uk/0.3_samples/7751.zarr/B/11/0
I'm wondering if we want to request that all OME-NGFF clients handle this flag and support a greyscale viewing mode, when you can achieve the same rendering effect by saving a channel as white (and turning the other channels off, if there are more than 1).
I don't know if any viewers (outside of OMERO) support this greyscale mode of toggling channels and if it's really an OMERO-specific viewing behaviour rather than a rendering setting?

@jburel
Copy link
Member

jburel commented Dec 6, 2021

@will-moore Thanks for checking. This could be a limiting factor for the rgb mode we discussed the other day.
Could you check the strategy in Napari and Vizarr? i.e. passing the flag rgb=true in napari does it set a "mode" and what is the value when that flag is not. This will help driving forward

@will-moore
Copy link
Member

In napari, the rgb flag is stored on the base image class and used in a bunch of methods: https://github.com/napari/napari/blob/main/napari/layers/image/image.py
If no value for rgb is provided, it is guessed from the shape (True if last dimension is 3).

vizarr has no 'rgb' mode

@will-moore
Copy link
Member

I think the rgb concept is a separate issue from greyscale.
I think there is a case for supporting rgb (since we know that there are viewers that support it), but I'm proposing that we don't support greyscale.
For images coming from OMERO (which are the only times we have colorModel: greyscale) we should just set the active channel to white.
This is what I do for OMERO.figure. It's also what we do in napari-ome-zarr because napari doesn't support greyscale.
But we force every client to adopt the same logic. It would be better to remove it from the spec and handle greyscale when we export from OMERO.

@jburel
Copy link
Member

jburel commented Dec 8, 2021

Following today's discussion

"renderingDefinition": {
   "@id": "renderingDef1"
   "groups": [
        {
            "views": ["view1", "view2"],
            "name": "Cycle1",
            "visible": "True"
        },
        {
            "views": ["view3"],
            "name": "Cycle2",
            "visible": "True"
        }
    ],
    "views": [
        {
            "@id": "view1",
            "dimension": [null, 0, null, null, null]  // (t,c,z,y,x)
            "visible": true,
            "mapping": {
               "family": "linear",
            }
            "color": {
                "format": "name",
                "type": "lut",
                "value": "brgbcmyw.lut"
            },
            "deviceSpaceMapping": {
                "reverseIntensity": "False"
            },
            "label": "GFP",
            "window": {
                "end": 188.89093017578125,
                "max": 188.89093017578125,
                "min": 0.007875712588429451,
                "start": 0.007875712588429451
            }
        },
        {
            "@id": "view2",
             "dimension": [null, 1, null, null, null]  // (t,c,z,y,x)
            "visible": "False",
            "color": {
                "format": "hex",
                "type": "rgb",
                "value": "00FF00"
            },
            "deviceSpaceMapping": {
                "reverseIntensity": "False"
            },
            "mapping": {
               "family": "polynomial",
               "coefficient": "1.5",
               "noiseReduction": "False",
            }
            "label": "DAPI",
            "window": {
                "end": 1542.5789794921875,
                "max": 1542.5789794921875,
                "min": 0.0,
                "start": 0.0
            }
        },
        {
            "@id": "view3",
             "dimension": [null, 2, null, null, null]  // (t,c,z,y,x)
            "visible": "True",
            "color": {
                "format": "hex",
                "type": "rgb",
                "value": "FFFFFF"
            },
            "deviceSpaceMapping": {
                "reverseIntensity": "False"
            },
            "mapping": {
               "family": "linear",
            }
            "label": "OOOO",
            "window": {
                "end": 1555.0537109375,
                "max": 1555.0537109375,
                "min": 0.0,
                "start": 0.0
            }
        }
    ],
    "colorModel": "color",
    // e.g. shape is (t,c,z,y,x)
    "defaultIndices": [5, null, 10:20, null, null],   // default view: t=5 and range 10-20 on z axis 
    // we want to view y,x plane by default
    "planarView": [3, 4],
    "zoom": 50 // percentage starting from highest resolution
}

RGB case could possibly be covered:

"renderingDefinition": {
   "@id": "renderingDef2"
    "views": [
        {
            "@id": "view1",
            "dimension": [null, 0:2, null, null, null]  // (t,c,z,y,x)
            "visible": true,
            "label": "RGB",
        },
        {
            "@id": "view2",
            "dimension": [null, 3, null, null, null]  // (t,c,z,y,x)
            "visible": "False",
            "color": {
                "format": "hex",
                "type": "rgb",
                "value": "00FF00"
            },
            "deviceSpaceMapping": {
                "reverseIntensity": "False"
            },
            "mapping": {
               "family": "polynomial",
               "coefficient": "1.5",
               "noiseReduction": "False",
            }
            "label": "DAPI",
            "window": {
                "end": 1542.5789794921875,
                "max": 1542.5789794921875,
                "min": 0.0,
                "start": 0.0
            }
        },
    ],
    "colorModel": "color",
    // e.g. shape is (t,c,z,y,x)
    "defaultIndices": [5, null, 10:20, null, null],   // default view: t=5 and range 10-20 on z axis 
    // we want to view y,x plane by default
    "planarView": [3, 4],
    "zoom": 100
}

@jburel
Copy link
Member

jburel commented Dec 8, 2021

Other option for RGB is to use a view per channel (as suggested previously) and combine the views in an RGB group

Last point: drop ColorModel

@jburel
Copy link
Member

jburel commented Dec 8, 2021

Latest example from today's discussion

"renderingDefinition": {
   "@id": "renderingDef1"
   "groups": [
        {
            "views": ["view1", "view2"],
            "name": "Cycle1",
            "visible": "True"
        },
        {
            "views": ["view3"],
            "name": "Cycle2",
            "visible": "True"
        }
    ],
    "views": [
        {
            "@id": "view1",
            "dimension": [null, 0, null, null, null]  // (t,c,z,y,x)
            "visible": true,
            "mapping": {
               "family": "linear",
            }
            "color": {
                "format": "name",
                "type": "lut",
                "value": "brgbcmyw.lut"
            },
            "deviceSpaceMapping": {
                "reverseIntensity": "False"
            },
            "label": "GFP",
            "window": {
                "end": 188.89093017578125,
                "max": 188.89093017578125,
                "min": 0.007875712588429451,
                "start": 0.007875712588429451
            }
        },
        {
            "@id": "view2",
             "dimension": [null, 1, null, null, null]  // (t,c,z,y,x)
            "visible": "False",
            "color": {
                "format": "hex",
                "type": "rgb",
                "value": "00FF00"
            },
            "deviceSpaceMapping": {
                "reverseIntensity": "False"
            },
            "mapping": {
               "family": "polynomial",
               "coefficient": "1.5",
               "noiseReduction": "False",
            }
            "label": "DAPI",
            "window": {
                "end": 1542.5789794921875,
                "max": 1542.5789794921875,
                "min": 0.0,
                "start": 0.0
            }
        },
        {
            "@id": "view3",
            "dimension": [null, 2, null, null, null]  // (t,c,z,y,x)
            "visible": "True",
            "color": {
                "format": "hex",
                "type": "rgb",
                "value": "FFFFFF"
            },
            "deviceSpaceMapping": {
                "reverseIntensity": "False"
            },
            "mapping": {
               "family": "linear",
            }
            "label": "OOOO",
            "window": {
                "end": 1555.0537109375,
                "max": 1555.0537109375,
                "min": 0.0,
                "start": 0.0
            }
        }
    ],
    // e.g. shape is (t,c,z,y,x)
    "defaultIndices": [5, null, 10:20, null, null],   // default view: t=5 and range 10-20 on z axis 
    "planarView": [3, 4], // we want to view y,x plane by default
    "zoom": 50 // percentage starting from highest resolution
}

RGB example

"renderingDefinition": {
   "@id": "renderingDef1"
   "groups": [
        {
            "views": ["view1", "view2", "view3"],
            "name": "RGB",
            "visible": "True"
        }
    ],
    "views": [
        {
            "@id": "view1",
            "dimension": [null, 0, null, null, null]  // (t,c,z,y,x)
            "visible": true,
            "color": {
                "format": "hex",
                "type": "rgb",
                "value": "FF0000"
            },
            "label": "Red",
        },
        {
            "@id": "view2",
            "dimension": [null, 2, null, null, null]  // (t,c,z,y,x)
            "visible": true,
            "color": {
                "format": "hex",
                "type": "rgb",
                "value": "00FF00"
            },
            "label": "Green",
        },
        {
            "@id": "view3",
            "dimension": [null, 1, null, null, null]  // (t,c,z,y,x)
            "visible": true,
            "color": {
                "format": "hex",
                "type": "rgb",
                "value": "0000FF"
            },
            "label": "Blue",
        },
    ],
}

Summary

  • drop colorModel
  • channel replaced by dimension (list) so it can be extended when for example angle is supported
  • combine mapping parameters from space to deviceSpace into a mapping field
  • rename defaultPlane by planarView
  • RGB case will be handled by groups and views
  • replace resolution by zoom percentage from the highest resolution

@will-moore
Copy link
Member

I still think the rgb case needs a more explicit flag on the group. The "name" is not enough, and we likely want to allow a more descriptive name. So I think "rgb": true would be easiest.

"groups": [
        {
            "views": ["view1", "view2", "view3"],
            "name": "My histology image",
            "visible": true
            "rgb": true
        }
    ],

@jburel
Copy link
Member

jburel commented Dec 10, 2021

Thinking about it other use cases might need some "group" flexibility e.g. FLIM with "small t"
so something like that will be extensible

"groups": [
        {
            "views": ["view1", "view2", "view3"],
            "name": "My histology image",
            "visible": true
            "metadata": {"rgb": true}    //similar to deviceSpaceMapping
        }
    ],

cc @will-moore

@jburel
Copy link
Member

jburel commented Dec 14, 2021

Final proposal before migrating to ngff issue.

"renderingDefinition": {
   "@id": "renderingDef1"
   "groups": [
        {
            "views": ["view1", "view2"],
            "name": "Cycle1",
            "visible": true
        },
        {
            "views": ["view3"],
            "name": "Cycle2",
            "visible": true
        }
    ],
    "views": [
        {
            "@id": "view1",
            "dimension": [null, 0, null, null, null]  // (t,c,z,y,x)
            "visible": true,
            "mapping": {
               "family": "linear",
            }
            "color": {
                "format": "name",
                "type": "lut",
                "value": "brgbcmyw.lut"
            },
            "deviceSpaceMapping": {
                "reverseIntensity": false
            },
            "label": "GFP",
            "window": {
                "end": 188.89093017578125,
                "max": 188.89093017578125,
                "min": 0.007875712588429451,
                "start": 0.007875712588429451
            }
        },
        {
            "@id": "view2",
             "dimension": [null, 1, null, null, null],  // (t,c,z,y,x)
            "visible": false,
            "color": {
                "format": "hex",
                "type": "rgb",
                "value": "00FF00"
            },
            "deviceSpaceMapping": {
                "reverseIntensity": false
            },
            "mapping": {
               "family": "polynomial",
               "coefficient": 1.5,
               "noiseReduction": false,
            }
            "label": "DAPI",
            "window": {
                "end": 1542.5789794921875,
                "max": 1542.5789794921875,
                "min": 0.0,
                "start": 0.0
            }
        },
        {
            "@id": "view3",
            "dimension": [null, 2, null, null, null]  // (t,c,z,y,x)
            "visible": true,
            "color": {
                "format": "hex",
                "type": "rgb",
                "value": "FFFFFF"
            },
            "deviceSpaceMapping": {
                "reverseIntensity": false
            },
            "mapping": {
               "family": "linear",
            }
            "label": "OOOO",
            "window": {
                "end": 1555.0537109375,
                "max": 1555.0537109375,
                "min": 0.0,
                "start": 0.0
            }
        }
    ],
    // e.g. shape is (t,c,z,y,x)
    "defaultIndices": [5, null, "10:20", null, null],   // default view: t=5 and range 10-20 on z axis 
    "planarView": [3, 4], // we want to view y,x plane by default
    "zoom": 50 // percentage starting from highest resolution
}

RGB example

"renderingDefinition": {
   "@id": "renderingDef1"
   "groups": [
        {
            "views": ["view1", "view2", "view3"],
            "name": "The name you want to use",
            "visible": true,
            "metadata": {"rgb": true}
        }
    ],
    "views": [
        {
            "@id": "view1",
            "dimension": [null, 0, null, null, null]  // (t,c,z,y,x)
            "visible": true,
            "color": {
                "format": "hex",
                "type": "rgb",
                "value": "FF0000"
            },
            "label": "Red",
        },
        {
            "@id": "view2",
            "dimension": [null, 2, null, null, null]  // (t,c,z,y,x)
            "visible": true,
            "color": {
                "format": "hex",
                "type": "rgb",
                "value": "00FF00"
            },
            "label": "Green",
        },
        {
            "@id": "view3",
            "dimension": [null, 1, null, null, null]  // (t,c,z,y,x)
            "visible": true,
            "color": {
                "format": "hex",
                "type": "rgb",
                "value": "0000FF"
            },
            "label": "Blue",
        },
    ],
}

@imagesc-bot
Copy link

This issue has been mentioned on Image.sc Forum. There might be relevant details there:

https://forum.image.sc/t/saving-volumetric-data-with-voxel-size-colormap-annotations/85537/28

@dominikl
Copy link
Member Author

I suppose we can just close this now @jburel ? bioformats2raw seems to put the rendering settings already in the .zattrs anyway, guess that's the spec then.

@dominikl
Copy link
Member Author

Indeed, it's already in the spec: https://ngff.openmicroscopy.org/latest/#omero-md .

@joshmoore
Copy link
Member

I understood the purpose of this work was to replace the #omero-md block with something more formal. I would very much 👍 an RFC following the previous model. The one topic that it might be worth considering is that with the ongoing ome-types work, whether or not we don't also define an XSD for the rendering settings which would allow us to be backwards and forwards compatible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants