Support passing PNG data directly to SvgLogo by Shane32 · Pull Request #491

Support passing PNG data directly to SvgLogo by Shane32 · Pull Request #491 · Shane32/QRCoder

Summary

This PR fixes/implements the following bugs and features:

Fixes a bug where extra null bytes are included with the logo embedded within a SVG
Adds a constructor allowing for a custom PNG file data to be supplied to the logo embedded within a SVG

What existing problem does the pull request solve?

When using a third-party library to encode image data, or when reading PNG directly from a stream, this additional constructor allows for it to be passed directly to the created SVG file, without passing through the System.Drawing library.

Test plan

Tests have been added to the codebase

		{
		bitmap.Save(ms, System.Drawing.Imaging.ImageFormat.Png);
		_logoData = Convert.ToBase64String(ms.GetBuffer(), Base64FormattingOptions.None);
		_logoData = Convert.ToBase64String(ms.GetBuffer(), 0, (int)ms.Length, Base64FormattingOptions.None);

Choose a reason for hiding this comment

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

GetBuffer() returns the current working memory buffer allocated by MemoryStream, including extra padding that is reserved but not yet used by the MemoryStream. This is unlike ToArray which copies the working data to a new array of the correct length. To properly use GetBuffer, the length must be passed to Convert.ToBase64String so that only the working data is encoded.


		var result = HelperFunctions.StringToHash(svg);
		result.ShouldBe("78e02e8ba415f15817d5ed88c4afca31");
		result.ShouldBe("7d53f25af04e52b20550deb2e3589e96");

Choose a reason for hiding this comment

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

Since the extra padding was removed from the SVG, the hash has changed.

		var svg = new SvgQRCode(data).GetGraphic(10, Color.DarkGray, Color.White, logo: logoObj);

		var result = HelperFunctions.StringToHash(svg);
		result.ShouldBe("7d53f25af04e52b20550deb2e3589e96");

Choose a reason for hiding this comment

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

We can see here that the new constructor results in the same hash as using the System.Drawing.Bitmap constructor.

Choose a reason for hiding this comment

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

Hmm, this is false. Well, I was sorta surprised; .NET should re-encode the PNG with the Bitmap constructor. Not sure why I thought otherwise.

Shane32 changed the title ~~Support passing PNG logos directly to SvgLogo~~ Support passing PNG data directly to SvgLogo

Apr 2, 2024

Can you please re-check the test-cases? It seem like the changes are breaking one of the test cases @Shane32

@codebude Done. FYI, I did save and open the outputs of the two affected tests to be sure they still look correct.

@Shane32 thanks for your support. I really appreciate it!

My plan is as follows:

Merge your pull requests
Update the wiki/documentation (to include the extensions/changes from your PRs)
Check other open PRs (especially the ones you listed)
Merge the PRs that can be merged without conflict (merging all PRs into a new one takes away the chance to be listed as a contributor in the project from the people who submitted the PRs. That's why I want to see what can be merged before we consolidate).
List non-mergeable (but interesting) requests in an issue and create a consolidated PR for them
Release a new QRC or version (1.4.4?, 1.5.0? - depending on how many changes are added).

Rough time plan: 4-7 days

Does that work for you? If you want to use the QRCoder with your changes in advance, you could also use the current package from the Github packages: https://github.com/codebude/QRCoder/pkgs/nuget/QRCoder

Shane32 deleted the allow_custom_png_image_in_svg branch

April 3, 2024 12:27

Certainly that works for me and I appreciate your attention. Thanks again, and let me know if I can be of assistance.

Just so you know, as maintainer you can push additional commits to any PR where the contributor checked the “allow edits from maintainers” checkbox, which most people do. It’s great for preserving authorship and due credit towards those contributors, but it’s a bit of pain to actually do. You need to configure another source within Git for the repo, which can be done from within VS, and then you can merge across repos, fix tests, etc. (But me as a contributor the most I could do is to create a new PR.)

@codebude I reviewed a number of the outstanding PRs and left comments. Some can be merged as-is; some need tests updated. Let me know if I can be of assistance. Thanks!

@codebude I also went through all of the outstanding issues and added a couple PRs:

Adding support for Base64QRCode within Linux is a much-requested feature, and as the default implementation creates a PNG, PR #495 leverages the existing PngByteQRCode class to base-64 encode a PNG file without using System.Drawing.Common. Attempting to create a JPG will throw a PlatformNotSupportedException unless on Windows.

Can I ask what your plans are for the remaining bugfix PRs and/or publishing a new release to NuGet?

Thanks again, and let me know if I can be of further assistance.

Hi @Shane32 ,

my plan is still to go through the open PRs and merge as many of them as possible. (Ideally all that can be merged with manageable effort. I would like to reserve larger PR for a later release).

Unfortunately, I've already missed the 4-7 days to the next release that I announced in the comments above, but I hope you can see that I'm making (slow but) steady progress. Unfortunately, things are not going as quickly as I would like at the moment. (My little daughter and my main job are currently taking up a lot of my time and they both have priority over my leisure projects). Nevertheless, I hope to be able to release version 1.5.0 soon.

I would still like to look at PRs #495, #411, #474 as well as #430 /#438 and (if possible) merge them. After that, I plan to push 1.5.0 to Nuget before continuing with the other issues and PRs.

Sounds great and thanks for the update!

This was referenced

Dec 5, 2025