Skip to content

Update / rewrite of the ofxSvg addon to remove dependency on libTinySvg and libxml2.#8266

Merged
ofTheo merged 23 commits into
openframeworks:masterfrom
NickHardeman:rewrite-ofxSvg
Sep 8, 2025
Merged

Update / rewrite of the ofxSvg addon to remove dependency on libTinySvg and libxml2.#8266
ofTheo merged 23 commits into
openframeworks:masterfrom
NickHardeman:rewrite-ofxSvg

Conversation

@NickHardeman

@NickHardeman NickHardeman commented Feb 15, 2025

Copy link
Copy Markdown
Contributor

Proposing this update to ofxSvg with the following implementation based on my ofx::svg::Parser addon (https://github.com/NickHardeman/ofxSvgParser).
This rewrite adds the following functionality:

  • Loading of elements with search functionality

  • Remove libtinysvg and libxml2 dependencies. The parsing is happening in the ofxSvg class ( it took a while getting through the spec on paths :0 and parsing/writing CSS manually ;) )

  • Add elements to the document

  • Save the document after loading or adding / removing elements.

  • Works with the current ofxSvg example.

Grab a group by passing in the name of the group.
auto logoGroup = svg.get("logo");

Grab nested groups by passing in the hierarchy to the group separated by colons.
auto cloudGroup = svg.get("sky:clouds");

Get all of the elements of a certain type by calling getElementsForType
auto sprinklePaths = svg.getElementsForType("Donut:Sprinkles");

Currently supports the following types:
Group, Rectangle, Image (linked and embedded), Ellipse, Circle, Path, Polygon and Line

Limited Parsing Support:
Text

We have been using some version of this for parsing svg in our projects for the last several years.

TODO:

  • Update scripts to remove libsvgtiny and libxml2
  • Add example for creating a svg document, adding items to it and saving.
  • Implement same constructors as previous ofxSvg ( copy and with default file path string ).
  • Complete in-line documentation.
  • Fix failed builds on certain platforms.
  • Discuss use of ofx::svg for subsequent classes, ofx::svg::Group vs. ofxSvgGroup. (ofxAssimp - uniformity on addon name, include name #8354)
  • Add image embedding saving
  • Image embed parsing.
  • Basic text parsing
  • Set text on text span with wrapping
  • Add text to document for saving
  • Use custom optional instead of std::optional to legacy and platform compatibility (fixes for windows oF 0.12.0 NickHardeman/ofxSvgParser#2)

@NickHardeman NickHardeman self-assigned this Feb 15, 2025
@NickHardeman

Copy link
Copy Markdown
Contributor Author

This issue on windows might make the argument for ofxSvgEllipse instead of ofx::svg::Ellipse

../../../addons/ofxSvg/src/ofxSvg.cpp:1988:17: error: reference to 'Ellipse' is ambiguous
   1988 | std::shared_ptr<Ellipse> ofxSvg::addEllipse( const glm::vec3& apos, float aradiusX, float aradiusY ) {
        |                 ^
  D:/a/_temp/msys64/clang64/include/wingdi.h:3009:28: note: candidate found by name lookup is 'Ellipse'
   3009 |   WINGDIAPI WINBOOL WINAPI Ellipse(HDC hdc,int left,int top,int right,int bottom);

@dimitre

dimitre commented Feb 15, 2025

Copy link
Copy Markdown
Member

Fantastic @NickHardeman let me know when it is ready to test and I can test with a lot of SVGs from previous projects here

@NickHardeman

Copy link
Copy Markdown
Contributor Author

Fantastic @NickHardeman let me know when it is ready to test and I can test with a lot of SVGs from previous projects here

Great, will do. I can also send a test app that loads an svg from a folder, saves it and reloads the saved one. Arrow keys to change index and load again in the directory.

@NickHardeman

Copy link
Copy Markdown
Contributor Author

@dimitre I think it's ready for some testing now that it's passed the checks. Attached is a quick app for testing loading, saving and loading the saved svgs. The svgs in the folder are from https://www.svgviewer.dev and are just for testing complex paths.
SvgTester.zip

@dimitre

dimitre commented Feb 21, 2025

Copy link
Copy Markdown
Member

Great I've just seen now and tested some things. I'm testing with a very crude set of svgs and will report some things.
sharing the files here if you want to check
svgs.zip

@dimitre

dimitre commented Feb 21, 2025

Copy link
Copy Markdown
Member

Most of the things work great for reading and rewriting.
I've noticed in art made with lines only, contours are thicker than the macos quickLook preview
and open outlines appear as closed.
Novapadrao6
Anitta2

@NickHardeman

Copy link
Copy Markdown
Contributor Author

@dimitre thank you for trying it out. I have fixed some of the fill issues that you mentioned.

@NickHardeman

Copy link
Copy Markdown
Contributor Author

I think the line width is due to the ofScale I am applying in the example to get the entire svg to fit within the window. When I remove the scaling, here is a comparison between OF and osx quicklook preview. The preview is on top and the OF app is in the bg.

image

@NickHardeman

Copy link
Copy Markdown
Contributor Author

Lines are scaled with attenuation by the very hidden function

auto rend = std::dynamic_pointer_cast<ofGLProgrammableRenderer>( ofGetCurrentRenderer() );
rend->enableLineSizeAttenuation();

Screenshot 2025-02-21 at 10 27 53 PM

Default screen space lines:
Screenshot 2025-02-21 at 10 28 15 PM

@dimitre

dimitre commented Jul 28, 2025

Copy link
Copy Markdown
Member

Hey Nick finally I've found this one again. I'm looking forward to see it merged :)
I'll cherry pick to the branch I'm using now.

@dimitre

dimitre commented Aug 8, 2025

Copy link
Copy Markdown
Member

it would be great to have this one merged. if you prefer it can have an alternative name, like ofxAssimpModelLoader / ofxAssimp

@NickHardeman

Copy link
Copy Markdown
Contributor Author

@dimitre I agree, been doing a lot of work on the addon, implementing some text parsing and cleaning it up a bit. Text parsing is exhausting!
Was hoping to just keep ofxSvg and it would replace the existing addon to avoid the continued maintenance of libsvgtiny and a new addon. Though the path parsing should function the same, if it doesn't there wouldn't be access to the old addon it it was replaced...
I implemented the previous functions so it should be compatible with the previous addon. As with all new addons it will need some more thorough testing. Thank you for the tests you have already provided, I have been using them while developing :)

@dimitre

dimitre commented Aug 14, 2025

Copy link
Copy Markdown
Member

Awesome!

@dimitre

dimitre commented Aug 15, 2025

Copy link
Copy Markdown
Member

I'm now testing and found some issues with this files here
Archive.zip

@NickHardeman

Copy link
Copy Markdown
Contributor Author

Due to the nature of ofPath, overlapping sub paths result in holes.

Screenshot 2025-08-22 at 5 10 56 PM Screenshot 2025-08-22 at 5 11 07 PM

@NickHardeman NickHardeman marked this pull request as ready for review September 4, 2025 14:11
@NickHardeman NickHardeman requested a review from dimitre September 4, 2025 14:11
@NickHardeman NickHardeman requested a review from ofTheo September 4, 2025 17:06
@ofTheo

ofTheo commented Sep 4, 2025

Copy link
Copy Markdown
Member

Due to the nature of ofPath, overlapping sub paths result in holes.

Screenshot 2025-08-22 at 5 10 56 PM Screenshot 2025-08-22 at 5 11 07 PM

Can this be fixed with one of the winding orders?

@ofTheo ofTheo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'll let @dimitre do a final test - but this all looks good to me!
Nice to have ofNode support too for local / global transformations

@NickHardeman

NickHardeman commented Sep 4, 2025

Copy link
Copy Markdown
Contributor Author

Due to the nature of ofPath, overlapping sub paths result in holes.
Screenshot 2025-08-22 at 5 10 56 PM Screenshot 2025-08-22 at 5 11 07 PM

Can this be fixed with one of the winding orders?

Not sure, but the tricky bit is that the 'O' has an internal subpath and renders correctly. So determining when it should apply different winding order would need to be implemented. Maybe check for an intersection and handle it differently?

@NickHardeman

Copy link
Copy Markdown
Contributor Author

Another item to consider is the removal of the svg tiny libs. Not sure about the best approach...

@NickHardeman

NickHardeman commented Sep 6, 2025

Copy link
Copy Markdown
Contributor Author
Screenshot 2025-09-06 at 2 41 52 PM

@ofTheo was right; I just tried changing the winding order and it looks correct with OF_POLY_WINDING_POSITIVE. However, calling getOutlines() no longer returns a vector of ofPolylines since there is no stroke width on the path.
And it breaks other types of paths. So we can maybe just add it an example of how to call
spath->getPath().setPolyWindingMode(OF_POLY_WINDING_POSITIVE);

void ofPath::tessellate(){
    generatePolylinesFromCommands();
    if(!bNeedsTessellation || polylines.empty() || std::all_of(polylines.begin(), polylines.end(), [](const ofPolyline & p) {return p.getVertices().empty();})) return;
    if(bFill){
        tessellator.tessellateToMesh( polylines, windingMode, cachedTessellation);
    }
    if(hasOutline() && windingMode!=OF_POLY_WINDING_ODD){
        tessellator.tessellateToPolylines( polylines, windingMode, tessellatedContour);
    }
    bNeedsTessellation = false;
}


//----------------------------------------------------------
const vector<ofPolyline> & ofPath::getOutline() const{
    if(windingMode!=OF_POLY_WINDING_ODD){
        const_cast<ofPath*>(this)->tessellate();
        return tessellatedContour;
    }else{
        const_cast<ofPath*>(this)->generatePolylinesFromCommands();
        return polylines;
    }
}

Wondering if we could duplicate this functionality
We may want to add another boolean to track if the outlines need to be tessellated.
tessellator.tessellateToPolylines( polylines, windingMode, tessellatedContour);
into

const vector<ofPolyline> & ofPath::getOutline() const{
    if(windingMode!=OF_POLY_WINDING_ODD){
        ////const_cast<ofPath*>(this)->tessellate(); <--- remove this 
        // add below 
        generatePolylinesFromCommands();
        if(!bNeedsContourTessellation || polylines.empty() || std::all_of(polylines.begin(), polylines.end(), [](const ofPolyline & p) {return p.getVertices().empty();})) return;
        tessellator.tessellateToPolylines( polylines, windingMode, tessellatedContour); 
        bNeedsContourTessellation = false;
        return tessellatedContour;
    }else{
        const_cast<ofPath*>(this)->generatePolylinesFromCommands();
        return polylines;
    }
}

…f OF_POLY_WINDING_NONZERO with a function to set it differently.
@NickHardeman

Copy link
Copy Markdown
Contributor Author

The ofPath::getOutline() seems like a separate issue and I think can be addressed outside of this thread.
I added functionality to use a default winding mode if all of the sub paths of an ofPath are closed. The default is OF_POLY_WINDING_NONZERO and can be set with
ofxSvg::setDefaultClosedPathWindingMode(ofPolyWindingMode)
It does not apply if a sub path is open because it seems to force close the polylines when rendering.

@ofTheo

ofTheo commented Sep 7, 2025

Copy link
Copy Markdown
Member

Cool - I am good to merge this!
@dimitre ?

@dimitre

dimitre commented Sep 7, 2025

Copy link
Copy Markdown
Member

Super! I'll be able to test next week and report back if anything.
but feel free to merge and fix later if needed.

@ofTheo ofTheo merged commit 2cc4b71 into openframeworks:master Sep 8, 2025
17 checks passed
@dimitre

dimitre commented Sep 8, 2025

Copy link
Copy Markdown
Member

I'm now testing ofxSvg. I had some issues with this SVGs and I'll be testing more soon
Archive.zip

@NickHardeman

Copy link
Copy Markdown
Contributor Author

hmm, I just tested the Archive.zip and they seem to render as they should. Can you talk more about the issues you are seeing?
Screenshot 2025-09-08 at 12 53 00 PM

@dimitre

dimitre commented Sep 8, 2025

Copy link
Copy Markdown
Member

heyyy sorry ignore the last one. they are all good. I merged changes somewhere else and tested in another OF copy.
I'll be testing lots of different SVGs soon and report back

@dimitre

dimitre commented Sep 8, 2025

Copy link
Copy Markdown
Member

I have issue with this one (hanging the software) here
pa2.svg.zip

@NickHardeman

NickHardeman commented Sep 8, 2025

Copy link
Copy Markdown
Contributor Author
Screenshot 2025-09-08 at 3 53 09 PM

I wasn't able to reproduce the issue. Maybe a build clean? I have been using Xcode to test.

@dimitre

dimitre commented Sep 11, 2025

Copy link
Copy Markdown
Member

Ok more info now. it works OK if I comment out
settings.setGLVersion(3,2);

It loads OK but hangs trying to draw element number 114 in this SVG.
using settings.setGLVersion(3,2);

I'll update with more info later

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 this pull request may close these issues.

3 participants