Repository navigation
Cancel abandoned loads, surface expansion failures, apply MIF scaling - #3
Merged
Merged
Conversation
First load not cancellable: Plugin.View ran MiqParser.ParsePartial with no token and only checked cancellation after the full read/decompress, so arrowing through large files left every abandoned load running. ParsePartial now takes a CancellationToken and threads it through the full-parse fallbacks, the volume-0 reads (ReadFully) and GunzipPartial. Reads are chunked at 4 MB so the token is checked during multi-GB reads, including the managed Gunzip loop. Cancellation escapes the partial-load fallback catches instead of triggering a full parse. Expansion failures swallowed: StartExpansion caught every exception, leaving the scrubber on "loading..." forever after an out-of-memory, I/O error or changed file. Only cancellation is now ignored; any other failure calls MiqTriPlanarControl.ExpansionFailed, which shows "first volume only (full load failed)" with the reason in a tooltip. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XqBwhnTRsqxk3SEvp9z4T9
MifParser hard-coded SclSlope/SclInter to 0 and never read the `scaling: offset,scale` field MRtrix writes for non-identity intensity scaling (e.g. integer data converted from DICOM with Rescale Slope/Intercept). The voxel readout showed raw stored values, the Scaling metadata row was missing, a negative scale rendered inverted, and scaled data passed the identity-scaling check for segmentation colouring. The field is now mapped to SclSlope = scale, SclInter = offset. Absent, identity, malformed, non-finite or zero-scale values stay unscaled, so an optional field never fails the parse. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XqBwhnTRsqxk3SEvp9z4T9
Resolve MiqParser.ParsePartial conflicts: keep main's header-based blocking (NiftiFileExtent, no gzip ISIZE) and pass this branch's cancellation token to every fallback Parse call. Also split ParsePartial into a one-argument overload plus ParsePartial(string, CancellationToken): with an optional parameter the method group no longer converted to Func<string, MiqImage>, which NiftiFormatLabelTests relies on. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XqBwhnTRsqxk3SEvp9z4T9
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three fixes, one commit each. Independent of #2 (branched from
main); both touchMiqParser.ParsePartial, so whichever merges second will need a small conflict resolution there.1. First load could not be cancelled
Plugin.ViewcalledMiqParser.ParsePartial(path)without a token and checked cancellation only after the full read/decompress. Arrowing through large files left every abandoned load running to completion (CPU, I/O and memory for files no longer on screen).ParsePartial(string, CancellationToken ct = default)threads the token through the full-parse fallbacks, the volume-0 reads (ReadFully) andGunzipPartial.Gunziploop.catchblocks instead of triggering a full parse.Plugin.Viewpassescts.Tokenand ignoresOperationCanceledExceptionafter nav-away.Remaining gap: the libdeflate path's
File.ReadAllBytesof the compressed file is not interruptible (the token is checked just before it).2. Background expansion failures were swallowed
StartExpansioncaught every exception, so an out-of-memory, I/O error or changed file left the scrubber on "loading…" forever with no retry. Now only cancellation is ignored; any other failure callsMiqTriPlanarControl.ExpansionFailed. That keeps volume 0 visible, shows "first volume only (full load failed)" (newScrubMode.Failed), and puts the exception message in the tooltip.3. MRtrix MIF
scaling:field was ignoredMifParserhard-codedSclSlope/SclInterto 0. It now mapsscaling: offset,scale(value = offset + scale × stored) toSclSlope = scale,SclInter = offset. The voxel readout and Scaling metadata row are now correct, negative scales no longer invert the image, and scaled data no longer passes the identity-scaling check for segmentation colouring. Absent, identity, malformed, non-finite or zero-scale values stay unscaled; the optional field never fails the parse.Tests
ParseCancellationTests(non-parallel collection, because one test lowersPartialLoadThreshold): a pre-cancelled token throws on the multi-volume.nii.gzpartial path, single-volume.nii.gzfull path, small.niifull path and large.niivolume-0 path, and each file still loads without a token. Also coversGunzipPartialdirectly.MifScalingTests: scaled voxel value, Scaling metadata text, segmentation colouring disabled by scaling (with an unscaled control that does colour), and six absent/unusable variants that stay unscaled.Not yet built or run: the authoring environment had no .NET SDK (download blocked by network policy), and CI builds only the plugin, not
MIQ.Tests. Please rundotnet buildanddotnet testlocally before merging. It is also worth confirming theoffset,scaleorder against a realmrconvertoutput with non-identity scaling.🤖 Generated with Claude Code
https://claude.ai/code/session_01XqBwhnTRsqxk3SEvp9z4T9
Generated by Claude Code