From db25f174c426572d2ed2e6603acf3da0a776e2cb Mon Sep 17 00:00:00 2001 From: raster Date: Sat, 30 May 2026 16:33:16 +0530 Subject: [PATCH 1/3] Fix Metal library loading, ARM64 Golomb-Rice clamp, gradient overflow Three defects surfaced when running the full suite on Apple Silicon via `swift test` (CI only runs on Linux, where Metal and the ARM64 accelerator are compiled out, so these never failed there): - MetalAccelerator: `makeDefaultLibrary(bundle:)` failed with "no default library was found" because SwiftPM's `.process` only copies the `.metal` source into the resource bundle without compiling a `default.metallib` (only Xcode's build system does). Add a runtime fallback that compiles the bundled shader source via `makeLibrary(source:)`, so the GPU path works under both `swift build`/`swift test` and Xcode. (~47 test failures) - ARM64Accelerator.computeGolombRiceParameter: returned an unclamped `k` (up to ~60) for pathological a/n ratios because the CLZ-based estimate can start above 31, skipping the `while k < 31` loop. Clamp to the documented [0, 31] range, matching X86_64Accelerator. (1 test failure) - MetalAccelerator.computeGradientsBatch CPU fallback used trapping subtraction while the GPU shader wraps in 32-bit int, so identical inputs made the CPU path trap (SIGTRAP on `0 - Int32.min`) while the GPU wrapped. Use wrapping subtraction (`&-`) so the paths are bit-identical and the public API no longer crashes on extreme Int32 inputs. Update the boundary test's expectations to match. Full suite now passes: 1388 tests in 105 suites, green debug + release builds. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../Platform/ARM64/ARM64Accelerator.swift | 7 ++- .../Platform/Metal/MetalAccelerator.swift | 62 ++++++++++++++++--- Tests/JPEGLSTests/MetalAcceleratorTests.swift | 15 +++-- 3 files changed, 69 insertions(+), 15 deletions(-) diff --git a/Sources/JPEGLS/Platform/ARM64/ARM64Accelerator.swift b/Sources/JPEGLS/Platform/ARM64/ARM64Accelerator.swift index 53f3d34..4196464 100644 --- a/Sources/JPEGLS/Platform/ARM64/ARM64Accelerator.swift +++ b/Sources/JPEGLS/Platform/ARM64/ARM64Accelerator.swift @@ -193,8 +193,11 @@ public struct ARM64Accelerator: PlatformAccelerator { while k < 31 && (n << k) < a { k += 1 } - - return k + + // Clamp to the documented [0, 31] range. When the CLZ-based estimate + // already starts above 31 (pathological a/n), the loop above is skipped, + // so the clamp here is what enforces the upper bound (matches X86_64Accelerator). + return min(k, 31) } // MARK: - NEON-Optimized MED Predictor diff --git a/Sources/JPEGLS/Platform/Metal/MetalAccelerator.swift b/Sources/JPEGLS/Platform/Metal/MetalAccelerator.swift index a376dc7..096dd0e 100644 --- a/Sources/JPEGLS/Platform/Metal/MetalAccelerator.swift +++ b/Sources/JPEGLS/Platform/Metal/MetalAccelerator.swift @@ -96,8 +96,8 @@ public final class MetalAccelerator: @unchecked Sendable { // Create compute pipeline states for all shaders do { - let library = try device.makeDefaultLibrary(bundle: .module) - + let library = try Self.loadShaderLibrary(device: device) + self.gradientPipelineState = try Self.makePipelineState( library: library, device: device, functionName: "compute_gradients") self.predictionPipelineState = try Self.makePipelineState( @@ -129,7 +129,46 @@ public final class MetalAccelerator: @unchecked Sendable { } // MARK: - Private Helpers - + + /// Load the Metal shader library for `JPEGLSShaders.metal`. + /// + /// Tries the precompiled `default.metallib` first — this is produced when the + /// package is built by Xcode's build system. Under Swift Package Manager on the + /// command line (`swift build`/`swift test`), `.process` only *copies* the + /// `.metal` file into the resource bundle without compiling it, so no + /// `default.metallib` exists and `makeDefaultLibrary(bundle:)` fails. In that + /// case we locate the bundled `.metal` source and compile it at runtime, so the + /// GPU path works in both environments. + private static func loadShaderLibrary(device: MTLDevice) throws -> MTLLibrary { + // Fast path: precompiled default library (Xcode-built bundles). + if let library = try? device.makeDefaultLibrary(bundle: .module) { + return library + } + + // Fallback: compile the bundled shader source at runtime (SPM CLI builds). + guard let sourceURL = Bundle.module.url( + forResource: "JPEGLSShaders", withExtension: "metal" + ) else { + throw MetalAcceleratorError.libraryCreationFailed( + "JPEGLSShaders.metal not found in module bundle and no default.metallib present") + } + + let source: String + do { + source = try String(contentsOf: sourceURL, encoding: .utf8) + } catch { + throw MetalAcceleratorError.libraryCreationFailed( + "Failed to read JPEGLSShaders.metal: \(error)") + } + + do { + return try device.makeLibrary(source: source, options: nil) + } catch { + throw MetalAcceleratorError.libraryCreationFailed( + "Runtime compilation of JPEGLSShaders.metal failed: \(error)") + } + } + /// Create a compute pipeline state for the named shader function. private static func makePipelineState( library: MTLLibrary, @@ -506,9 +545,13 @@ public final class MetalAccelerator: @unchecked Sendable { var d3 = [Int32](repeating: 0, count: count) for i in 0.. Date: Sat, 30 May 2026 16:33:27 +0530 Subject: [PATCH 2/3] Add first-mismatch diagnostics to bench-dicom failures When a round-trip fails, bench-dicom now writes an actionable line to stderr: for a lossless mismatch it reports the first differing sample (row, col, decoded vs original), and for a thrown error it reports the file path and the error. Previously failures were only tallied as a count, with no way to tell which file or pixel diverged. Co-Authored-By: Claude Opus 4.8 (1M context) --- Sources/jpeglscli/BenchDICOMCommand.swift | 24 ++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/Sources/jpeglscli/BenchDICOMCommand.swift b/Sources/jpeglscli/BenchDICOMCommand.swift index 841ce4f..f60010c 100644 --- a/Sources/jpeglscli/BenchDICOMCommand.swift +++ b/Sources/jpeglscli/BenchDICOMCommand.swift @@ -161,7 +161,12 @@ extension JPEGLSCLITool { s.encodeSeconds += encS s.decodeSeconds += decS s.pixels += pixelCount - if near == 0 && !lossless { s.failures += 1 } + if near == 0 && !lossless { + s.failures += 1 + let where_ = firstMismatch(decoded.components.first?.pixels, pixels) + FileHandle.standardError.write(Data( + "FAIL lossless: \(url.path) \(img.columns)x\(img.rows) \(bps)-bit\(where_)\n".utf8)) + } frames.append(FrameResult( modality: mod, width: img.columns, height: img.rows, @@ -171,6 +176,8 @@ extension JPEGLSCLITool { )) } catch { s.failures += 1 + FileHandle.standardError.write(Data( + "FAIL error: \(url.path) \(error)\n".utf8)) } } stats[mod] = s @@ -191,6 +198,21 @@ extension JPEGLSCLITool { return true } + /// Describe the first differing sample between decoded and original + /// pixels, for actionable failure reports. Empty string if identical. + private func firstMismatch(_ a: [[Int]]?, _ b: [[Int]]) -> String { + guard let a = a else { return " (decoded image had no component)" } + if a.count != b.count { return " (decoded \(a.count) rows != original \(b.count))" } + for r in 0.. Date: Sat, 30 May 2026 16:33:37 +0530 Subject: [PATCH 3/3] Prepare 0.8.0 release: finalize CHANGELOG and README - Promote the [Unreleased] / "0.8.0 - In Progress" sections to a dated 0.8.0 release, folding in the DICOM-independence (Milestone 20) work. - Document changes made since the milestones were last recorded: bench-dicom harness + diagnostics, docs/man reorg, jpegls->jpeglscli target rename, and the build/SIMD/Metal/Golomb-Rice/gradient fixes. - Add CHANGELOG comparison links for v0.8.0. - Bump the README SPM install example to `from: "0.8.0"`. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 37 ++++++++++++++++++++++--------------- README.md | 2 +- 2 files changed, 23 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dc27742..b04274a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,19 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -### Added - -#### DICOM Independence & Final Integration (Milestone 20) -- Non-DICOM usage examples in USAGE_EXAMPLES.md (general-purpose compression, web assets, archival storage) -- DICOM-aware/DICOM-independent architecture documented in README.md - -### Fixed - -#### DICOM Independence & Final Integration (Milestone 20) -- Fixed incorrect threshold values in `JPEGLSPresetParametersTests` for 8-bit and 12-bit images; test expectations now match the ITU-T.87 Table C.2 formula (8-bit: T1=3, T2=7, T3=21; 12-bit: T1=18, T2=67, T3=276) -- Fixed incorrect expected values in `JPEGLSNearLosslessTests` for reconstructed-value boundary tests; test inputs now avoid the ITU-T.87 modular-wrap threshold, correctly exercising the clamp-to-MAXVAL and clamp-to-zero paths - -## [0.8.0] - In Progress +## [0.8.0] - 2026-05-30 ### Added @@ -53,16 +41,35 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Man page documentation (man/jpegls.1) in groff format - Man page installation instructions in README +#### DICOM Test Harness & Tooling +- `jpegls bench-dicom` command: JPEG-LS round-trip benchmark over a DICOM corpus, reporting per-modality encode/decode throughput and lossless verification +- First-mismatch diagnostics on `bench-dicom` lossless failures (reports row/col and decoded vs original sample value, written to stderr) + +#### DICOM Independence & Final Integration (Milestone 20) +- Non-DICOM usage examples in USAGE_EXAMPLES.md (general-purpose compression, web assets, archival storage) +- DICOM-aware/DICOM-independent architecture documented in README.md + ### Changed - Overall project test coverage measured at 95.80% on Linux x86_64 (exceeds 95% threshold) - Coverage varies by platform due to conditional compilation of platform-specific optimisations - README.md updated with accurate coverage measurements and platform notes - README.md updated with links to new documentation guides - MILESTONES.md updated to reflect progress on Milestone 8 and 9 +- Reorganised repository layout: documentation moved into `docs/`, man page into `man/` +- Renamed executable target `jpegls` → `jpeglscli` to avoid a case-insensitive-filesystem collision with the `JPEGLS` library target (the built product is still named `jpegls`) ### Fixed - Parser now handles CharLS-encoded files with extension markers - All 12 CharLS reference files can be parsed without errors +- Build & test compilation on case-insensitive (Apple) filesystems +- `SIMDMask.any()` build error on ARM64 (replaced with an equality check) +- Metal GPU path now works under `swift build`/`swift test`: when no precompiled `default.metallib` is present (SwiftPM copies the `.metal` source rather than compiling it), `MetalAccelerator` compiles the bundled shader source at runtime instead of failing with "no default library was found" +- `ARM64Accelerator.computeGolombRiceParameter` now clamps the result to the documented `[0, 31]` range for pathological `a/n` ratios (previously could return an unclamped value; now matches `X86_64Accelerator`) +- `MetalAccelerator.computeGradientsBatch` CPU fallback now uses wrapping subtraction to match the GPU shader's two's-complement semantics, keeping the two paths bit-identical and preventing a trap on extreme `Int32` inputs (e.g. `0 - Int32.min`) + +#### DICOM Independence & Final Integration (Milestone 20) +- Fixed incorrect threshold values in `JPEGLSPresetParametersTests` for 8-bit and 12-bit images; test expectations now match the ITU-T.87 Table C.2 formula (8-bit: T1=3, T2=7, T3=21; 12-bit: T1=18, T2=67, T3=276) +- Fixed incorrect expected values in `JPEGLSNearLosslessTests` for reconstructed-value boundary tests; test inputs now avoid the ITU-T.87 modular-wrap threshold, correctly exercising the clamp-to-MAXVAL and clamp-to-zero paths ## [0.7.0] - Completed @@ -314,5 +321,5 @@ See [RELEASE_NOTES_TEMPLATE.md](docs/RELEASE_NOTES_TEMPLATE.md) for the release - **0.8.0** - Validation & conformance (CharLS, benchmarks, edge cases) - **1.0.0** - Planned stable release - -[Unreleased]: https://github.com/Raster-Lab/JLSwift/commits/main +[Unreleased]: https://github.com/Raster-Lab/JLSwift/compare/v0.8.0...HEAD +[0.8.0]: https://github.com/Raster-Lab/JLSwift/releases/tag/v0.8.0 diff --git a/README.md b/README.md index 4fc20eb..e438b94 100644 --- a/README.md +++ b/README.md @@ -33,7 +33,7 @@ Add JLSwift as a dependency in your `Package.swift`: ```swift dependencies: [ - .package(url: "https://github.com/Raster-Lab/JLSwift.git", from: "0.1.0") + .package(url: "https://github.com/Raster-Lab/JLSwift.git", from: "0.8.0") ] ```