Skip to content
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

Added an example with png rounded modules using gd #215

Merged
merged 3 commits into from
Sep 18, 2023

Conversation

livingroot
Copy link
Contributor

Result:

Similar to:
#127

@codemasher
Copy link
Member

Oh wow, this is incredible! Just let me have a quick look over the commit. 🚀

examples/imageRoundedModules.png Outdated Show resolved Hide resolved
examples/pngWithRoundedShapes.php Show resolved Hide resolved
examples/pngWithRoundedShapes.php Outdated Show resolved Hide resolved
examples/pngWithRoundedShapes.php Outdated Show resolved Hide resolved
examples/pngWithRoundedShapes.php Outdated Show resolved Hide resolved
examples/pngWithRoundedShapes.php Show resolved Hide resolved
@codemasher
Copy link
Member

Also, i noticed that your branch is like 6 months behind dev-main - could you please update and check if it still works for you (spoiler: it probably won't and I'm currently trying to find out why).

@codemasher
Copy link
Member

Update: it's because the setPixel() method has ben renamed to module() for consistency across the several output classes.

@livingroot
Copy link
Contributor Author

Thank you for your review, i will fix this issues. But i can't see dev-main branch. Did you mean main?

@codemasher
Copy link
Member

Yes, main it is - dev-main is the composer name - a habit of mine i guess haha.

@livingroot
Copy link
Contributor Author

All done! However, I am not sure about PHPCS complaining about single if conditions like if ($neighbours & (1 << 3)) {. I think it would be even worse for readability if this condition had additional brackets. Should I add them too?

@livingroot
Copy link
Contributor Author

Also, I have checked that everything is working fine

@codemasher
Copy link
Member

All done! However, I am not sure about PHPCS complaining about single if conditions like if ($neighbours & (1 << 3)) {. I think it would be even worse for readability if this condition had additional brackets. Should I add them too?

I agree that this case is a bit weird, but there's no way to my knowledge to exclude these cases via the xml config. I have added them throughout the library for consistency, alternatively you can just disable the ckech for that line with a // phpcs:ignore.

Also, I have checked that everything is working fine

That's great! Thank you for your contribution! 🚀

$x2 = (($x + 1) * $this->scale);
$y2 = (($y + 1) * $this->scale);

$rectsize = $this->scale / 2;
Copy link
Member

Choose a reason for hiding this comment

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

This causes an "implicit conversion from float to int" exception in imagefilledrectangle() -> cast to int or use intdiv()

// --------------------

class QRCustomPNG extends QRGdImage {

Copy link
Member

Choose a reason for hiding this comment

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

You could also intercept the constructor to enable the internal upscaling for "rounder" corners at lower scales (which would be counter-intuitive for the user to set):

	public function __construct(SettingsContainerInterface $options, QRMatrix $matrix){
		$options->drawCircularModules = true;

		parent::__construct($options, $matrix);
	}

@codemasher codemasher merged commit f88b53a into chillerlan:main Sep 18, 2023
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