bodoni / svg

Composer and parser for SVG
Other
302 stars 44 forks source link

Better API support. Added public getters for elements and default hashability for nodes. #25

Closed coastalwhite closed 4 years ago

coastalwhite commented 4 years ago

Added getters for Element struct and All element Structs. Furthermore added the ability to Default Hash elements in order to have an easy way of detecting duplicates, which is usable from the API.

IvanUkhov commented 4 years ago

Thank you for the pull request. In the context of this crate, could you please explain

  1. the utility of the getters and
  2. the utility of the default hasher?

Regarding the latter, do you have an example of using DefaultHasher from some other crate?

coastalwhite commented 4 years ago

Yeah ofcourse!

Firstly, the getters provide great flexibility in the use-cases for other crates (where the svg crate would be a dependency). One use-case I can think of and have experience with is WASM, where you would like to add to the DOM directly in the form of web_sys::Element 's instead of text based html because of XSS security concerns. This is also why I myself suggested this change; I am working on such a crate. You can find that one here: WASM SVG Graphics

But these getters mostly provide a huge amount of flexibility for other crate developers, and come only with the slight downside of a bit more clutter in the docs (If a big problem this could even be hidden). Furthermore, since a borrow occurs, no weird artifacts/invalid states should be able to be created.

Secondly, for the DefaultHash, within SVG it is very memory and computationally effective to use definitions. To dynamically detect if we already have a certain definition, we can use the DefaultHash to generate a 'unique' string to represent a svg::node::element::Element. This could with huge effectiveness and efficiency detect these duplicates. And instead of either creating a new definition or adding it in plain, create a use of the old definition. In this way we don't have to keep a HashMap of definitions.

Maybe this is a more niche way of using SVG's, which only applies in a more interactive and big use-case. But without a reasonably sized wrapper around your crate it is close to impossible to represent these changes. Without the getters, this wrapper would be even more impossible. For an idea behind such a implementation you can again look at WASM SVG Graphics crate.

Ofcourse, both of my adjustments are for a more modular and dependency based use-case, and I don't know if this was your idea behind your crate.

Lastly, there is a huge chance my implementation is maybe not how you would implement this or you would like to implement this differently. By all means go for it!

Thank you!

Edit: For the api calls to be useful a get_inner would also need to be added to the seperate node elements.

IvanUkhov commented 4 years ago

Sorry. I have not had time to think about it yet.

IvanUkhov commented 4 years ago

@coastalwhite, is it still relevant to you?

coastalwhite commented 4 years ago

Yeah, I think this would make a overall good change.