cppfw / svgren

:camera: SVG rendering library in C++
MIT License
206 stars 41 forks source link

broken parsing on android #32

Closed Papirosnik closed 7 years ago

Papirosnik commented 7 years ago

after upgrading to 0.4.38 my android app stopped working. Small investigation shows that libsvgren cannot read svg files generated on windows platforms (with 0d 0a line breaks) an example: z_tiger1.zip I see migration from pugixml to mikroxml. What's the reason? Pugixml works correctly with any line styles. Moreover, simple time measurements show that pugixml is a bit faster on my testing android devices. It would be nice to provide us ability to switch to another xml parser.

igagis commented 7 years ago

Thanks for reporting the problem. Yes, I switched to another xml lib. I thought that using streaming XML lib instead of DOM xml lib will make loading faster. Unfortunately I did not see tmuch difference in loading time, althogh on desktop PC I observed slight increase in loading times, I haven't tested it on android devices. But, on the other hand, using streaming xml library gives huge decrease in memory footprint, so I decided to switch to it, despite there was no improvement in loading time. Other benefit is that I will have more control on packaging the library to different platforms where I deploy my libs. I will make a fix for the problem you reported.

Papirosnik commented 7 years ago

streamed reading and smaller memory footprint are great things, however I see that pugixml is still used in your msvc solution

igagis commented 7 years ago

it shouldn't be there, if it is then it's a bug

Papirosnik commented 7 years ago

just look into svgren/msvs_solution/libsvgren/packages.config

igagis commented 7 years ago

Fix ready. Please update mikroxml to version 0.1.3. It should be ready soon: https://travis-ci.org/igagis/mikroxml/builds/269651049 https://ci.appveyor.com/project/igagis/mikroxml/build/master-33

Papirosnik commented 7 years ago

thanks for the fix attempt. I'll check it on android a bit later. However now I see it doesn't work on windows (with native 0d 0a line breaks). I'm getting malformed document exception when trying to launch your render test Tiger from the mac (sole 0x0a line break ) works well. Switching to milroxml v0.1.3 doesn't help. And BTW, you are still targeting mikroxml 0.1.2 in your msvs solution. give us an opportunity to use pugixml as option, please. At least until mikroxml become stable. Here are those problematic tigers tigers.zip

regards!

igagis commented 7 years ago

It works for me on windows after updating to mikroxml 0.1.3. Make sure that you have updated to 0.1.3 on both, libsvgren and render_test projects in MSVS. I'll think about providing an interface to use side xml parsers in svgdom.

Papirosnik commented 7 years ago

Yeah, it works after manually switching mikroxml to 0.1.3 in both test and lib. And interface for reader would be great feature (I'm planning use binary serialized svg on mobile platforms)

igagis commented 7 years ago

Ok, closing this bug then. What do you mean under "binary serialized"?

Papirosnik commented 7 years ago

something like these: https://en.wikipedia.org/wiki/Binary_XML probably https://msdn.microsoft.com/en-us/library/cc219210.aspx

Papirosnik commented 7 years ago

Unfortunately I did not see tmuch difference in loading time, althogh on desktop

Hi Ivan! just a reminder about interface for xml side... I've just tested that nice back.svg on PC with both pugi- and mikro-xml libs. Loading with pugixml: 4.2 sec Loading with mikroxml: 4.6 sec pugi wins ) 0.4s is about 10%...

igagis commented 7 years ago

@Papirosnik could you submit a new issue about it to svgdom?