Skip to content

Conversation

@wgevaert
Copy link

Proposed changes

See #322

Types of changes

What types of changes does your code introduce?

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation fix or enhancement (no code was touched)
  • Other (CI, dependencies, etc., please describe)

Checklist:

  • I have checked to ensure there aren't other open Issues or Pull Requests for the same update/change
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)
  • Any dependent changes have been merged and published in downstream modules There are none
  • Static analysis and unit tests pass locally with my changes

@codemasher
Copy link
Member

Hey, I haven't had a close look at the specification yet, but I see that there are multiple formats, so I'm thinking what the best way to abstract is (QRNetpbmAbstract -> QRPbm, ...). I'll add my thoughts to the code.

@codemasher
Copy link
Member

codemasher commented Dec 16, 2025

I just noticed that the binary output looks a bit weird in GIMP:
image

edit: leaving this here for reference: https://gitlab.gnome.org/GNOME/gimp/-/blob/master/plug-ins/common/file-pnm.c

@wgevaert
Copy link
Author

I just noticed that the binary output looks a bit weird in GIMP: image

You added a comment in the binary-data section. It is interpreted as binary data. Note that the spec gives a reason why not to use comments:

The PBM format is so simple that many programmers write programs to process it directly, without using a library such as libnetpbm, reverse engineering a subset of the format from examples rather than using the specification. Therefore, if you want to produce an image that as many programs as possible can read, you should stick to the following subset.

The image consists of the following bytes:

  1. "P4" followed by newline
  2. width and height separated by a space (e.g. "50 100"), followed by newline
  3. The raster

(Note that this subset does not mention comments)

@codemasher
Copy link
Member

Oh that's right! It wasn't the comment that's the issue but its position - it must be between the magic number and the size - GIMP does the same. Also in the GIMP source there is a note that the line length of the ASCII source must not exceed 70 characters (?!).

$line .= str_repeat($isDark ? '1' : '0', $this->scale);
}
// Lines should not be longer than 70 chars
$line = implode("\n", str_split($line,70))."\n";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't you just use the example I provided? This here would split a single line to multiples of 70 and leave multiple shorter lines then, which is not intended.

Copy link
Author

@wgevaert wgevaert Dec 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer if using plain format, when you open the image with a text reader like less or notepad, the image reads as if reading on a 70-tokens-wide terminal. Such that if your image is less than 70 tokens wide, you can kind-of see it when reading it, and if it is less than 140 tokens wide, you can still make it out a bit.
The few extra newline characters don't increase the image size that much, since ascii is already a lot of superfluous bits anyways.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's still 2 unnecessary function calls within a loop that can be avoided, while at the same time producing a cleaner output similar to GIMP:

P1
# Created by GIMP version 3.0.4 PNM plug-in
33 33
0000000000000000000000000000000000000000000000000000000000000000000000
0000000000000000000000000000000000000000000000000000000000000000001111
1110000101001011111110000000010000010010011110010000010000000010111010
1100100100101110100000000101110100001110000101110100000000101110100001
1010101011101000000001000001000111111001000001000000001111111010101010
1011111110000000000000000110010101000000000000000011101111101111011110
0010000000000010110011100010010001100000000000111101100011111001110001
1000000001100100011001101011011110000000001011011000100111001000011000
0000000001001110010011111010000000000010010011011010000001011100000000
0010011010110110001010001000000000100010110011010011111111000000000000
0000011010110100010000000000001111111010110110101011000000000001000001
0110000111000110110000000010111010110110101111101110000000010111010000
0011110101110000000000101110101110000000111100100000000100000101000010
0011001011000000001111111010100111000101001000000000000000000000000000
0000000000000000000000000000000000000000000000000000000000000000000000
000000000000000000000000000000000000000

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see why GIMP's output is considered "cleaner" than this:

P1
# Not created by GIMP version 3.0.4 PNM plug-in
33 33
000000000000000000000000000000000
000000000000000000000000000000000
000000000000000000000000000000000
000000000000000000000000000000000
000011111110000101001011111110000
000010000010010011110010000010000
000010111010110010010010111010000
000010111010000111000010111010000
000010111010000110101010111010000
000010000010001111110010000010000
000011111110101010101011111110000
000000000000110010101000000000000
000011101111101111011110001000000
000001011001110001001000110000000
000011110110001111100111000110000
000011001000110011010110111100000
000010110110001001110010000110000
000000001001110010011111010000000
000010010011011010000001011100000
000001001101011011000101000100000
000010001011001101001111111100000
000000000000110101101000100000000
000011111110101101101010110000000
000010000010110000111000110110000
000010111010110110101111101110000
000010111010000001111010111000000
000010111010111000000011110010000
000010000010100001000110010110000
000011111110101001110001010010000
000000000000000000000000000000000
000000000000000000000000000000000
000000000000000000000000000000000
000000000000000000000000000000000

The image data above looks like a QR code if you look from far away and squint your eyes a bit. The GIMP image is difficult to see what it is supposed to be.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try your code with a QR version >= 14 and see.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants