Skip to content

fix(labelmap): use content-based comparison for segmentsHidden Set#2675

Open
pedrokohler wants to merge 1 commit into
cornerstonejs:mainfrom
pedrokohler:fix/labelmap-set-reference-comparison
Open

fix(labelmap): use content-based comparison for segmentsHidden Set#2675
pedrokohler wants to merge 1 commit into
cornerstonejs:mainfrom
pedrokohler:fix/labelmap-set-reference-comparison

Conversation

@pedrokohler

Copy link
Copy Markdown
Contributor

Context

_needsTransferFunctionUpdate() in labelmapDisplay.ts compares
oldSegmentsHidden !== segmentsHidden using reference equality. Because
segmentsHidden is a new Set instance on every render call, this comparison
always returns true — even when the contents are identical. This forces
forceOpacityUpdate = true unconditionally, triggering unnecessary .slice() and
new Set() allocations on every render cycle for every segmentation.

With 13 segmentations across ~180 render calls per scroll, this produces hundreds
of redundant transfer-function updates per interaction.

Changes & Results

Replace the reference comparison with a content-based comparison:

  1. Check size equality first (fast path).
  2. If sizes match, check [...segmentsHidden].some(idx => !oldSegmentsHidden.has(idx)).

Before: transfer function updated on every render (~180 times per scroll).
After: transfer function updated only when segments are actually hidden/shown (~15 times total in a typical session).

Testing

  1. Open a StackViewport with multiple labelmap segmentations.
  2. Scroll through slices without toggling segment visibility.
  3. Place a breakpoint or log in _needsTransferFunctionUpdateforceOpacityUpdate
    should be false when no visibility changes occurred.

Checklist

PR

  • My Pull Request title is descriptive, accurate and follows the
    semantic-release format and guidelines.

Code

  • My code has been well-documented (function documentation, inline comments,
    etc.)

Public Documentation Updates

  • The documentation page has been updated as necessary for any public API
    additions or removals.

Made with Cursor

@pedrokohler pedrokohler marked this pull request as draft March 25, 2026 22:19
@pedrokohler pedrokohler marked this pull request as ready for review March 27, 2026 11:39
@pedrokohler

Copy link
Copy Markdown
Contributor Author

@sedghi this is a performance optimization, it reduces allocation churn / CPU overhead during scrolling.

@sedghi

sedghi commented Mar 27, 2026

Copy link
Copy Markdown
Member

Thanks, i let QA test it

@pedrokohler

Copy link
Copy Markdown
Contributor Author

@sedghi it's been almost 2 months since QA was asked to review. Could we merge?

@sedghi

sedghi commented May 12, 2026

Copy link
Copy Markdown
Member

@james-hanks

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

Successfully merging this pull request may close these issues.

2 participants