Black background when converting SVG to PNG about oksvg

[srwiley]

Yes, I see no reason why that should be mandatory if it is not in the specifications.

…

On Fri, Aug 12, 2022 at 1:11 AM Hannes Leutloff ***@***.***> wrote: One more update from me (sorry for the multipost): After looking at the tests I realize that the style tag *is* supported, but only within an enclosing defs tag. That is understandable, but is probably not how most SVGs look and as far as I know there is nothing in the SVG spec that says that style tags need to be enclosed in defs tags. @srwiley <https://github.com/srwiley> would you be fine with changing this requirement? — Reply to this email directly, view it on GitHub <#34 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AC4VOB3QIOHGW6LNNM7MKT3VYYBMBANCNFSM5IL4JHIQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[yeldiRium]

That is great to hear. Would you mind taking a look at my PR then?
I'm not sure how clean my implementation is, since I basically circumvent the inDefs check by setting inDefsStyle without it. But I added a test and top level styles work the way they are supposed to.

[mysugar]

Hi ,have you solved this problem? @giancarlobastos

[yeldiRium]

Some input on this: I believe this is due to oksvg not supporting style tags in SVG. I can't say for sure, but I'm having the same problem and removing the style block and putting the style attributes directly into the path tags solved the rendering issue for me.

Edit: I've looked at @giancarlobastos SVG and it also uses a style block to assign fill colors to the paths. I think we are experiencing the same issue.

[yeldiRium]

I've perused the code and I've found something that confuses me:

style tags should be handled here

oksvg/public.go

Line 61 in a61f04f

if se.Name.Local == "style" && cursor.inDefs {

and the code to do so looks fine to me. Once the endElement for the style tag is found, its text content is parsed and added to the styles here

oksvg/public.go

Line 89 in a61f04f

icon.classes, err = parseClasses(classInfo)

But before the style tag can be handled, the readStartElement call here

oksvg/public.go

Line 57 in a61f04f

err = cursor.readStartElement(se)

will always fail, since style tags are not handled there and not a valid draw function, so this line will fail

oksvg/icon_cursor.go

Line 345 in a61f04f

df, ok := drawFuncs[se.Name.Local]

I'm not that far into the code, so I'm not sure enough on how to fix this to provide a PR. However, my intuition is to handle style tags before and without calling readStartElement on them. @srwiley what do you think about this? Can you do anything with this research?

[yeldiRium]

One more update from me (sorry for the multipost):

After looking at the tests I realize that the style tag is supported, but only within an enclosing defs tag. That is understandable, but is probably not how most SVGs look and as far as I know there is nothing in the SVG spec that says that style tags need to be enclosed in defs tags. @srwiley would you be fine with changing this requirement?

[goloroden]

@srwiley To me the solution that @yeldiRium came up with looks pretty good. Is there a chance that the according PR gets merged soon?

[srwiley]

Yes, is everyone in agreement? Have the tests been run and are there tests supporting the new code?

…

On Thu, Sep 1, 2022 at 7:52 AM Golo Roden ***@***.***> wrote: @srwiley <https://github.com/srwiley> To me the solution that @yeldiRium <https://github.com/yeldiRium> came up with looks pretty good. Is there a chance that the according PR gets merged soon? — Reply to this email directly, view it on GitHub <#34 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AC4VOBYIY6TER3LWRNGYOCTV4C7MNANCNFSM5IL4JHIQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[goloroden]

Yes, there are tests to support the code (and given the discussion above it seems as if @yeldiRium did a pretty decent job 😊).