Skip to content

Commit 38f1528

Browse files
committed
fix: Diff percentage regression
In v4 parsable stdout doesn't print diff percentage
1 parent c601883 commit 38f1528

File tree

6 files changed

+83
-68
lines changed

6 files changed

+83
-68
lines changed

build.zig

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ fn configurePlatformLibraries(
254254
const native_target = b.resolveTargetQuery(.{});
255255
const is_cross_compiling = target_info.cpu.arch != native_target.result.cpu.arch or
256256
target_info.os.tag != native_target.result.os.tag;
257-
257+
258258
// For cross-compilation, vcpkg is required - don't fall back to native libraries
259259
if (is_cross_compiling) {
260260
std.log.err("Cross-compilation requires vcpkg libraries. Target: {s}-{s}", .{ @tagName(target_info.cpu.arch), @tagName(target_info.os.tag) });
@@ -486,7 +486,11 @@ fn tryConfigureVcpkg(
486486
if (target_info.os.tag == .windows) {
487487
lib.linkSystemLibrary("zlib"); // vcpkg installs as libzlib.a on Windows
488488
lib.linkSystemLibrary("lzma"); // vcpkg installs as liblzma.a
489-
// Note: deflate library not available in vcpkg mingw-static, skip it
489+
//
490+
// Link GCC runtime for emulated TLS (__emutls_get_address)
491+
// This is required for libjpeg-turbo which uses thread-local storage in SIMD code
492+
lib.linkSystemLibrary("gcc_s");
493+
lib.linkSystemLibrary("gcc");
490494
} else {
491495
lib.linkSystemLibrary("z");
492496
lib.linkSystemLibrary("lzma");

npm_package/odiff.js

Lines changed: 23 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -73,44 +73,33 @@ function optionsToArgs(options) {
7373
/** @type {(stdout: string) => Partial<{ diffCount: number, diffPercentage: number, diffLines: number[] }>} */
7474
function parsePixelDiffStdout(stdout) {
7575
try {
76-
const lines = stdout.trim().split('\n');
77-
78-
if (lines.length >= 1) {
79-
const diffCount = parseInt(lines[0]);
80-
81-
if (isNaN(diffCount)) {
82-
throw new Error(`Invalid diff count: ${lines[0]}`);
83-
}
84-
85-
const result = { diffCount };
86-
87-
// Calculate percentage (odiff outputs absolute count, we calculate percentage)
88-
// This is approximate - in a real implementation we'd need image dimensions
89-
if (lines.length >= 2) {
90-
const diffPercentage = parseFloat(lines[1]);
91-
if (!isNaN(diffPercentage)) {
92-
result.diffPercentage = diffPercentage;
93-
}
94-
}
95-
96-
// Parse diff lines if present
97-
if (lines.length >= 3) {
98-
const diffLines = lines.slice(2)
99-
.map(line => parseInt(line.trim()))
100-
.filter(line => !isNaN(line));
101-
102-
if (diffLines.length > 0) {
103-
result.diffLines = diffLines;
104-
}
105-
}
106-
107-
return result;
76+
const parts = stdout.trim().split(";");
77+
78+
if (parts.length === 2) {
79+
const [diffCount, diffPercentage] = parts;
80+
81+
return {
82+
diffCount: parseInt(diffCount),
83+
diffPercentage: parseFloat(diffPercentage),
84+
};
85+
} else if (parts.length === 3) {
86+
const [diffCount, diffPercentage, linesPart] = parts;
87+
88+
return {
89+
diffCount: parseInt(diffCount),
90+
diffPercentage: parseFloat(diffPercentage),
91+
diffLines: linesPart.split(",").flatMap((line) => {
92+
let parsedInt = parseInt(line);
93+
94+
return isNaN(parsedInt) ? [] : parsedInt;
95+
}),
96+
};
10897
} else {
109-
throw new Error(`No output lines found: ${stdout}`);
98+
throw new Error(`Unparsable stdout from odiff binary: ${stdout}`);
11099
}
111100
} catch (e) {
112101
console.warn(
113-
"Can't parse output from internal process. Please submit an issue at https://github.com/odiff-project/odiff/issues/new with the following stacktrace:",
102+
"Can't parse output from internal process. Please submit an issue at https://github.com/dmtrKovalenko/odiff/issues/new with the following stacktrace:",
114103
e
115104
);
116105
}

src/diff.zig

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ pub noinline fn compareSameLayouts(base: *const Image, comp: *const Image, diff_
217217
const base_data = base.data;
218218
const comp_data = comp.data;
219219

220-
const SIMD_SIZE = if (HAS_AVX512) 16 else if (HAS_NEON) 8 else 4;
220+
const SIMD_SIZE = std.simd.suggestVectorLength(u32) orelse if (HAS_AVX512) 16 else if (HAS_NEON) 8 else 4;
221221
const simd_end = (size / SIMD_SIZE) * SIMD_SIZE;
222222

223223
var offset: usize = 0;

src/main.zig

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -131,14 +131,17 @@ pub fn main() !void {
131131
}
132132

133133
if (args.parsable_stdout) {
134-
stdout.print("{d}\n", .{pixel_result.diff_count}) catch {};
134+
stdout.print("{d};{d:.2}", .{ pixel_result.diff_count, pixel_result.diff_percentage }) catch {};
135135
if (pixel_result.diff_lines) |diff_lines| {
136136
if (diff_lines.count > 0) {
137-
for (diff_lines.getItems()) |line| {
138-
stdout.print("{d}\n", .{line}) catch {};
137+
stdout.print(";", .{}) catch {};
138+
for (diff_lines.getItems(), 0..) |line, i| {
139+
if (i > 0) stdout.print(",", .{}) catch {};
140+
stdout.print("{d}", .{line}) catch {};
139141
}
140142
}
141143
}
144+
stdout.print("\n", .{}) catch {};
142145
} else {
143146
print("Found {d} different pixels ({d:.2}%)\n", .{ pixel_result.diff_count, pixel_result.diff_percentage });
144147
if (args.diff_lines) {

test/node-binding.test.cjs

Lines changed: 46 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -3,33 +3,27 @@ const test = require("ava");
33
const { compare } = require("../npm_package/odiff");
44

55
const IMAGES_PATH = path.resolve(__dirname, "..", "images");
6-
const BINARY_PATH = path.resolve(
7-
__dirname,
8-
"..",
9-
"zig-out",
10-
"bin",
11-
"odiff"
12-
);
6+
const BINARY_PATH = path.resolve(__dirname, "..", "zig-out", "bin", "odiff");
137

148
console.log(`Testing binary ${BINARY_PATH}`);
159

1610
const options = {
1711
__binaryPath: BINARY_PATH,
18-
}
12+
};
1913

2014
test("Outputs correct parsed result when images different", async (t) => {
2115
const result = await compare(
2216
path.join(IMAGES_PATH, "donkey.png"),
2317
path.join(IMAGES_PATH, "donkey-2.png"),
2418
path.join(IMAGES_PATH, "diff.png"),
25-
options
19+
options,
2620
);
2721

2822
t.is(result.reason, "pixel-diff");
29-
t.true(typeof result.diffCount === 'number');
23+
t.true(typeof result.diffCount === "number");
3024
t.true(result.diffCount > 0);
3125
console.log(`Found ${result.diffCount} different pixels`);
32-
})
26+
});
3327

3428
test("Correctly works with reduceRamUsage", async (t) => {
3529
const result = await compare(
@@ -39,11 +33,11 @@ test("Correctly works with reduceRamUsage", async (t) => {
3933
{
4034
...options,
4135
reduceRamUsage: true,
42-
}
36+
},
4337
);
4438

4539
t.is(result.reason, "pixel-diff");
46-
t.true(typeof result.diffCount === 'number');
40+
t.true(typeof result.diffCount === "number");
4741
t.true(result.diffCount > 0);
4842
});
4943

@@ -55,11 +49,11 @@ test("Correctly parses threshold", async (t) => {
5549
{
5650
...options,
5751
threshold: 0.5,
58-
}
52+
},
5953
);
6054

6155
t.is(result.reason, "pixel-diff");
62-
t.true(typeof result.diffCount === 'number');
56+
t.true(typeof result.diffCount === "number");
6357
t.true(result.diffCount > 0);
6458
});
6559

@@ -71,11 +65,11 @@ test("Correctly parses antialiasing", async (t) => {
7165
{
7266
...options,
7367
antialiasing: true,
74-
}
68+
},
7569
);
7670

7771
t.is(result.reason, "pixel-diff");
78-
t.true(typeof result.diffCount === 'number');
72+
t.true(typeof result.diffCount === "number");
7973
t.true(result.diffCount > 0);
8074
});
8175

@@ -100,32 +94,32 @@ test("Correctly parses ignore regions", async (t) => {
10094
y2: 1334,
10195
},
10296
],
103-
}
97+
},
10498
);
10599

106100
// With our placeholder images, this might still show differences
107101
// but the test should at least run without errors
108-
t.true(typeof result.match === 'boolean');
102+
t.true(typeof result.match === "boolean");
109103
});
110104

111105
test("Outputs correct parsed result when images different for cypress image", async (t) => {
112106
const result = await compare(
113107
path.join(IMAGES_PATH, "www.cypress.io.png"),
114108
path.join(IMAGES_PATH, "www.cypress.io-1.png"),
115109
path.join(IMAGES_PATH, "diff.png"),
116-
options
110+
options,
117111
);
118112

119113
// Our placeholder implementation returns synthetic data, so we just check structure
120-
t.true(typeof result.match === 'boolean');
114+
t.true(typeof result.match === "boolean");
121115
});
122116

123117
test("Correctly handles same images", async (t) => {
124118
const result = await compare(
125119
path.join(IMAGES_PATH, "donkey.png"),
126120
path.join(IMAGES_PATH, "donkey.png"),
127121
path.join(IMAGES_PATH, "diff.png"),
128-
options
122+
options,
129123
);
130124

131125
// With placeholder C implementation, identical images should match
@@ -139,8 +133,8 @@ test("Correctly outputs diff lines", async (t) => {
139133
path.join(IMAGES_PATH, "diff.png"),
140134
{
141135
captureDiffLines: true,
142-
...options
143-
}
136+
...options,
137+
},
144138
);
145139

146140
t.is(result.match, false);
@@ -158,10 +152,35 @@ test("Returns meaningful error if file does not exist and noFailOnFsErrors", asy
158152
{
159153
...options,
160154
noFailOnFsErrors: true,
161-
}
155+
},
162156
);
163157

164158
t.is(result.match, false);
165159
// Our error handling might be different, but it should handle the case gracefully
166-
t.true(['file-not-exists', 'pixel-diff'].includes(result.reason));
167-
});
160+
t.true(["file-not-exists", "pixel-diff"].includes(result.reason));
161+
});
162+
163+
test("Correctly calculates and outputs diff percentage", async (t) => {
164+
const result = await compare(
165+
path.join(__dirname, "png", "orange.png"),
166+
path.join(__dirname, "png", "orange_diff.png"),
167+
path.join(IMAGES_PATH, "diff.png"),
168+
options,
169+
);
170+
171+
t.is(result.match, false);
172+
t.is(result.reason, "pixel-diff");
173+
174+
t.true(typeof result.diffPercentage === "number");
175+
t.true(result.diffPercentage > 0);
176+
t.true(typeof result.diffCount === "number");
177+
t.true(result.diffCount > 0);
178+
179+
const expectedPercentage = (result.diffCount / (510 * 234)) * 100;
180+
t.true(Math.abs(result.diffPercentage - expectedPercentage) < 0.01);
181+
182+
console.log(
183+
`Percentage test: ${result.diffCount} pixels (${result.diffPercentage}%)`,
184+
);
185+
});
186+

vcpkg.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"$schema": "https://raw.githubusercontent.com/microsoft/vcpkg-tool/main/docs/vcpkg.schema.json",
33
"builtin-baseline": "fe1cde61e971d53c9687cf9a46308f8f55da19fa",
4-
"dependencies": ["libspng", "tiff", "libjpeg-turbo"]
4+
"dependencies": ["libspng", "tiff", "libjpeg-turbo", "liblzma"]
55
}
66

0 commit comments

Comments
 (0)