Limited SVG support - SVGTexture

[VL-Impact]

Hi folks!

I've found nice library named Nanosvg that provides both SVG parser and rasterizer;
so I've implemented small wrapper around it, that allows to load SFML Texture from '.svg' files.

According to Nanosvg's author, his library (and therefore SVGTexture) has limited support of SVG standart, but still, pretty complex images can be processed. I.e. it renders SFML SVG logo pretty well!

So, here's the code: https://github.com/VL-Impact/SFML-SVGTexture

Any feedback is welcome.

[DarkRoku12]

Good to see the community adding features.

Only one remark:

goto error;

Using goto is considered a bad practice in programming.

You could use a function to free the buffers:

inline errorSVG( type_of_rast *rast , type_of_image *image , const char* error )
{            
        printf(error); // Or use cout or std::string.
                nsvgDeleteRasterizer(rast);
                nsvgDelete(image);
}
 

and "return false" from the if statement like:

    errorSVG( rast , image , "My error message" ) ; return false ;  
 

And one suggestion:

If your class is:

SVGTexture

you don't need to add SVG prefix on the method:

loadFromSvgFile

because is clear that if you are using the class SVGTexture obviously you will pass to it a svg file's path.

[FRex]

goto error;

I knew someone would say it's bad.
Well yes, I have to agree it's bad to do that C style goto error/fail/ret in C++, even though I'm the C leper of this forum usually.
Just use a RAII wrapper for it instead, that's MUCH better way to do things like that in C++.
0167aadb400215ac35e8453ff58e2650f5322499

inline errorSVG( type_of_rast *rast , type_of_image *image , const char* error )

inline void errorSVG( type_of_rast *rast , type_of_image *image , const char* error )

A typo there.

you don't need to add SVG prefix on the method

He inherits from sf::Texture and so he'd hide the loadFromFile from sf::Texture if he named a method the same and it'd be hard to distinguish methods to load a file and an svg from just the parameter lists and overloading is nice but it shouldn't be abused to do stuff like:

int compute(int seed);
int compute(Randomizer * rnd);
int compute(const char * methodname);

It should be:

int computeFromSeed(int seed);
int computeFromRandomizer(Randomizer * rnd);
int computeByMethod(const char * methodname);

or something like that, overloading is fine but within reason.
This is accidentally one of my problems with nullptr, promoting overloading functions with complete impunity.


As for the library itself:
Manual memory management with malloc and free is a no-no in C++, obviously...

And the inheritance from sf::Texture is also a little worrying.
There's no need for it. To use this code you'll now need to replace textures in code with SVGTextures, which are not even SVG of any kind at all, just an SVG file rasterized into a normal Texture, or even a normal image Texture since you inherit everything so the SVGTexture can always replace the real Texture in code, even if the new load method is never used and the usage has nothing to do with SVG at all.

And since one big advantage of scalable vector graphics is not being raster graphics and being scalable and so on I think I'd enjoy a way to get a triangle strip vertex array with right colors and all more than just rasterizing but I have no idea if that's possible with that nanosvg library.
I mean, If I want to just rasterize an SVG I can do it offline (possibly few times with different settings) with way more capable tool and then just load the resulting png. It'd be bigger than the xml, but still small enough for practical purposes.

[DarkRoku12]

goto error;

even though I'm the C leper of

I like more C than C++. I have my reasons. But i admit, if you are working on a mixed C/C++ project ,is better to use the C++ advantages. RAII with function/operator overloading (and virtual functions) is one of then main advantages of C++. We must use then when possible.

But even we cannot take advantage of SVG format but you made the first step of this approach.

[VL-Impact]

goto error;

Agreed, it's a bad habit to mix in C-style error handling when RAII is available; now it's fixed.

Manual memory management with malloc and free is a no-no in C++, obviously...

Yup, fixed that too.

And the inheritance from sf::Texture is also a little worrying.
There's no need for it.

Yes, good point. I've removed inheritance and made load function static. Or maybe it will be even better to make it free function, like

bool loadTextureFromSvgFile (Texture& targetTexture, const std::string & filename,
                             const std::string & units = "px", float dpi = 96.0f);

?

I think I'd enjoy a way to get a triangle strip vertex array with right colors and all more than just rasterizing but I have no idea if that's possible with that nanosvg library.

Well, I've managed to get at least lines strip vertex array using nanosvg, so I have to think about that.

I mean, If I want to just rasterize an SVG I can do it offline

Yes, I know, but topics about SVG support appear time to time at forum, so I've decided to make some start