Optimize AlignmentPatterns

[KrisVandermotten]

Summary

This PR applies a round of optimizations to the calculation and placement of alignment patterns.

First, several improvements reduce the amount of memory allocated by CreateAlignmentPatternTable:

• The AlignmentPattern structure has been removed. It was wrapper around a list of Points and a Version property. As the Version property wasn't used, there is no need for the structure.
• The Point structure has been reduced from two int properties to two byte properties. Given that the result of the method is essentially a table of Points, this reduces the amount of memory needed to store the actual data by a factor 4.
• The resulting table has been changed from a Dictionary to an array.
• The entries in the table have been changed from List<Point> to Point[].

All of the above contribute to less memory being needed to store the result and keep it referenced for later usage. In addition, some memory allocations have been reduced in the process to produce that result:

• On modern .NET, the allocation (and initialization) of alignmentPatternBaseValues is avoided. It's values are now bytes instead of ints.
• A single List<Point> is used as a buffer to store intermediate results, instead of a list per version.

Some changes have been made to reduce execution time:

• The calculation for version 1 is avoided, as it's empty anyway.
• The inner loops are terminated as soon as they can, instead of always looping 7 times.
• The unnecessary check whether a calculated point was not already present in the points intermediate list has been removed. As a result, the IEquatable<Point> implementation is no longer needed and removed.
• The outer loop is now a simple loop over the versions. Not only does this improve the clarity of the code, it also avoids an integer division per version.
• The numbers in alignmentPatternBaseValues are now the coordinates of the upper left corners of the alignment patters, not the center points. This saves two substractions per point.

I ran a simple benchmark to measure the results.

BenchmarkDotNet v0.13.12, Windows 11 (10.0.26200.7462)
11th Gen Intel Core i7-1165G7 2.80GHz, 1 CPU, 8 logical and 4 physical cores
.NET SDK 10.0.302
  [Host]     : .NET 8.0.29 (8.0.2926.32403), X64 RyuJIT AVX-512F+CD+BW+DQ+VL+VBMI
  DefaultJob : .NET 8.0.29 (8.0.2926.32403), X64 RyuJIT AVX-512F+CD+BW+DQ+VL+VBMI

On master:

Method	Mean	Error	StdDev	Gen0	Gen1	Allocated
CreateAlignmentPatternTable	12.63 us	0.225 us	0.211 us	3.3722	0.2441	20.74 KB

This PR:

Method	Mean	Error	StdDev	Gen0	Gen1	Allocated
CreateAlignmentPatternTable	1.811 us	0.0242 us	0.0226 us	0.5455	0.0057	3.35 KB

Memory allocations have been reduced by a factor 6. Even though the method is executed only once, it's nice to see that it is now 7 times faster. More optimizations are possible but deemed unnecessary, as they would also reduce the readability of the code.

Secondly, some improvements have been made to the PlaceAlignmentPatterns method.

Obviously, placement already benefits from the fact that AlignmentPatterns.FromVersion now returns a Point[] instead of (a struct around) a List<Point>, and does so by indexing into an array instead of a dictionary.

On top of that:

• The y and x loops have been swapped to improve data locality, and avoid redundant indexing into the ModuleMatrix array.
• The inner x loop has been unrolled, primarily to reduce branches and branch mispredictions.
• A redundant construction of alignmentPatternRect is avoided.

The results are visible in the existing QRCodeGeneratorBenchmark.

On master:

Method	Mean	Error	StdDev	Gen0	Allocated
CreateQRCode	126.8 us	2.52 us	3.28 us	0.6104	4.02 KB
CreateQRCodeMultiMode	984.3 us	19.62 us	31.69 us	-	7.22 KB
CreateQRCodeLong	2,330.6 us	43.59 us	70.39 us	-	10.89 KB
CreateQRCodeLongest	13,594.9 us	268.84 us	298.82 us	-	43.84 KB

This PR:

Method	Mean	Error	StdDev	Gen0	Allocated
CreateQRCode	121.9 us	2.43 us	4.96 us	0.6104	4.02 KB
CreateQRCodeMultiMode	924.9 us	18.14 us	20.90 us	0.9766	7.22 KB
CreateQRCodeLong	2,141.0 us	41.43 us	46.04 us	-	10.89 KB
CreateQRCodeLongest	12,644.3 us	193.28 us	171.34 us	-	43.84 KB

Test plan

All modified code is exercised by existing unit tests. Benchmarks show the performance benefit.

Summary by CodeRabbit

Improvements

• Optimized QR code alignment pattern processing for faster, more efficient generation.
• Reduced memory usage when handling alignment coordinates.
• Improved alignment pattern generation across supported QR code versions, including Micro QR variants.
• Preserved the existing 5×5 alignment pattern appearance and placement.
• Added validation to help ensure alignment data remains within expected limits.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: dbb44a43-4e4b-462e-ad0c-5c730a894ef6

📥 Commits

Reviewing files that changed from the base of the PR and between de2309f and 1a9d458.

📒 Files selected for processing (1)

• QRCoder/QRCodeGenerator/Point.cs

🚧 Files skipped from review as they are similar to previous changes (1)

• QRCoder/QRCodeGenerator/Point.cs

Included review availability: Your plan includes up to 8 reviews per rolling hour; 7 remain after this review.

---

📝 Walkthrough

Walkthrough

The PR replaces dictionary-backed alignment data with version-indexed point arrays, changes point coordinates to bytes, and updates alignment pattern placement to consume arrays and render reserved 5×5 regions by row.

Changes

Alignment pattern pipeline

Layer / File(s)	Summary
Byte-based point representation QRCoder/QRCodeGenerator/Point.cs	Point now stores byte-based coordinates and no longer implements custom equality members.
Alignment coordinate table QRCoder/QRCodeGenerator/AlignmentPatterns.cs	AlignmentPatterns now returns version-indexed Point[] values and builds versions 2–40 from byte coordinate data.
Alignment pattern placement QRCoder/QRCodeGenerator.cs, QRCoder/QRCodeGenerator/ModulePlacer.cs	QRCodeGenerator passes alignment coordinates directly to ModulePlacer. ModulePlacer reserves non-overlapping areas and renders each pattern by row.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 1a9d4

This PR optimizes alignment-pattern calculation and placement without supplied evidence of a concrete correctness, security, availability, or deployment risk. It is merge-ready after normal checks and review.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the primary change: optimizing alignment-pattern calculation and placement.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[gfoidl]

Quoting coderabbitai:

No actionable comments were generated in the recent review. 🎉

😃 one AI hasn't found something reviewing a other AI¹...(same model in the background?).

Footnotes

1. note the "Summary by CodeRabbit" in the TOP got added later on by CodeRabbit. ↩

[KrisVandermotten]

😃 one AI hasn't found something reviewing a other AI...(same model in the background?).

Not sure what "other AI" you're referring to? No AI was used in making this PR, nor its description.

[Shane32]

• The unnecessary check whether a calculated point was not already present in the points intermediate list has been removed. As a result, the IEquatable<Point> implementation is no longer needed and removed.

Makes sense, but should we leave the IEquatable implementation anyway? It really hurts performance if needed and missing. See #509 (comment) But if unneeded we can remove it ....

[KrisVandermotten]

• The unnecessary check whether a calculated point was not already present in the points intermediate list has been removed. As a result, the IEquatable<Point> implementation is no longer needed and removed.

Makes sense, but should we leave the IEquatable implementation anyway? It really hurts performance if needed and missing. See #509 (comment) But if unneeded we can remove it ....

The QRCodeGenerator.Point struct is private, so there is complete control over where and how it is used. It's Equals and GetHashCode methods aren't called. There are other such structs, e.g. QRCodeGenerator.ECCInfo and QRCodeGenerator.CodewordBlock that also don't have an IEquatble<T> implementation. That's why I removed the unused methods in QRCodeGenerator.Point as well.

Finally, benchmarks show an improvement in performance, not a regression.

[Shane32]

Fair. Just indicating that Point.Equals wasn't previously called directly; it was a hidden performance issue due to the call to List<>.Contains. Let's at least add a comment to the struct to that effect, something like:

Be sure to add an IEquatable<> implementation if calling List<Point>.Contains or similar methods requiring equality comparison.

to help prevent the same mistake in the future.

[KrisVandermotten]

Fair. Just indicating that Point.Equals wasn't previously called directly; it was a hidden performance issue due to the call to List<>.Contains.

I do understand. That's why I wrote: "The unnecessary check whether a calculated point was not already present in the points intermediate list has been removed. As a result, the IEquatable implementation is no longer needed and removed."

Let's at least add a comment to the struct to that effect, something like:

Be sure to add an IEquatable<> implementation if calling List<Point>.Contains or similar methods requiring equality comparison.

to help prevent the same mistake in the future.

Just to be clear: there are nine such structs, do you suggest to add such a comment to all of them, or just to the Point struct?

[Shane32]

Let's just add a comment for the Point struct and call it a day.

[Shane32]

Thanks! I'll merge once tests pass. Thanks for your help!!