From c15d272071c4aaaa5323345d11506513942cd3d6 Mon Sep 17 00:00:00 2001 From: Sean Martin Date: Wed, 13 May 2026 14:42:35 +0200 Subject: [PATCH 1/4] feat: add hover highlight and seg highlight also indicate via comments diff paths and clear seg hasSelectedSegment --- src/skeleton/frontend.ts | 101 ++++++++++++++++++++++++++++++--------- 1 file changed, 78 insertions(+), 23 deletions(-) diff --git a/src/skeleton/frontend.ts b/src/skeleton/frontend.ts index 7a13000ac2..55d441fa52 100644 --- a/src/skeleton/frontend.ts +++ b/src/skeleton/frontend.ts @@ -227,6 +227,7 @@ interface SkeletonShaderParameters { dynamicSegmentAppearance: boolean; hasSegmentStatedColors: boolean; hasSegmentDefaultColor: boolean; + hoverHighlight: boolean; spatialChunkCulling: boolean; } @@ -327,9 +328,6 @@ class RenderHelper extends RefCounted { if (skeletonParams.dynamicSegmentAppearance) { this.defineDynamicSegmentAppearance(builder, skeletonParams); } - if (skeletonParams.dynamicSegmentAppearance) { - builder.addVarying("highp uint", "vSegmentValue", "flat"); - } if (skeletonParams.spatialChunkCulling) { builder.addUniform("highp vec3", "uChunkOrigin"); builder.addUniform("highp vec3", "uChunkBound"); @@ -441,9 +439,14 @@ void spatialChunkCull() { } builder.addUniform("highp float", "uVisibleAlpha"); builder.addUniform("highp float", "uHiddenAlpha"); + builder.addUniform("highp float", "uSaturation"); if (params.hasSegmentDefaultColor) { builder.addUniform("highp vec3", "uSegmentDefaultColor"); } + if (params.hoverHighlight) { + builder.addUniform("highp uvec2", "uHoveredSegmentId"); + } + builder.addVarying("highp uint", "vSegmentValue", "flat"); const statedColorFragment = params.hasSegmentStatedColors ? ` @@ -457,14 +460,29 @@ void spatialChunkCull() { ? " return uSegmentDefaultColor;" : ` ${colorExpression}`; + const hoverAdjustFragment = params.hoverHighlight + ? ` + if (segmentId.value.x == uHoveredSegmentId.x && + segmentId.value.y == uHoveredSegmentId.y) { + if (saturation > 0.5) { saturation -= 0.5; } + else { saturation += 0.5; } + }` + : ""; + builder.addFragmentCode(` uint64_t getSegmentAppearanceId(highp uint segmentValue) { return uint64_t(uvec2(segmentValue, 0u)); } -vec3 getSegmentLookupColor(uint64_t segmentId) { +vec3 getSegmentBaseColor(uint64_t segmentId) { ${statedColorFragment} ${defaultColorFragment} } +vec3 getSegmentLookupColor(uint64_t segmentId) { + vec3 baseColor = getSegmentBaseColor(segmentId); + float saturation = uSaturation; +${hoverAdjustFragment} + return mix(vec3(1.0, 1.0, 1.0), baseColor, saturation); +} float getSegmentLookupAlpha(uint64_t segmentId) { if (${this.excludedSegmentsShaderManager.hasFunctionName}(segmentId)) { return ${excludedSegmentAlpha}; @@ -537,6 +555,19 @@ vec4 getSegmentAppearance(highp uint segmentValue) { this.gpuSegmentStatedColorHashTable, ); } + + const { saturation, segmentSelectionState } = this.base.displayState; + gl.uniform1f(shader.uniform("uSaturation"), saturation.value); + if (skeletonParams.hoverHighlight) { + const seg = segmentSelectionState.hasSelectedSegment + ? segmentSelectionState.selectedSegment + : 0n; + gl.uniform2ui( + shader.uniform("uHoveredSegmentId"), + Number(seg & 0xffff_ffffn), + Number((seg >> 32n) & 0xffff_ffffn), + ); + } } maybeDisableDynamicSegmentAppearance( @@ -647,6 +678,9 @@ highp uint vertexIndex = aVertexIndex.x * (1u - lineEndpointIndex) + aVertexInde ? "uColor.a" : `${segmentColorExpression}.a`; if (skeletonParams.dynamicSegmentAppearance) { + // Dynamic path (spatial skeletons): per-segment color, visibility, + // saturation and hover highlight all resolved in the shader via + // getSegmentAppearance(). uColor is unused in this path. builder.addFragmentCode(` vec4 segmentColor() { return getSegmentAppearance(vSegmentValue); @@ -665,10 +699,9 @@ void emitDefault() { } `); } else if (this.segmentColorAttributeIndex === undefined) { - // Preserve legacy skeleton behavior where `uColor` is already - // premultiplied by `objectAlpha` in `getObjectColor`. - // in this path, whole skeletons are drawn at one time - // as opposed to multiple skeletons + // Legacy path (non-spatial skeletons): one skeleton drawn per call; + // uColor is set per-skeleton by the CPU via getObjectColor(), which + // already incorporates saturation and hover highlighting. builder.addFragmentCode(` vec4 segmentColor() { return ${segmentColorExpression}; @@ -681,6 +714,8 @@ void emitDefault() { } `); } else { + // Per-vertex color attribute path: color comes from a per-vertex + // attribute; alpha is taken from uColor. builder.addFragmentCode(` vec4 segmentColor() { return ${segmentColorExpression}; @@ -772,6 +807,9 @@ emitCircle( skeletonParams.dynamicSegmentAppearance && this.segmentAttributeIndex !== undefined ) { + // Dynamic path (spatial skeletons): per-segment color, visibility, + // saturation and hover highlight all resolved in the shader via + // getSegmentAppearance(). uColor is unused in this path. const segmentExpression = `vSegmentValue`; const selectedNodeExpression = this.selectedNodeAttributeIndex === undefined @@ -802,7 +840,9 @@ void emitDefault() { } `); } else if (this.segmentColorAttributeIndex === undefined) { - // Preserve legacy skeleton behavior for non-spatial skeletons. + // Legacy path (non-spatial skeletons): one skeleton drawn per call; + // uColor is set per-skeleton by the CPU via getObjectColor(), which + // already incorporates saturation and hover highlighting. builder.addFragmentCode(` vec4 segmentColor() { return ${segmentColorExpression}; @@ -819,6 +859,8 @@ void emitDefault() { } `); } else { + // Per-vertex color attribute path: color comes from a per-vertex + // attribute; alpha is taken from the attribute's alpha component. const selectedNodeExpression = this.selectedNodeAttributeIndex === undefined ? undefined @@ -1199,6 +1241,7 @@ export class SkeletonLayer extends RefCounted implements SkeletonShaderContext { dynamicSegmentAppearance: false, hasSegmentStatedColors: false, hasSegmentDefaultColor: false, + hoverHighlight: false, spatialChunkCulling: false, }); fallbackShaderParameters = new WatchableValue( @@ -2440,30 +2483,42 @@ export class SpatiallyIndexedSkeletonLayer dynamicSegmentAppearance: true, hasSegmentStatedColors: false, hasSegmentDefaultColor: false, + hoverHighlight: false, spatialChunkCulling: false, }); + const updateSkeletonShaderParameters = () => { + const colorGroupState = + this.displayState.segmentationColorGroupState.value; + this.skeletonShaderParameters.value = { + dynamicSegmentAppearance: true, + hasSegmentStatedColors: colorGroupState.segmentStatedColors.size !== 0, + hasSegmentDefaultColor: + colorGroupState.segmentDefaultColor.value !== undefined || + DEBUG_SPATIAL_SKELETON_CHUNKS, + hoverHighlight: this.displayState.hoverHighlight.value, + spatialChunkCulling: false, + }; + }; this.registerDisposer( registerNested((context, colorGroupState) => { - const update = () => { - this.skeletonShaderParameters.value = { - dynamicSegmentAppearance: true, - hasSegmentStatedColors: - colorGroupState.segmentStatedColors.size !== 0, - hasSegmentDefaultColor: - colorGroupState.segmentDefaultColor.value !== undefined || - DEBUG_SPATIAL_SKELETON_CHUNKS, - spatialChunkCulling: false, - }; - }; context.registerDisposer( - colorGroupState.segmentStatedColors.changed.add(update), + colorGroupState.segmentStatedColors.changed.add( + updateSkeletonShaderParameters, + ), ); context.registerDisposer( - colorGroupState.segmentDefaultColor.changed.add(update), + colorGroupState.segmentDefaultColor.changed.add( + updateSkeletonShaderParameters, + ), ); - update(); + updateSkeletonShaderParameters(); }, this.displayState.segmentationColorGroupState), ); + this.registerDisposer( + this.displayState.hoverHighlight.changed.add( + updateSkeletonShaderParameters, + ), + ); this.browsePassSkeletonShaderParameters = this.registerDisposer( makeCachedLazyDerivedWatchableValue( (params) => ({ ...params, spatialChunkCulling: true }), From 9a7ed1af598ab12400bc2707c1fc7fe2b007a7f2 Mon Sep 17 00:00:00 2001 From: Sean Martin Date: Wed, 13 May 2026 15:09:14 +0200 Subject: [PATCH 2/4] fix: don't consider lod undefined as not ready --- src/skeleton/frontend.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/skeleton/frontend.ts b/src/skeleton/frontend.ts index 55d441fa52..38be264c58 100644 --- a/src/skeleton/frontend.ts +++ b/src/skeleton/frontend.ts @@ -2817,7 +2817,11 @@ export class SpatiallyIndexedSkeletonLayer ) { return true; } - if (lod === undefined || transformedSources.length === 0) { + if (lod === undefined) { + // No LOD configured — draw() renders nothing in this case, so nothing to wait for. + return true; + } + if (transformedSources.length === 0) { return false; } const lodSuffix = `:${lod}`; From 6506b38407b4133f446026f5e027df984439d4cc Mon Sep 17 00:00:00 2001 From: Sean Martin Date: Wed, 13 May 2026 15:13:08 +0200 Subject: [PATCH 3/4] fix: isReady considers only current LOD level --- src/skeleton/frontend.ts | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/src/skeleton/frontend.ts b/src/skeleton/frontend.ts index 38be264c58..fa9c964fa3 100644 --- a/src/skeleton/frontend.ts +++ b/src/skeleton/frontend.ts @@ -2807,6 +2807,8 @@ export class SpatiallyIndexedSkeletonLayer } private areVisibleChunksReady( + view: SpatiallyIndexedSkeletonView, + gridLevel: number | undefined, transformedSources: readonly TransformedSource[][], projectionParameters: ProjectionParameters, lod: number | undefined, @@ -2824,12 +2826,18 @@ export class SpatiallyIndexedSkeletonLayer if (transformedSources.length === 0) { return false; } + const selectedSourceIds = new Set( + this.selectSourcesForViewAndGrid(view, gridLevel).map((s) => + getObjectId(s.chunkSource), + ), + ); const lodSuffix = `:${lod}`; const seenChunkKeysBySource = new Map>(); let ready = true; for (const scales of transformedSources) { for (const tsource of scales) { const sourceId = getObjectId(tsource.source); + if (!selectedSourceIds.has(sourceId)) continue; let seenChunkKeys = seenChunkKeysBySource.get(sourceId); if (seenChunkKeys === undefined) { seenChunkKeys = new Set(); @@ -3252,14 +3260,15 @@ export class SpatiallyIndexedSkeletonLayer } isReady( + view: SpatiallyIndexedSkeletonView, + gridLevel: number | undefined, transformedSources: readonly TransformedSource[][], projectionParameters: ProjectionParameters, lod?: number, ) { - // TODO (SKM) I don't think this is getting - // called as expected, for example, I think - // the screenshot should call this but it doesn't seem to return this.areVisibleChunksReady( + view, + gridLevel, transformedSources, projectionParameters, lod, @@ -3591,11 +3600,12 @@ export class PerspectiveViewSpatiallyIndexedSkeletonLayer extends PerspectiveVie >, ) { const { displayState } = this.base; - const lodValue = displayState.skeletonLod?.value; return this.base.isReady( + "3d", + displayState.spatialSkeletonGridLevel3d?.value, this.transformedSources, renderContext.projectionParameters, - lodValue, + displayState.skeletonLod?.value, ); } } @@ -3734,11 +3744,12 @@ export class SliceViewPanelSpatiallyIndexedSkeletonLayer extends SliceViewPanelR >, ) { const { displayState } = this.base; - const lodValue = displayState.spatialSkeletonLod2d?.value; return this.base.isReady( + "2d", + displayState.spatialSkeletonGridLevel2d?.value, this.transformedSources, renderContext.projectionParameters, - lodValue, + displayState.spatialSkeletonLod2d?.value, ); } } From c36306fd0e80eb620ce527aadfb50920eb552f24 Mon Sep 17 00:00:00 2001 From: Sean Martin Date: Wed, 13 May 2026 15:54:13 +0200 Subject: [PATCH 4/4] refactor: remove uneeded seen set management, combine chunk manage paths this allows us to be more maintainable. This bug stemmed from the isReady using a different path for chunk manage then the rendering did --- src/skeleton/frontend.ts | 130 +++++++++++++++++++-------------------- 1 file changed, 64 insertions(+), 66 deletions(-) diff --git a/src/skeleton/frontend.ts b/src/skeleton/frontend.ts index fa9c964fa3..4867bf1a56 100644 --- a/src/skeleton/frontend.ts +++ b/src/skeleton/frontend.ts @@ -2759,50 +2759,77 @@ export class SpatiallyIndexedSkeletonLayer }; } - getVisibleChunksInCurrentViewAndLod( + // Iterates every chunk slot in view for the given view/gridLevel/lod. + // Callback receives (chunkKey, chunkSource, chunkLayout); return false to stop early. + private forEachVisibleChunkSlot( view: SpatiallyIndexedSkeletonView, gridLevel: number | undefined, transformedSources: readonly TransformedSource[][], - projectionParameters: any, - lod: number | undefined, - ): VisibleChunk[] { - if (lod === undefined) { - return []; - } + projectionParameters: ProjectionParameters, + lod: number, + callback: ( + chunkKey: string, + chunkSource: SpatiallyIndexedSkeletonSource, + chunkLayout: ChunkLayout, + ) => boolean | void, + ) { const selectedSourceIds = new Set( this.selectSourcesForViewAndGrid(view, gridLevel).map((s) => getObjectId(s.chunkSource), ), ); const lodSuffix = `:${lod}`; - const result: VisibleChunk[] = []; - const seenChunkKeysBySource = new Map>(); + let shouldContinue = true; for (const scales of transformedSources) { for (const tsource of scales) { - const sourceId = getObjectId(tsource.source); - if (!selectedSourceIds.has(sourceId)) continue; - let seenChunkKeys = seenChunkKeysBySource.get(sourceId); - if (seenChunkKeys === undefined) { - seenChunkKeys = new Set(); - seenChunkKeysBySource.set(sourceId, seenChunkKeys); - } + if (!shouldContinue) return; + if (!selectedSourceIds.has(getObjectId(tsource.source))) continue; forEachVisibleVolumetricChunk( projectionParameters, this.localPosition.value, tsource, (positionInChunks) => { + if (!shouldContinue) return; const chunkKey = `${positionInChunks.join()}${lodSuffix}`; - if (seenChunkKeys!.has(chunkKey)) return; - seenChunkKeys!.add(chunkKey); - const chunkSource = - tsource.source as SpatiallyIndexedSkeletonSource; - const chunk = chunkSource.chunks.get(chunkKey); - if (chunk?.state !== ChunkState.GPU_MEMORY) return; - result.push({ chunk, chunkLayout: tsource.chunkLayout }); + if ( + callback( + chunkKey, + tsource.source as SpatiallyIndexedSkeletonSource, + tsource.chunkLayout, + ) === false + ) { + shouldContinue = false; + } }, ); } } + } + + getVisibleChunksInCurrentViewAndLod( + view: SpatiallyIndexedSkeletonView, + gridLevel: number | undefined, + transformedSources: readonly TransformedSource[][], + projectionParameters: any, + lod: number | undefined, + ): VisibleChunk[] { + if (lod === undefined) { + return []; + } + const result: VisibleChunk[] = []; + this.forEachVisibleChunkSlot( + view, + gridLevel, + transformedSources, + projectionParameters, + lod, + (chunkKey, chunkSource, chunkLayout) => { + const chunk = chunkSource.chunks.get(chunkKey); + if (chunk?.state === ChunkState.GPU_MEMORY) { + result.push({ chunk, chunkLayout }); + } + }, + ); return result; } @@ -2826,52 +2853,23 @@ export class SpatiallyIndexedSkeletonLayer if (transformedSources.length === 0) { return false; } - const selectedSourceIds = new Set( - this.selectSourcesForViewAndGrid(view, gridLevel).map((s) => - getObjectId(s.chunkSource), - ), - ); - const lodSuffix = `:${lod}`; - const seenChunkKeysBySource = new Map>(); let ready = true; - for (const scales of transformedSources) { - for (const tsource of scales) { - const sourceId = getObjectId(tsource.source); - if (!selectedSourceIds.has(sourceId)) continue; - let seenChunkKeys = seenChunkKeysBySource.get(sourceId); - if (seenChunkKeys === undefined) { - seenChunkKeys = new Set(); - seenChunkKeysBySource.set(sourceId, seenChunkKeys); - } - forEachVisibleVolumetricChunk( - projectionParameters, - this.localPosition.value, - tsource, - (positionInChunks) => { - if (!ready) { - return; - } - const chunkKey = `${positionInChunks.join()}${lodSuffix}`; - if (seenChunkKeys!.has(chunkKey)) { - return; - } - seenChunkKeys!.add(chunkKey); - const chunkSource = - tsource.source as SpatiallyIndexedSkeletonSource; - const chunk = chunkSource.chunks.get(chunkKey) as - | SpatiallyIndexedSkeletonChunk - | undefined; - if (chunk?.state !== ChunkState.GPU_MEMORY) { - ready = false; - } - }, - ); - if (!ready) { + this.forEachVisibleChunkSlot( + view, + gridLevel, + transformedSources, + projectionParameters, + lod, + (chunkKey, chunkSource, _) => { + const chunk = chunkSource.chunks.get(chunkKey); + if (chunk?.state !== ChunkState.GPU_MEMORY) { + ready = false; return false; } - } - } - return true; + return true; + }, + ); + return ready; } getNode(