-
-
Notifications
You must be signed in to change notification settings - Fork 324
Add QRPbm output format for Portable BitMap outputs #323
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
base: main
Are you sure you want to change the base?
Changes from all commits
0a069c3
e70a744
f5a2d2b
aa748d2
9b94cfe
aff03cc
78ae3a3
03802e8
ddef97d
e5f60fd
a520c0f
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 |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| <?php | ||
| /** | ||
| * Class QRPbm | ||
| * | ||
| * @created 11.12.2025 | ||
| * @author wgevaert | ||
| * @copyright 2025 wgevaert | ||
| * @license MIT | ||
| */ | ||
| declare(strict_types=1); | ||
|
|
||
| namespace chillerlan\QRCode\Output; | ||
|
|
||
| use chillerlan\QRCode\Output\QROutputAbstract; | ||
| use UnexpectedValueException; | ||
|
|
||
| abstract class QRNetpbmBitmapAbstract extends QROutputAbstract { | ||
| public const MIME_TYPE = 'image/x-portable-bitmap'; | ||
|
|
||
| protected function prepareModuleValue( mixed $value ): mixed { | ||
| if ( !is_bool( $value ) ) { | ||
| throw new UnexpectedValueException( 'Expected boolean module value' ); | ||
| } | ||
| return $value; | ||
| } | ||
|
|
||
| protected function getDefaultModuleValue( bool $isDark ): bool { | ||
| return $isDark; | ||
| } | ||
|
|
||
| public static function moduleValueIsValid( mixed $value ): bool { | ||
| return is_bool( $value ); | ||
| } | ||
|
|
||
| abstract protected function getHeader(): string; | ||
|
|
||
| protected function getBody(): string { | ||
| $body = ''; | ||
| foreach ($this->matrix->getBooleanMatrix() as $row) { | ||
| $line = ''; | ||
| foreach ($row as $isDark) { | ||
| $line .= str_repeat( $isDark ? '1' : '0', $this->scale ); | ||
| } | ||
| $line .= "\n"; | ||
| $body .= str_repeat( $line, $this->scale ); | ||
| } | ||
| return trim($body,"\n"); | ||
| } | ||
|
|
||
| public function dump( string|null $file = null ): mixed { | ||
| $qrString = $this->getHeader()."\n" | ||
| .$this->length.' '.$this->length."\n".$this->getBody(); | ||
|
|
||
| $this->saveToFile( $qrString, $file ); | ||
|
|
||
| if ( $this->options->outputBase64 ) { | ||
| $qrString = $this->toBase64DataURI( $qrString ); | ||
| } | ||
|
|
||
| return $qrString; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| <?php | ||
| /** | ||
| * Class QRPbm | ||
| * | ||
| * @created 11.12.2025 | ||
| * @author wgevaert | ||
| * @copyright 2025 wgevaert | ||
| * @license MIT | ||
| */ | ||
| declare(strict_types=1); | ||
|
|
||
| namespace chillerlan\QRCode\Output; | ||
|
|
||
| use chillerlan\QRCode\Output\QRNetpbmBitmapAbstract; | ||
|
|
||
| class QRNetpbmBitmapAscii extends QRNetpbmBitmapAbstract { | ||
| protected function getHeader(): string { | ||
| return 'P1'; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| <?php | ||
| /** | ||
| * Class QRPbm | ||
| * | ||
| * @created 11.12.2025 | ||
| * @author wgevaert | ||
| * @copyright 2025 wgevaert | ||
| * @license MIT | ||
| */ | ||
| declare(strict_types=1); | ||
|
|
||
| namespace chillerlan\QRCode\Output; | ||
|
|
||
| use chillerlan\QRCode\Output\QRNetpbmBitmapAbstract; | ||
|
Member
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. I don't think we need separate classes for ASCII and binary here - it's quite excessive for a file format that's barely used. My initial recommendation was an abstract class for all netpbm based formats with child classes for each PBM, PGM, PPM and PAM. The conversion to binary can be done in the loop that creates the body (avoiding to loop over the same data 3 times), with a switch in the options, same goes for the header.
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. I don't see how adding an extra class is more "excessive" than splitting the logic in a single class everywhere to do two different things based on a member variable. Classes and functions should only do one thing, so having two different classes here, each for doing the single thing they're named after, makes very much sense to me. Looping two times over data, each time doing a single operation, or looping once over data, each time doing two operations, is virtually the same complexity. (I only see two loops tbh).
Member
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.
It is excessive because it would spawn at least 8 new classes for an image format that barely has practical use and for something that can be handled within the abstract parent. I'm fine with a class for each image format, that makes 4 new classes plus an abstract parent.
By that logic we'd also need separate classes for base64 formats. Since the ASCII -> binary encoding is something common to all netpbm formats, it can be abstracted to the common parent (or be done in the creation loop) with a switch in the options, similar to base64 output.
Right, I see the other loops are only over the rows, but even these can be reduced. It's not so much about the complexity, but the performance impact of 2 additional loops with overhead from How about something like this: abstract class QRNetpbmAbstract extends QROutputAbstract{
protected const HEADER_ASCII = '';
protected const HEADER_BINARY = '';
protected function getMagicNumber():string{
// @todo: add option to QROptionsTrait
return $this->options->netpbmUseBinary ? static::HEADER_BINARY : static::HEADER_ASCII;
}
protected function getHeader():string{
// comment can be a const or an option or jsut a var here idc
$comment = 'created by https://github.com/chillerlan/php-qrcode';
return sprintf("%s\n%s %s\n# %s\n", $this->getMagicNumber(), $this->length, $this->length, $comment);
}
abstract protected function getBody():string;
public function dump(string|null $file = null):string{
$qrString = $this->getHeader().$this->getBody();
$this->saveToFile($qrString, $file);
if($this->options->outputBase64){
$qrString = $this->toBase64DataURI($qrString);
}
return $qrString;
}
protected function asciiBinToBinary(array $bodyChunks):string{
$bin = '';
foreach($bodyChunks as $row){
foreach(str_split($row, 8) as $chunk){
$bin .= pack('C', bindec($chunk));
}
}
return $bin;
}
}With a child class for PBM: class QRPbm extends QRNetpbmAbstract{
public const MIME_TYPE = 'image/x-portable-bitmap';
protected const HEADER_ASCII = 'P1';
protected const HEADER_BINARY = 'P4';
protected function prepareModuleValue(mixed $value):mixed{
// noop (required by abstract)
return null;
}
protected function getDefaultModuleValue(bool $isDark):mixed{
// noop (required by abstract)
return null;
}
public static function moduleValueIsValid(mixed $value):bool{
// noop (required by interface)
return true;
}
protected function setModuleValues():void{
// noop (disables use of the above methods)
}
protected function getBody():string{
$bodyChunks = [];
foreach($this->matrix->getBooleanMatrix() as $row){
$line = implode('', array_map(fn(bool $isDark):string => str_repeat((string)(int)$isDark, $this->scale), $row));
for($i = 0; $i < $this->scale; $i++){
$bodyChunks[] = $line;
}
}
if($this->options->netpbmUseBinary){
return $this->asciiBinToBinary($bodyChunks);
}
return implode("\n", $bodyChunks);
}
}I haven't done benchmarking yet, but as you see we've removed the extra |
||
|
|
||
| class QRNetpbmBitmapBinary extends QRNetpbmBitmapAbstract { | ||
| protected function getHeader(): string { | ||
| return 'P4'; | ||
| } | ||
|
|
||
| protected function getBody(): string { | ||
| $asciiBody = parent::getBody(); | ||
| $body = ''; | ||
| foreach (explode("\n", $asciiBody) as $row) { | ||
| $body .= $this->asciiBinToBinary( $row ); | ||
| } | ||
| return $body; | ||
| } | ||
|
|
||
| private function asciiBinToBinary( string $asciiBin ): string { | ||
| $binaryString = ''; | ||
| foreach(str_split( $asciiBin, 8 ) as $currentChunk) { | ||
| $currentChunk = str_pad( $currentChunk, 8, '0' ); | ||
| $binaryString .= pack( 'C', bindec( $currentChunk ) ); | ||
| } | ||
| return $binaryString; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| <?php | ||
| /** | ||
| * Class QRPbmTest | ||
| * | ||
| * @created 11.12.2025 | ||
| * @author wgevaert | ||
| * @copyright 2025 wgevaert | ||
| * @license MIT | ||
| */ | ||
| declare(strict_types=1); | ||
|
|
||
| namespace chillerlan\QRCodeTest\Output; | ||
|
|
||
| use chillerlan\QRCode\QROptions; | ||
| use chillerlan\QRCode\Data\QRMatrix; | ||
| use chillerlan\QRCode\Output\{QROutputInterface, QRNetpbmBitmapAscii}; | ||
| use chillerlan\Settings\SettingsContainerInterface; | ||
| use PHPUnit\Framework\Attributes\Test; | ||
|
|
||
| /** | ||
| * Tests the QRNetpbmBitmapAscii output class | ||
| */ | ||
| final class QRNetpbmBitmapAsciiTest extends QROutputTestAbstract { | ||
|
|
||
| protected function getOutputInterface( | ||
| SettingsContainerInterface|QROptions $options, | ||
| QRMatrix $matrix, | ||
| ):QROutputInterface{ | ||
| return new QRNetpbmBitmapAscii($options, $matrix); | ||
| } | ||
|
|
||
| /** | ||
| * @phpstan-return array<string, array{0: mixed, 1: bool}> | ||
| */ | ||
| public static function moduleValueProvider():array{ | ||
| return [ | ||
| 'invalid: wrong type: array' => [[], false], | ||
| 'invalid: wrong type: string' => ['abc', false], | ||
| 'valid: true' => [true, true], | ||
| 'valid: false' => [false, true], | ||
| ]; | ||
| } | ||
|
|
||
| #[Test] | ||
| public function setModuleValues():void{ | ||
| $this->options->moduleValues = [ | ||
| // data | ||
| QRMatrix::M_DATA_DARK => true, | ||
| QRMatrix::M_DATA => false, | ||
| ]; | ||
|
|
||
| $this->outputInterface = $this->getOutputInterface($this->options, $this->matrix); | ||
| $data = $this->outputInterface->dump(); | ||
|
|
||
| $this::assertStringContainsString('1', $data); | ||
| $this::assertStringContainsString('0', $data); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| <?php | ||
| /** | ||
| * Class QRPbmTest | ||
| * | ||
| * @created 11.12.2025 | ||
| * @author wgevaert | ||
| * @copyright 2025 wgevaert | ||
| * @license MIT | ||
| */ | ||
| declare(strict_types=1); | ||
|
|
||
| namespace chillerlan\QRCodeTest\Output; | ||
|
|
||
| use chillerlan\QRCode\QROptions; | ||
| use chillerlan\QRCode\Data\QRMatrix; | ||
| use chillerlan\QRCode\Output\{QROutputInterface, QRNetpbmBitmapBinary}; | ||
| use chillerlan\Settings\SettingsContainerInterface; | ||
| use PHPUnit\Framework\Attributes\Test; | ||
|
|
||
| /** | ||
| * Tests the QRNetpbmBitmapBinary output class | ||
| */ | ||
| final class QRNetpbmBitmapBinaryTest extends QROutputTestAbstract { | ||
|
|
||
| protected function getOutputInterface( | ||
| SettingsContainerInterface|QROptions $options, | ||
| QRMatrix $matrix, | ||
| ):QROutputInterface{ | ||
| return new QRNetpbmBitmapBinary($options, $matrix); | ||
| } | ||
|
|
||
| /** | ||
| * @phpstan-return array<string, array{0: mixed, 1: bool}> | ||
| */ | ||
| public static function moduleValueProvider():array{ | ||
| return [ | ||
| 'invalid: wrong type: array' => [[], false], | ||
| 'invalid: wrong type: string' => ['abc', false], | ||
| 'valid: true' => [true, true], | ||
| 'valid: false' => [false, true], | ||
| ]; | ||
| } | ||
|
|
||
| #[Test] | ||
| public function setModuleValues():void{ | ||
| $this->options->moduleValues = [ | ||
| // data | ||
| QRMatrix::M_DATA_DARK => true, | ||
| QRMatrix::M_DATA => false, | ||
| ]; | ||
|
|
||
| $this->outputInterface = $this->getOutputInterface($this->options, $this->matrix); | ||
| $data = $this->outputInterface->dump(); | ||
|
|
||
| $this::assertStringContainsString("\0", $data); | ||
| } | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.