Enhancement: Make `*Document` classes inheritance-friendly

[a-ludi]

I am trying to add some more functionality and fix the reported issues in my own code base so I don't depend on your immediate implementation. However, I am facing the issue that most of the methods and properties in the *Document classes are marked as private. This effectively forces me to copy&paste your code to make my changes.

It would be very easy to extend your code if the methods were protected instead. For example I want to add the setLineDash and lineDashOffset methods/properties to the renderers.

[p0nce]

What methods in particular do you need?

[a-ludi]

I implemented the example in a fork (see pull request #32) so it is probably useless to go into detail about this. In general, I think every method and property that is not part of the public API should be protected unless there is a good reason to make it private.

For example, I would tend to make SVGDocument._bytes a private field because it should only be manipulated by calls to output and friends. This would require an additional method protected const(ubyte)[] getDocumentBytes() that retrieves the field value so one can manipulate the implementation of bytes().

[p0nce]

My opinion after re-reading.

HTMLRenderer does inherits from SVGRenderer, but in reality it's unclean and should use delegation to a SVGRenderer member instead of inheritance (that would cause larger code, but ultimately cleaner). All these renderers would be final classes like PDFRenderer. They are closed because their only purpose is to implement the canvas interface, so there should be no reason left to inherit from them.

If there is a documented need for protected, then the API boundary in irenderer.d needs to be changed with the new requirement. I see no such need right now. But no we shouldn't change all methods to protected out of just principle. On the contrary I think everything should be private unless there is a good reason to make it public. (It is indeed a bit difficult because there is no interface that would be private to printed for sharing more code across renderers, the only interface is aimed at library users)

bytes is indeed a bad name that should be changed to getDocumentBytes()

[a-ludi]

HTMLRenderer does inherits from SVGRenderer, but in reality it's unclean and should use delegation to a SVGRenderer member instead of inheritance (that would cause larger code, but ultimately cleaner). Agreed. That makes it also possible to exchange the SVG implementation without touching HTMLDocument. The code base size comes second, in my opinion.

All these renderers would be final classes like PDFRenderer.

Why do you want them to be final classes?

[...] so there should be no reason left to inherit from them.

I think it can be beneficial if the user has the possibility to adjust details. For instance, a user may be interested in generating only SVG files with clickable elements or other SVG-specific features. This is clearly out of scope for this library but could be easily added if the renderer methods were protected while having all the benefits of an actively maintained library.

I feel this is somewhat important because there is no other way to hook into the rendering process but it is a perfectly valid wish to exploit more features than IRenderingContext2D exposes.

[p0nce]

Why do you want them to be final classes?

Because I'd prefer if every purpose can be reached with the available IRenderingContext2D, and that way it's easier to keep track that targets really feature parity (currently, they haven't with respect to save/restore).

For instance, a user may be interested in generating only SVG files with clickable elements or other SVG-specific features.

The current API doesn't allow to add links, but this is a problem of IRenderingContext2D (it lacks features) rather than a problem of visibility. If somehow you want to hack into classes, you can change them in the dub directory?

If a method is open for extension, then the library maintainer is making additional promise that invariants will be maintained. It's the best way to have code silently break in the future, as the base code change without knowing what derive from it.

[a-ludi]

[...] but this is a problem of IRenderingContext2D (it lacks features)

Okay, this settles the development path then. New features are to be defined in IRenderingContext2D and implemented in every subclass before they make it into a release. That is entirely legit.

If somehow you want to hack into classes, you can change them in the dub directory?

I would discourage this very strongly because that directory is maintained by DUB and not subject to version control. Instead, I would recommend (as I am doing) to git submodule add a fork of the repo to the project in question and maintain the code changes there. This allows the user not only to track changes but also to share them with the main repo via pull request. Also one can have shared access across one's own projects if that is desirable.

[p0nce]

For reference, is there something you want out of printed that isn't currently available? (apart from easier text layout :| )

[a-ludi]

After adding the dashed lines I am mostly happy for now. I am using it for plotting very specific diagrams so I am not really interested in text layout right now.

There are currently two minor things on my mind:

1. For my use case it is inconvenient to have the fonts embedded in every file (~8000 files). I have a local extension to SVGDocument that avoids embedding if requested by the user (bytes(bool embedFonts = true)). This could certainly be achieved for PDF, too. I did not propose this to you as it is in contrast to the aim of having standalone documents.
2. Again for SVG it can be beneficial to set the background color, e.g. when examining the generated file in an editor. But this is not possible for PDF as I have learned. So it should not really be part of this library. It is one of the little things where inheritance could be a remedy. :wink:

I will let you certainly know if there is something worth including. I really appreciate your effort with this library!

[p0nce]

Very cool, glad that you use it! I'm using it for invoice and quotes.

I can see the idea of not embedding fonts for both SVG and PDF. In SVG you'd have access to all system fonts. In PDF this drastically reduce the choice to only 5 fonts, and I'm not sure how portable it still is. But it can definately produce much smaller files too, so I'd say it's valuable, only complicated.

In a former contract job I was tasked to generate SVG and lack of font reproducibility was a problem, it would look different in every SVG reader. Hence this library.

[a-ludi]

In a former contract job I was tasked to generate SVG and lack of font reproducibility was a problem, it would look different in every SVG reader. Hence this library.

I fully support this approach. Font handling in varying environments is complicated and it is nice to have a file that is self-contained. You may even have this problem if you are looking at your own files years after they were generated. In my particular case it is nice to reduce files size as well as build time (not compile time).

In PDF this drastically reduce the choice to only 5 fonts, and I'm not sure how portable it still is.

Can you explain how the font system in PDF works? Sounds like there is a core set of fonts that every PDF viewer must ship and all other glyphs must be embedded into the file, correct?

[p0nce]

Yes, this is correct ; you have a few fonts. So very handy to generate small files. https://kbpdfstudio.qoppa.com/standard-14-pdf-fonts/

On the other hand, I'm not sure how the support is with printers, and I think the default fonts are deprecated in newer PDF version of the spec. The main reason I've been avoiding it is that you would need to extract glyph data anyway or have default tables of glyph advance. printed would need to provides these fonts data.

[a-ludi]

I think this is a feature that should be implemented only by specific (SVG/HTML so far) renderers that have a natural binding to system fonts. Actually, no vector graphics format comes to my mind that has this feature apart from the web technologies.

[p0nce]

Closing this issue.