-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Optimize alphanumeric encoder and galois field constants #684
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
9a9d539
134d7f8
0ebc8e4
74eaa58
ff57ed9
e1bf23d
3dabc79
8fd262c
b6dfc37
af8b4cc
5e32a4b
8d7f12f
d9be37d
4ee6742
3b4c988
f2ed9bb
6b92bab
2417a57
f1c0eea
599af03
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 |
|---|---|---|
|
|
@@ -175,7 +175,7 @@ private static EncodingMode SelectInitialMode(string text, int startPos, int ver | |
| if (numericCount < threshold) | ||
| { | ||
| var nextPos = startPos + numericCount; | ||
| if (nextPos < text.Length && !IsAlphanumericNonDigit(text[nextPos])) | ||
| if (nextPos < text.Length && !IsAlphanumeric(text[nextPos])) | ||
|
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. The next character is known to not be a digit, so it doesn't matter if this checks against IsAlphanumeric or IsAlphanumericNonDigit. Since the new optimzation is faster with the former, it's changed here and below. |
||
| return EncodingMode.Byte; | ||
| } | ||
| // ELSE IF there are less than [7-9] characters followed by data from the exclusive subset of the Alphanumeric character set | ||
|
|
@@ -184,7 +184,7 @@ private static EncodingMode SelectInitialMode(string text, int startPos, int ver | |
| if (numericCount < threshold) | ||
| { | ||
| var nextPos = startPos + numericCount; | ||
| if (nextPos < text.Length && IsAlphanumericNonDigit(text[nextPos])) | ||
| if (nextPos < text.Length && IsAlphanumeric(text[nextPos])) | ||
Shane32 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| return EncodingMode.Alphanumeric; | ||
| } | ||
| return EncodingMode.Numeric; | ||
|
|
@@ -329,9 +329,6 @@ private static int CountConsecutive(string text, int startPos, Func<char, bool> | |
| private static bool IsNumeric(char c) => IsInRange(c, '0', '9'); | ||
|
|
||
| // Checks if a character is alphanumeric (can be encoded in alphanumeric mode). | ||
| private static bool IsAlphanumeric(char c) => IsNumeric(c) || IsAlphanumericNonDigit(c); | ||
|
|
||
| // Checks if a non-digit character can be encoded in alphanumeric mode. | ||
| private static bool IsAlphanumericNonDigit(char c) => AlphanumericEncoder.CanEncodeNonDigit(c); | ||
| private static bool IsAlphanumeric(char c) => AlphanumericEncoder.CanEncode(c); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,14 @@ | |
| <Guid>e668b98b-83bb-4e60-b33c-4fd5ed9c0156</Guid> | ||
| </PropertyGroup> | ||
|
|
||
| <ItemGroup Condition="'$(Configuration)' == 'Release'"> | ||
| <InternalsVisibleTo Include="QRCoderTests, PublicKey=002400000480000094000000060200000024000052534131000400000100010071e74d3f6dcbba7726fcbb7ad05a2dfa3b8e13f9bf2938c05de266a83a7f3c95201b7cdc9e1f88842093ece868c3ec7874e3b8907008763c85711bf0e2e9757a3068385380a757bd52fa77248f227f602b0785e040756ef42dabc7de7f8376847c71b3f20356a9176cc88067583ee3501e61f8aa07bdd70f41418986fb79a1a3" /> | ||
| </ItemGroup> | ||
|
|
||
| <ItemGroup Condition="'$(Configuration)' != 'Release'"> | ||
| <InternalsVisibleTo Include="QRCoderTests" /> | ||
| </ItemGroup> | ||
|
Comment on lines
+19
to
+25
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. What a pain! A strongly-named assembly can only reference another strongly-named assembly. If I make it conditional upon release, then the solution won't build in release mode because the tests can't see the internal members. So I had to make the test project strongly named, and extract the public key to put in the prop here, and it had to be conditional because otherwise in debug mode it couldn't find the assembly with the strong name...
Collaborator
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.
Yeah, that virality sucks.
When it's strong named in DEBUG and RELEASE it should work in all cases?! So no conditional is needed. But it's fine as is too.
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. Codecov can’t inject its code into a strongly named assembly. So to get the coverage reports working I had to disable strong name signing.
Collaborator
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. Ah, I didn't think about this. Thanks for clarification. |
||
|
|
||
| <ItemGroup Condition=" '$(TargetFramework)' == 'net35' OR '$(TargetFramework)' == 'net40' "> | ||
| <Reference Include="PresentationCore" /> | ||
| <Reference Include="PresentationFramework" /> | ||
|
|
||
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 is what I referred to. Just make it unconditional signed.
But we won't change that often (never), so fine as is too.
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.
Codecov - See note above