Conversation
Both diff screenshot inputs had to be PNG, so a capture from another tool had to be converted first and a HarmonyOS artifact, which the platform serves as JPEG, never compared. Each input is now decoded from its own bytes. png-transcode.ts became screenshot-image.ts, the one owner of container sniffing for both the decode and the provider transcode path, and the PNG worker gained a decode-image job that answers pixels instead of PNG bytes.
Drops the screenshot-diff module mock and the package export that existed only to feed it, so the JPEG cases run the same worker client production uses; pins the diff artifact container and the decode-image transfer arm, and corrects two doc claims the first commit overreached on: HarmonyOS still serves JPEG, and a stored JPEG compares exactly against itself at threshold 0.
The gate rewrites those timing and result artifacts on every run; they were swept in by a broad stage, not changed by this feature.
|
Size Report
Startup median (7 runs, lower is better):
|
Member
Author
|
Reviewed at 8d3a871. This looks ready to merge. CI is green across all 20 checks, and I didn't find anything that blocks it. I traced the JPEG decode and re-encode paths manually rather than running the suite, relying on CI green plus reading the code against the test assertions. Not blocking: |
thymikee
added
the
ready-for-human
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
1 participant
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
diff screenshotdecoded both inputs as PNG, so a capture exported by another tool had to be converted first and a HarmonyOS artifact — which the device serves as JPEG under whatever name the command was given — could never be compared. Each input is now decoded from its own bytes: the container is sniffed, so a.pngname holding JPEG decodes as JPEG.png-transcode.tsbecamescreenshot-image.ts, the one owner of container sniffing for both the decode path and the provider transcode path, and the PNG worker gained adecode-imagejob that answers pixels instead of PNG bytes, so a full-resolution decode never blocks the daemon event loop. The--outdiff image stays PNG, as do the crop, overlay, and resize passes that rewrite a screenshot in place.13 files, ~80 net production lines. No new flag, no schema or wire change.
Validation
pnpm check:affected --runpassed (all runnable checks) at46b0079daf, afterpnpm check:quick.8d3a8719a9only restores gate-generated.agent-device/test-artifactsfiles to theirorigin/maincontent; no source changed.New coverage: worker↔sync decode parity for both containers,
decode-imagetransfer ownership, a JPEG stored under a.pngname comparing exactly at--threshold 0, decoded dimensions on a JPEG, corrupt-JPEG and neither-container refusals with their typed details, and the diff artifact pinned to a PNG signature. The screenshot-diff suite now runs the shipped worker client instead of a module mock.Unresolved risks, both pre-existing and deliberately not folded in here:
packages/platform-harmonyos/src/screenshot.ts:15still writes JPEG bytes into a.pngartifact, so--scaleand--overlay-refsfail on that platform. The provider-side transcode Limrun got in fix(limrun): write the iOS screenshot as PNG instead of Limrun's JPEG bytes #2645 is the owning fix; this PR makes those artifacts diffable meanwhile.maxImagePixelsis consulted (already true for PNG);jpeg-jsself-caps at 100 MP / 512 MB.