-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Make Postscript renderer deterministic #643
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -101,7 +101,7 @@ public string GetGraphic(Size viewBox, Color darkColor, Color lightColor, bool d | |
| var pointsPerModule = (double)Math.Min(viewBox.Width, viewBox.Height) / (double)drawableModulesCount; | ||
|
|
||
| string psFile = string.Format(PS_HEADER, new object[] { | ||
| DateTime.Now.ToString("s"), CleanSvgVal(viewBox.Width), CleanSvgVal(pointsPerModule), | ||
| CleanSvgVal(viewBox.Width), CleanSvgVal(pointsPerModule), | ||
| epsFormat ? "EPSF-3.0" : string.Empty | ||
| }); | ||
| psFile += string.Format(PS_FUNCTIONS, new object[] { | ||
|
|
@@ -130,68 +130,75 @@ public string GetGraphic(Size viewBox, Color darkColor, Color lightColor, bool d | |
| /// <returns>Returns the cleaned string representation of the double value.</returns> | ||
| private string CleanSvgVal(double input) => input.ToString(System.Globalization.CultureInfo.InvariantCulture); | ||
|
|
||
| private const string PS_HEADER = @"%!PS-Adobe-3.0 {3} | ||
| %%Creator: QRCoder.NET | ||
| %%Title: QRCode | ||
| %%CreationDate: {0} | ||
| %%DocumentData: Clean7Bit | ||
| %%Origin: 0 | ||
| %%DocumentMedia: Default {1} {1} 0 () () | ||
| %%BoundingBox: 0 0 {1} {1} | ||
| %%LanguageLevel: 2 | ||
| %%Pages: 1 | ||
| %%Page: 1 1 | ||
| %%EndComments | ||
| %%BeginConstants | ||
| /sz {1} def | ||
| /sc {2} def | ||
| %%EndConstants | ||
| %%BeginFeature: *PageSize Default | ||
| << /PageSize [ sz sz ] /ImagingBBox null >> setpagedevice | ||
| %%EndFeature | ||
| "; | ||
|
|
||
| private const string PS_FUNCTIONS = @"%%BeginFunctions | ||
| /csquare {{ | ||
| newpath | ||
| 0 0 moveto | ||
| 0 1 rlineto | ||
| 1 0 rlineto | ||
| 0 -1 rlineto | ||
| closepath | ||
| setrgbcolor | ||
| fill | ||
| }} def | ||
| /f {{ | ||
| {0} {1} {2} csquare | ||
| 1 0 translate | ||
| }} def | ||
| /b {{ | ||
| 1 0 translate | ||
| }} def | ||
| /background {{ | ||
| {3} {4} {5} csquare | ||
| }} def | ||
| /nl {{ | ||
| -{6} -1 translate | ||
| }} def | ||
| %%EndFunctions | ||
| %%BeginBody | ||
| 0 0 moveto | ||
| gsave | ||
| sz sz scale | ||
| background | ||
| grestore | ||
| gsave | ||
| sc sc scale | ||
| 0 {6} 1 sub translate | ||
| "; | ||
|
|
||
| private const string PS_FOOTER = @"%%EndBody | ||
| grestore | ||
| showpage | ||
| %%EOF | ||
| "; | ||
| // Note: line terminations here will encode differently based on which platform QRCoder was compiled on (CRLF vs LF); | ||
| // however, PostScript interpreters should handle both equally well. | ||
|
Comment on lines
+133
to
+134
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion | 🟠 Major 🧩 Analysis chainVerify cross-platform test determinism. The comment correctly notes that line terminations will differ based on compilation platform (CRLF on Windows, LF on Unix). While PostScript interpreters handle both, this could cause approval test failures when developers compile on different platforms and the tests perform exact string comparisons. Consider one of these approaches:
Run this script to check how the approval tests handle line ending differences: 🏁 Script executed: #!/bin/bash
# Check if approval tests normalize line endings
# Search for the ShouldMatchApproved implementation and configuration
rg -n "ShouldMatchApproved|NoDiff" --type cs -A 5 -B 5
# Look for any line ending normalization in test utilities
rg -n "LineEnding|CRLF|\\\\r\\\\n|normalize" --type csLength of output: 66105 Ensure deterministic LF line endings in PostScript output
🤖 Prompt for AI Agents |
||
| private const string PS_HEADER = """ | ||
| %!PS-Adobe-3.0 {2} | ||
| %%Creator: QRCoder.NET | ||
| %%Title: QRCode | ||
| %%DocumentData: Clean7Bit | ||
| %%Origin: 0 | ||
| %%DocumentMedia: Default {0} {0} 0 () () | ||
| %%BoundingBox: 0 0 {0} {0} | ||
| %%LanguageLevel: 2 | ||
| %%Pages: 1 | ||
| %%Page: 1 1 | ||
| %%EndComments | ||
| %%BeginConstants | ||
| /sz {0} def | ||
| /sc {1} def | ||
| %%EndConstants | ||
| %%BeginFeature: *PageSize Default | ||
| << /PageSize [ sz sz ] /ImagingBBox null >> setpagedevice | ||
| %%EndFeature | ||
|
|
||
| """; | ||
|
|
||
| private const string PS_FUNCTIONS = """ | ||
| %%BeginFunctions | ||
| /csquare {{ | ||
| newpath | ||
| 0 0 moveto | ||
| 0 1 rlineto | ||
| 1 0 rlineto | ||
| 0 -1 rlineto | ||
| closepath | ||
| setrgbcolor | ||
| fill | ||
| }} def | ||
| /f {{ | ||
| {0} {1} {2} csquare | ||
| 1 0 translate | ||
| }} def | ||
| /b {{ | ||
| 1 0 translate | ||
| }} def | ||
| /background {{ | ||
| {3} {4} {5} csquare | ||
| }} def | ||
| /nl {{ | ||
| -{6} -1 translate | ||
| }} def | ||
| %%EndFunctions | ||
| %%BeginBody | ||
| 0 0 moveto | ||
| gsave | ||
| sz sz scale | ||
| background | ||
| grestore | ||
| gsave | ||
| sc sc scale | ||
| 0 {6} 1 sub translate | ||
|
|
||
| """; | ||
|
|
||
| private const string PS_FOOTER = """ | ||
| %%EndBody | ||
| grestore | ||
| showpage | ||
| %%EOF | ||
|
|
||
| """; | ||
| } | ||
|
|
||
| /// <summary> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,7 +9,7 @@ public void can_render_postscript_qrcode_simple() | |
| var gen = new QRCodeGenerator(); | ||
| var data = gen.CreateQrCode("This is a quick test! 123#?", QRCodeGenerator.ECCLevel.L); | ||
| var ps = new PostscriptQRCode(data).GetGraphic(5); | ||
| RemoveCreationDate(ps).ShouldMatchApproved(x => x.NoDiff()); | ||
| ps.ShouldMatchApproved(x => x.NoDiff()); | ||
|
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tests previously removed the creation date from the verified files. So all tests continue to match with no change to test result data. |
||
| } | ||
|
|
||
| [Fact] | ||
|
|
@@ -19,7 +19,7 @@ public void can_render_postscript_qrcode_eps() | |
| var gen = new QRCodeGenerator(); | ||
| var data = gen.CreateQrCode("This is a quick test! 123#?", QRCodeGenerator.ECCLevel.L); | ||
| var ps = new PostscriptQRCode(data).GetGraphic(5, true); | ||
| RemoveCreationDate(ps).ShouldMatchApproved(x => x.NoDiff()); | ||
| ps.ShouldMatchApproved(x => x.NoDiff()); | ||
| } | ||
|
|
||
| [Fact] | ||
|
|
@@ -29,7 +29,7 @@ public void can_render_postscript_qrcode_size() | |
| var gen = new QRCodeGenerator(); | ||
| var data = gen.CreateQrCode("This is a quick test! 123#?", QRCodeGenerator.ECCLevel.L); | ||
| var ps = new PostscriptQRCode(data).GetGraphic(new Size(33, 33)); | ||
| RemoveCreationDate(ps).ShouldMatchApproved(x => x.NoDiff()); | ||
| ps.ShouldMatchApproved(x => x.NoDiff()); | ||
| } | ||
|
|
||
| [Fact] | ||
|
|
@@ -39,7 +39,7 @@ public void can_render_postscript_qrcode_size_no_quiet_zones() | |
| var gen = new QRCodeGenerator(); | ||
| var data = gen.CreateQrCode("This is a quick test! 123#?", QRCodeGenerator.ECCLevel.L); | ||
| var ps = new PostscriptQRCode(data).GetGraphic(new Size(50, 50), false); | ||
| RemoveCreationDate(ps).ShouldMatchApproved(x => x.NoDiff()); | ||
| ps.ShouldMatchApproved(x => x.NoDiff()); | ||
| } | ||
|
|
||
| [Fact] | ||
|
|
@@ -49,15 +49,6 @@ public void can_render_postscript_qrcode_colors() | |
| var gen = new QRCodeGenerator(); | ||
| var data = gen.CreateQrCode("This is a quick test! 123#?", QRCodeGenerator.ECCLevel.L); | ||
| var ps = new PostscriptQRCode(data).GetGraphic(5, Color.Red, Color.Blue); | ||
| RemoveCreationDate(ps).ShouldMatchApproved(x => x.NoDiff()); | ||
| } | ||
|
|
||
| private static string RemoveCreationDate(string text) | ||
| { | ||
| // Regex pattern to match lines that start with %%CreationDate: followed by any characters until the end of the line | ||
| string pattern = @"%%CreationDate:.*\r?\n?"; | ||
|
|
||
| // Use Regex.Replace to remove matching lines | ||
| return Regex.Replace(text, pattern, string.Empty, RegexOptions.Multiline); | ||
| ps.ShouldMatchApproved(x => x.NoDiff()); | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This "CreationDate" line is the only removed line; the constant is otherwise identical. (although {1} becomes {0} etc)