feat(segmentation_user_layer): add user shader for segmentation layers with support for segment properties by chrisj · Pull Request #1074 · google/neuroglancer

chrisj · 2026-07-13T16:59:53Z

continuation of #862

seankmartin

This looks nice, thanks for all the changes! In general the API in the shader seems pretty reasonable to me. I've not gotten to go through it all, but some small comments from what I have looked through. There are also some TODO markers and the likes, not sure if those are still relevant or you settled on the approach in those cases

seankmartin · 2026-07-16T10:41:01Z

-  colorGroupState.segmentColorHash.compute(color, objectId);
-  return color;
+  const { getShaderSegmentColor } = displayState;
+  return getShaderSegmentColor(objectId) ?? color;


Could we detect if the shader is the default one, and in that case use the old non-shader color path to display the colors in the UI for performance. I noticed that the segmentation menu can sometimes feel a bit more sluggish to go through, which seems pretty fair when you've chosen to use the segment shader and to me it seems fine. But if possible to avoid this slight hit if you didn't change your shader I think that would be good

I think that is what I should do for now but maybe I should give a quick try at adding batching. I don't notice sluggishness but I do notice that the color doesn't load immediately if scrolling very fast on my machine. I can't replicate that on the main branch.

It's easy to add support to calculate many segment colors in a single draw call, the issue is that the segment list is designed so that each row is processed independently. But if I can get it working, it should be far more performant.

That makes a lot of sense, thanks Chris

@seankmartin can you check if this deployment has noticeably better performance for you?

demo

Oh wow, that's so much better yes thanks @chrisj

seankmartin · 2026-07-16T10:44:16Z

+      const hasStringProperty = inlineProperties.properties.some(
+        (p) => p.type === "string",
+      );
+      if (!hasStringProperty) {


I guess this is leftover but it does raise what we should do here. Shader error comes to mind, but then in theory the underlying data could change and lose a property. Maybe that's fine though.

yeah that was for demonstration as string properties are not common. But do you mean if a shader references a property that was later removed from the segment properties file? If so I think the current error "property does not exist" is correct.

seankmartin · 2026-07-16T10:52:18Z

+  });
+}
+
+function makeSkeletonShaderCodeWidget(layer: SegmentationUserLayer) {


Minor but the style of the element layout seems different on this new shader controls in the UI compared to the skeleton ones

seankmartin · 2026-07-16T10:57:31Z

 }

+export const DEFAULT_FRAGMENT_SEGMENT_COLOR = `
+vec3 segmentColor(vec3 color, bool hasProperties, bool isStated) {


Oh also minor but leading newline in the resulting shader here that probably is better left out

also I should rename it from FRAGMENT becomes it is sometimes used in the vertex shader

chrisj · 2026-07-16T15:46:15Z

@seankmartin thanks for the review! I have some changes I'll push shortly that I noticed yesterday

seankmartin · 2026-07-16T15:53:49Z

@seankmartin thanks for the review! I have some changes I'll push shortly that I noticed yesterday

sounds great, thanks!

chrisj · 2026-08-17T11:09:24Z

@seankmartin I simplified the performance optimizations and added it to this PR. I added a temporary setting that you can toggle with "q" to easily observe the difference. Can you give it a test to see if you are still see the big performance improvement? I can see the a difference when I press "q" on and off while dragging the scroll bar. If it looks good I'll revert those last few commits.

example

seankmartin · 2026-08-17T11:12:28Z

@seankmartin I simplified the performance optimizations and added it to this PR. I added a temporary setting that you can toggle with "q" to easily observe the difference. Can you give it a test to see if you are still see the big performance improvement? I can see the a difference when I press "q" on and off while dragging the scroll bar. If it looks good I'll revert those last few commits.

example

Thanks @chrisj, that's great! And yes still much better for me when batched

chrisj · 2026-08-22T01:47:04Z

  toMerged: Uint32Array,
 ): InlineSegmentNumericalProperty {
-  const values = new Float32Array(numMerged);
+  const values = property.values.slice(0, numMerged);


I've been diving into this change and need some thoughts.

This function is called when merging multiple segment property maps together. It was casting every numerical property values array to Float32Array. If the maps had differing sets of segments, each segment received a value of NaN for the numerical property of each map it was not in.

The easiest solution would be to just use a zero value and keep each values as it's original numerical type. Another option is to do that but add a validity mask. This is a rough idea of what that would look like: AllenInstitute@4e7a970

if (hasProp("myProperty")) { uint value = prop("myProperty"); // use value }

I'm leaning towards using 0 instead of NaN for non-float and skipping the validity mask as I imagine it won't have much use.

Seems reasonable to me to do the simpler version, I also can't imagine it's used too often

I think it would be fine to use 0 for non-float properties --- using NaN for float properties would still be good I think.

chrisj · 2026-08-25T12:19:18Z

 ): InlineSegmentNumericalProperty {
-  const values = new Float32Array(numMerged);
-  values.fill(Number.NaN);
+  const values = new (property.values


@jbms are you good with this change?

759a4d6

fcollman · 2026-08-31T18:18:40Z

@jbms @seankmartin I think we think this PR is now in a state that it is ready for review again and chris thinks he has addressed all the issues. There is the one spot he would like feedback on that is in the most recent comment.

fcollman · 2026-09-10T17:08:00Z

@jbms @seankmartin recently we have been trying to specify colors across a large number of meshes (for an atlas, ~1300 regions) where we have some semantic coloring available. Right now that requires a custom information in the state in a segmentColors, rather than having those colors available as segmentProperties that would lift them out of the state.

seankmartin · 2026-09-15T15:01:53Z

+    return color;
+  }
+
+Use ``type="string"`` for a string property picker. ``type="number"`` and


Very minor but not sure the "tag" type shown in the code snippet is clear from this line as "tag" isn't mentioned

seankmartin · 2026-09-15T15:47:08Z

 float absCosAngle = abs(dot(normal, uLightDirection.xyz));
 float lightingFactor = absCosAngle + uLightDirection.w;
-vColor = vec4(lightingFactor * uColor.rgb, uColor.a);
+vColor = segmentColorUserShader(uint64_t(uID));


I was experimenting with this a little more, here is an example

I think this may have made the same mistake I made in the new cylinder and ball rendering mode for skeletons where it missed that the old color coming in from getObjectColor premultiplied by the user setting of opacity in the layer, and that seems to be missing here and also in the skeleton/frontend code

There's a similar problem with equivalences, equivalence colors are no longer used, for example looks like

but should be like
Where around the center of the screen at least two meshes of segments should be the same color as they were merged and the skeletons in 2d seem to not use the equivalence color while the segments do. And I think the hover highlight isn't behaving quite right with equivalences

seankmartin · 2026-09-15T16:04:15Z

+      this.layer.manager.chunkManager.chunkQueueManager.gl,
+    );
+
+    this.offscreenGL = initializeWebGL(new OffscreenCanvas(1, 1));


I'm not overly sure about the new canvas and GL context being made inside of the user layer display state, I think it would be better if we could use something at a higher level than this. For example at the viewer context level. If I remember correctly from when I was working on transfer functions, the empirical CDF code in webgl/empirical_cdf used a pattern for this for working with the viewer context which we could maybe reuse here. I believe it made some new framebuffers instead of a full context. What do you think?

seankmartin · 2026-09-15T16:17:42Z

+### Examples
+
+```glsl
+#uicontrol float property1 invlerp(property="myNumericalProperty", window=[1, 10])


This doesn't work for me, the type has to be invlerp not float. Is this intended to work? Or are the docs written here and in rst incorrect?

seankmartin · 2026-09-15T16:29:19Z

+
+    // 2d/3d share GL context
+    // the offscreen canvas is a separate context that doesn't share textures
+    this.segmentationColorUserShader = new SegmentColorUserShaderManager(


I think this and the other shader color user manager should be wrapped in a registerDisposer call

seankmartin · 2026-09-15T16:36:51Z

+    this.hashMapManager.enable(
+      gl,
+      shader,
+      GPUHashTable.get(this.gl, this.segmentPropertyIndexMap),


This I believe should instead be like on L983 because otherwise we will add many refs to the same hash table

seankmartin · 2026-09-15T16:50:56Z

+            gl.deleteTexture(previousTextureFromValues);
+            if (!values || !window) return;
+            const { dataType } = this;
+            const texture = createHistogramTextureFromValues(


I think this might be a bit cleaner and less likely to leak if it is more like the ScaleBarTexture in scale_bar.ts or the DirectLookupTableTexture or BaseLookupTexture in transfer_function.ts, so you can make a HistogramTexture class which is ref counted

…segment properties, remove todo comments

Previously, merged numerical properties were initialized with NaN, which coerced integer typed arrays to zero while leaving float properties as NaN. Allocate the merged values with the source typed-array constructor so missing entries consistently default to zero, preserve the property datatype, and expand the bounds to include zero.

…N for missing values instead of zero

…t color user shader

…r shader generates 6 or less colors for more than 6 visible segments. perf(ui/layer_bar): limit automaticLayerBarColors to first 10,0000 visible segments

…rols and add detection for property shader controls

…ng a type that only changes the visbility of numerical properties

refactor(segmentation): remove usage of negative alpha as it is unnecessary

…estored controls against the merged property set

…n safe, otherwise rename the properties with a numerical suffix, similar to the layer renaming strategy

…o Segmentation Shaders user guide

…when constrained vertically fix(segmentation): segmentation user shader editors now hide separately and correctly restore the value from the state

chrisj force-pushed the cj-segment-color-user-shader branch 8 times, most recently from 07f7c7c to 7adf3fc Compare July 15, 2026 16:43

chrisj marked this pull request as ready for review July 15, 2026 16:43

chrisj force-pushed the cj-segment-color-user-shader branch from 7adf3fc to 53cfe9b Compare July 15, 2026 16:45

seankmartin reviewed Jul 16, 2026

View reviewed changes

chrisj force-pushed the cj-segment-color-user-shader branch 2 times, most recently from 00444f6 to 9f4e76c Compare July 20, 2026 15:28

chrisj force-pushed the cj-segment-color-user-shader branch 2 times, most recently from 72bcce9 to b3fd516 Compare August 13, 2026 16:56

chrisj force-pushed the cj-segment-color-user-shader branch 4 times, most recently from 775e56d to c4c7d9d Compare August 19, 2026 14:55

chrisj commented Aug 22, 2026

View reviewed changes

chrisj force-pushed the cj-segment-color-user-shader branch from 7dac20e to 1784f69 Compare August 25, 2026 12:16

chrisj commented Aug 25, 2026

View reviewed changes

seankmartin reviewed Sep 15, 2026

View reviewed changes

chrisj added 29 commits October 29, 2026 16:13

clean up shader property references to either refer to annotation or …

5fd42f5

…segment properties, remove todo comments

fix(skeleton): apply segment and layer opacity

2ec944b

fix(segmentation): guard fallback highlight uniform

a5393c3

fix(segment-color): detect formatted vec3 shaders

7173ca1

fix(invlerp): handle empty and uint64 property values

1e71dd5

fix(segmentation): aggregate shader errors by render path

3da94ab

fix(segment-color): preserve large string literal ids

0359a23

when remapping Float32 properties, keep existing behavior of using Na…

f5ab54e

…N for missing values instead of zero

Fix shader control documentation

0da532b

Fix base segment coloring for meshes and skeletons

782a76b

Fix mesh and skeleton opacity rendering

5ed6080

Enable transparency for meshes and skeletons when using a vec4 segmen…

14af5b4

…t color user shader

refactor(segmentation): use framebuffer for color lookups

1b71754

fix(segmentation): batch shader color lookups

81457ea

fix(segmentation): validate shader color output buffer

4298d0d

refactor(segmentation): reuse shader color lookup buffer

e2040f1

fix: missing registerDisposer on SegmentColorUserShaderManager

bda0189

fix: avoid accumulating references on segmentPropertyIndexMap

ddf5c24

fix: add missing disposers in InvlerpWidget

7f75608

fix(ui/layer_bar): generate automaticLayerBarColors when a custom use…

d5f0ca9

…r shader generates 6 or less colors for more than 6 visible segments. perf(ui/layer_bar): limit automaticLayerBarColors to first 10,0000 visible segments

fix: fix reachability detection for invlerp and transferFunction cont…

8139901

…rols and add detection for property shader controls

fix(segmentation): apply slice view layer opacity after color shader

8692fa8

fix(shader_ui_controls): update property control dropdown when choosi…

0cc8cf8

…ng a type that only changes the visbility of numerical properties

docs: remove references to negative alpha for segmentation shaders

d433f8f

refactor(segmentation): remove usage of negative alpha as it is unnecessary

fix(segmentation): Wait for every attached source before validating r…

d90fc2f

…estored controls against the merged property set

feat(segmentation): reconcile duplicate segment properties, merge whe…

e0e9247

…n safe, otherwise rename the properties with a numerical suffix, similar to the layer renaming strategy

docs(segmentation): add Relationship to the Skeleton Shader section t…

98300bc

…o Segmentation Shaders user guide

fix(segmentation): segmentation user shader editors no longer shrink …

edb4c9b

…when constrained vertically fix(segmentation): segmentation user shader editors now hide separately and correctly restore the value from the state

chrisj force-pushed the cj-segment-color-user-shader branch from a2b2cf5 to edb4c9b Compare October 29, 2026 20:51

Conversation

chrisj commented Jul 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

seankmartin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chrisj commented Jul 16, 2026

Uh oh!

seankmartin commented Jul 16, 2026

Uh oh!

chrisj commented Aug 17, 2026

Uh oh!

seankmartin commented Aug 17, 2026

Uh oh!

chrisj Aug 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fcollman commented Aug 31, 2026

Uh oh!

fcollman commented Sep 10, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

chrisj commented Jul 13, 2026 •

edited

Loading

chrisj Aug 22, 2026 •

edited

Loading