update svg backend code to write all data directly into output buffer

[kristopherbullinger]

This update will restructure the SVGBackend to write all tag attribute values such as for width, dx, viewBox etc directly into the backend's string buffer, rather than creating temporary strings using the format! macro.

The existing workflow for opening a tag includes providing a tag type, a list of attrs, and whether the tag is self-closing or not. Because the attrs are provided in a list, all list items' values must have a common type of &str. As such, attrs which exist in the code in the form of integers or any other non-string type must first be written to a string before being included in the attrs list.

To avoid the cost of allocating many temporary strings, we instead use an AttrWriter. When SVGBackend::open_tag is called, you are given an AttrWriter, which includes methods for writing a key followed by a value. Typestate is used to ensure that a value is only written if there is an associated key. The AttrWriter::write_value method is generic over its parameter so that each type can decide how it is written to the buffer: string-like types utilize the existing html-escaping functionality, while number-like types are written directly into the buffer with their Display impl. When the user has finished writing all attrs to the tag, they will then call AttrWriter::close to finish a self-closing element with />, or AttrWriter::finish_without_closing to finish the opening tag with > and then push the tag onto the tag_stack so that it can be closed in the future. Thus, it preserves existing behavior.

[AaronErhardt]

This is duplicated code, you could reuse the implementation of FormatEscaped for char

[AaronErhardt]

I think opacity and color are accidentally mixed here.

[AaronErhardt]

Code looks good overall, but there are a couple of new clippy warnings, code duplication and correctness issues. Once those are addressed, I'll do another review.

[kristopherbullinger]

Good catch. Thanks. Code updated.

[AaronErhardt]

I just found one more thing.

Also, would you mind running a couple of tests to verify that the generated SVG does work as before, at least for the examples? With changes like this it's easy to overlook something and I don't want to deal with avoidable regressions.

[AaronErhardt]

This can't be correct given that 446 does it differently. Plus in line 446 there might be a whitespace missing at the end of each point.

[kristopherbullinger]

You were right about the missing spaces between line points. I've just pushed a fix for that. This time I compared the files generated by the tests byte-for-byte against those generated by 7e7d2aa463a202a8ff1b340f41feb703482e753a (from which my repo is forked) to confirm they are all equal:

cmp target/test/svg_master/test_draw_mesh_negative_ticks.svg target/test/svg/test_draw_mesh_negative_ticks.svg
cmp target/test/svg_master/test_draw_mesh_no_ticks.svg target/test/svg/test_draw_mesh_no_ticks.svg
cmp target/test/svg_master/test_draw_pixel_alphas.svg target/test/svg/test_draw_pixel_alphas.svg
cmp target/test/svg_master/test_series_labels.svg target/test/svg/test_series_labels.svg
cmp target/test/svg_master/test_text_alignments.svg target/test/svg/test_text_alignments.svg
cmp target/test/svg_master/test_text_clipping.svg target/test/svg/test_text_clipping.svg
cmp target/test/svg_master/test_text_draw.svg target/test/svg/test_text_draw.svg

returns no errors. Also, as a sanity check, I manually inspected each line side-by-side and confirmed visually that the outputs are equal.

Thanks for your careful review. Next time I will be more thorough to ensure behavior is unchanged.

[AaronErhardt]

Thanks for your contribution and for checking against the master branch!