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

SvgLogo background in release 1.4.2 #343

Closed
btmi opened this issue Nov 11, 2021 · 7 comments · Fixed by #346
Closed

SvgLogo background in release 1.4.2 #343

btmi opened this issue Nov 11, 2021 · 7 comments · Fixed by #346
Assignees
Labels

Comments

@btmi
Copy link

btmi commented Nov 11, 2021

When using the new SvgLogo prop on the pre-release build 1.4.2 for SVG output, the background area of the logo should be cleared before the logo is rendered otherwise the logo will be obscured by erroneous data.

A workaround to this is to add a white fill in the SVG logo used.

This is not a problem with bitmap logos.

@codebude codebude added the bug label Nov 11, 2021
@codebude codebude self-assigned this Nov 11, 2021
@btmi
Copy link
Author

btmi commented Nov 12, 2021

Working on this a little more, the resulting SVG has some strange characteristics. In the attached example (which had to be zipped but was created with QRCoder), you can see it renders fine in browsers, but if you import it into an app (like Inkscape, Canva, Boxy or Illustrator) the icon is lost or ignored or there are security warnings. Note: this icon was created with a white fill to avoid the problem originally highlighted in this issue.

google.zip
,

@codebude
Copy link
Owner

Hi @btmi , I could test only Boxy and Inkscape. Boxy showed the logo off canvas/misplaced. Inkscape didn't show it at all. It seems like those applications (in opposite to webbrowsers) don't like relative placement of the image tag.

QRCoder places the image with relative position:

<svg width="100%" height="100%" version="1.1"
        xmlns = "http://www.w3.org/2000/svg">
        <image x="43%" y="43%" width="15%" height="15%" href="data:image/svg+xml;base64,PD94bWwgdmVyc2lvbj0i...z4K" />
    </svg>

If I fix this manually to absolute positions, like below, it seems to work:

<svg width="100%" height="100%" version="1.1"
    xmlns="http://www.w3.org/2000/svg">
    <image x="552.767" y="640.709" width="516" height="216" xlink:href="data:image/svg+xml;base64,PD94bWwgdmVyc2lvbj0i...z4K"/>
  </svg>

So I guess we have to rewrite the logo functionality to use fixed positions instead of relative ones.

The following .zip contains a manually patched SVG. Can you try if this works in Illustrator and Canva, too?
google_test.zip

@btmi
Copy link
Author

btmi commented Nov 13, 2021

Yes - that change seems to fix most of the issues I have witnessed. Canva was perfect, and Illustrator was able to load the file (my version of Illustrator is old and it did not include the icon but I am sure that is a version issue). Also the previews were correct as well. In Inkscape I notice that the image loads as a bitmap and so is not scaled, but I read that this was the case somewhere.

So this is the best solution for the bitmaps.

For SVGs though, I have another solution that changes the height. width, x, y coordinates and adds a viewport of the main svg node to get the scaling right. See my attached example, which also includes a fix for the white space as well (the line immediately before the added svg). The values were obtained by using the same co-ordinates as the barcode - relative values did not work. By doing it this way, you also can then (optionally) change the background and fill color of SVG should so desire. Please note, that I am not an svg or qrcode expert and I have made some assumptions (like the placement of the whitespace), but all of my tests so far have worked. If you want me to do the coding to put this theory in practice, I am happy to give it a go.
google_test3.zip
o.

@codebude
Copy link
Owner

Hi Peter,

Thanks for the feedback. I already started the implementation. You can find the current progress in this branch: https://github.com/codebude/QRCoder/tree/bugfix/343-svglogo-background-and-positioning

This implementation is working with absolute positions, but it seems to work/scale fine for me. Instead of painting the area behind the logo in the light color, I calculate if a dark module shall be painted or if it would overlap with a logo. I think this is a more elegant solution, because when the user gives a logo size that would partly overlap some of the module/blocks we would end up with "half" modules at the logo's edges. That would look somehow "wrong". So my approach to clean the background behind the logo, is to just paint not data modules behind it.

I'll merge this branch on Monday. (I have to write some more tests, before I merge it, but I'm on the road this weekend...) So either wait for the CI build/GitHub package or feel free to checkout the branch and compile it yourself. (If you want to test it now.) :-)

@btmi
Copy link
Author

btmi commented Nov 13, 2021

My personal view is that rendering the logo as a SVG via a viewport is a better option than converting it to an image if the source is a SVG as it will make the resulting QRCode completely scalable (which for my mind is the goal of SVG output) and also removes all rendering issues on all platforms (the image tag is ignored by some). I do understand your approach though and when you have finished making your changes I will try this out on my fork and share with you the results when I am done. Have a great weekend.

@codebude
Copy link
Owner

Hi @btmi ,

I finalized the changes, merged them and created a CI-version. (You can find the package including all changes here: https://github.com/codebude/QRCoder/packages/1072246)
After your last feedback I also re-wrote the rendering for logos of SVG-type. They will be rendered as "native" SVG instead of -tag.

Feel free to re-open this (or a new) issue, in case you find any problems with the current state of the implementation.

@btmi
Copy link
Author

btmi commented Nov 15, 2021

Looks good.

I took a slightly different approach by adding a bool to the SvgLogo definition to set whether or not it is embedded or rendered. Can I also suggest you add the Attribute shape-rendering="geometricPrecision" to the sub svg node to counteract the "crispEdges" in the main svg.

My version is here https://github.com/btmi/QRCoder/blob/78b9680e9bdc37330d84c014ff720191c71db6e5/QRCoder/SvgQRCode.cs, but I will play with yours when it is released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants