create-dmg icon indicating copy to clipboard operation
create-dmg copied to clipboard

generate the image in swift and remove dependency on imagemagic and graphicsmagick

Open gsabran opened this issue 4 months ago • 8 comments

This should address https://github.com/sindresorhus/create-dmg/issues/57

Disclaimer: this code is mostly vibe coded. The result looks good from my testing though: Before:

Before After
Before After

gsabran avatar Aug 29 '25 06:08 gsabran

Ask AI to improve and simplify the Swift code.

sindresorhus avatar Aug 29 '25 08:08 sindresorhus

swift build's output works on both arm64 and x86-64 right? I thought so but was not entirely sure.

Is the change in package.json fine to have the built file part of the package? I didn't know how to verify this.

gsabran avatar Aug 29 '25 16:08 gsabran

Ask AI to improve and simplify the Swift code.

At some point it's easier to take over. Good feedback on the code.

gsabran avatar Aug 29 '25 16:08 gsabran

AI review:


  • Artifacts committed: compose-icon binary and Xcode .swiftpm/** user files are in the PR. Drop them and add ignores. ([GitHub]1)

    • Add to .gitignore:

      /compose-icon
      /.build
      /.swiftpm
      
  • Build script is single-arch and brittle pathing. It does swift build --configuration release --product compose-icon and copies from .build/release/…. That yields an arm64-only binary on Apple Silicon and breaks Intel; path can vary. Build Universal 2 and copy via --show-bin-path. ([GitHub]1)

    #!/usr/bin/env bash
    set -euo pipefail
    cd imageComposition
    xcrun swift build -c release --arch arm64 --arch x86_64 --product compose-icon
    BIN_DIR="$(xcrun swift build -c release --show-bin-path)"
    cp "$BIN_DIR/compose-icon" ../compose-icon
    
  • README still claims GraphicsMagick is required. Update it in the same PR to avoid confusion. ([GitHub]2)

  • package.json: Good: removes gm, adds prepublishOnly and includes the binary in files. Keep that, but don’t commit the binary—only generate it on publish. ([GitHub]1)

Bugs / correctness

  • Wrong crop and unused context in perspectiveTransform. You construct a CGContext you never use and crop the produced CGImage to (0,0,width,height), which can be wrong when the CI output extent is translated. Crop the CI output to the input extent before rasterizing. ([GitHub]1)
  • Nearest-neighbor-ish resampling. resizeImage does not set interpolation quality. Results can be soft or jaggy depending on defaults. Set .high. ([GitHub]1)
  • Truncation on resize. Int(Double(...) / 1.58) truncates. Use .rounded() for stable, symmetric rounding. ([GitHub]1)
  • Confusing Y-axis comment. The comment says “flip Y” but you do not flip. With a bitmap CGContext, origin is bottom-left; adding a positive offsetY moves the overlay up, which matches the original GM geometry (-offset). Fix the comment or flip consistently. ([GitHub]1)
  • Error messages go to stdout. Send usage and errors to stderr, exit non-zero. ([GitHub]1)
  • Color space not explicit. Use sRGB for predictable compositing across machines. ([GitHub]1)
  • Bitmap info incomplete. Specify byte order with premultiplied alpha to avoid platform-specific defaults. ([GitHub]1)
  • Minor API style nit. You mix typed Core Image API with setValue(_, forKey:). Use typed properties throughout. ([GitHub]1)

sindresorhus avatar Aug 31 '25 07:08 sindresorhus

@sindresorhus thoughts? Unclear why the test timed-out only on node 20.

gsabran avatar Sep 23 '25 00:09 gsabran

@sindresorhus so?

gsabran avatar Nov 07 '25 01:11 gsabran

I don't have time to look into it. Try asking AI.

sindresorhus avatar Nov 10 '25 08:11 sindresorhus

It's green now.

gsabran avatar Nov 10 '25 17:11 gsabran