Wiladams / svgandme

SVG Parser and rendering using blend2d for graphics
MIT License
6 stars 4 forks source link

Unexpected changes #14

Open abu-irrational opened 3 months ago

abu-irrational commented 3 months ago

Hi Wiliam,

I noticed you recently made two changes that broke compatibility with my integration.

The first one: seconds().

This function is not defined among all those in svg/.h (it's defined in app/). Not bad, I can always put

define seconds() 0

since I'm not interested in timing..

The second change is about the SVGDocument constructor.

It was before SVGDocument(FontHandler fh) now it is SVGDocument(FontHandler fh, const double w, const double h, const double ppi = 96)

Honestly, I don't understand why it's required to specify the size of the canvas when loading/analyzing the SVG file. From my point of view, the sizing of the canvas concerns the rendering phase, i.e. when I initialize an SVGDrawingContext by passing it an appropriately sized BLImage. What's more, only after loading an SVG file, I can know the image's dimensions (or rather the height/width ratios) to size the canvas (or set up a scale transformation).

Maybe I didn't understand correctly, but I think that canvaHeight,width should not be part of SVFDocument.

Wiladams commented 3 months ago

seconds() made a brief appearance for debugging. I'll turn that back off.

I've gone back and forth on the SVGDocument including or not including the canvas size. You're right in that the document does not need this information until it is time to render.

If you look at the SVGDocument class, you'll see that it inherits from "IAmGroot". This facilitates being able to render, because it contains environment information, like the Window/canvas width and height, and dpi.

This information is only needed when you actually render, but it's needed for two things. 1) Size of the canvas. We need this so we can resolve cases where "%" is used for a size, instead of absolute units. We also need the 'dpi' for the same reason. When units, such as 'in', 'mm', 'cm' are used, we need to know the dpi so we can resolve to actual number of pixels. 2) Looking up objects. There are many places that might be holding a reference to an object, such as the 'use' element, or gradients referring to a template gradient, or colors, referring to a gradient, etc. at 'bind' time, we need to know where we can lookup all those values. They are tied to the "IAmGroot" interface.

At the moment, 'bindToGroot()' is called within the 'addNode()' function of the document. Therefore, the SVGDocument already needs to know this information.

Well, it doesn't have to be like this at all. As long as when the document is being constructed, all those nodes are held in some form where they can be looked up later, this binding can happen at any time, such as when you're drawing into a context.

In that case, it make more sense to change the SVGDocument::draw() function to be more like

void draw(IRenderSVG * ctx, IAmGroot *groot) override

That way, you don't need to bind early, and you won't need to pass the width and height or dpi earlier.

The same can go for document construction. There's no reason SVGDocument needs to inherit from IAMGroot. As long as a pointer to such an object is included in the "loadFromXmlIterator" call, we can completely remove this code to a separate object, and the SVGDocument itself becomes even simpler.

Those are my thoughts on it.