Aloxaf / silicon

Create beautiful image of your source code.
MIT License
3.12k stars 82 forks source link

SVG support #6

Open Aloxaf opened 5 years ago

Aloxaf commented 5 years ago

Render the source code into a SVG image will be cool and useful

Spaceface16518 commented 3 years ago

I'd be interested in working on this issue. Is there any current progress on it I should be aware of, as well as any opinions you have on the API design?

I was thinking about abstracting the image formatting using a trait OutputFormat which would be implemented by several structs, namely DynamicImageFormat and SVGFormat. These structs would implement OutputFormat::format, which would take the necessary parameters and return Self::Output (an associated constant of OutputFormat).

Then, ImageFormatter could contain an output_format field of type O, constrained by O: OutputFormat. The user would then pass in a valid format struct (they would be zero-sized) and be able to use the ImageFormatter::format function which would delegate to the OutputFormat::format function.

pub trait OutputFormat {
    type Output;

    /// parameters elided
    fn format(&self) -> Self::Output;
}

pub struct DynamicImageFormat;

impl OutputFormat for DynamicImageFormat {
    type Output = image::DynamicImage;

    fn format(&self) -> Self::Output {
        todo!("extract current implementation for ImageFormatter::format")
    }
}

pub struct SVGFormat;

// implementation uses the svg library for example
impl OutputFormat for SVGFormat {
   type Output = svg::Document;

   fn format(&self) -> Self::Output {
       todo!("svg implementation")
   }
}

pub struct ImageFormatter<O: OutputFormat> {
    ..., // existing fields
    output_format: O,
}

impl<O: OutputFormat> ImageFormatter<O> {
    // other methods elided

    pub fn format(&self, v: &[Vec<(Style, &str)>], theme: &Theme) -> O::Output {
        todo!("method delegation")
    }
}

Alternatively, the signature of ImageFormatter could be changed to include output_format: impl OutputFormat so that ImageFormatter wouldn't have to include any extra generics.

impl ImageFormatter {
    pub fn format(&self, v: &[Vec<(Style, &str)>] theme: &Theme) -> O::Output {
        todo!("method delegation")
    }
}

Note that using a zero-sized struct would not take up any physical space in method parameters or struct fields.

If this implementation is satisfactory, I'll go ahead an work on it, but if there's something that needs to be done first or something in the way, just let me know!

Aloxaf commented 3 years ago

The design is nice. Just go ahead. :)

I used to work on this feature. But I failed find a way to get a same look on all platforms.

The original svg

What it looks like on my computer 图片

Spaceface16518 commented 3 years ago

Well the thing about SVG is that the actual look is implementation defined, right? So if I view it in a browser like Firefox and then go view it in a native editor like Inkscape, they might look slightly different, and I don't think that's necessarily a bug. The important thing is to emit semantic, bare-bones SVG markup that means the right thing rather than focus too much on getting a consistent look across all platforms.

This is opposed to the philosophy I would have for pixel map pictures—for literal images, cross platform consistency is important.

This is, of course, just my opinion. If you think I should pursue cross-platform consistency for SVG output, I'll do that.

Spaceface16518 commented 3 years ago

With regards to the structure of the implementation, I have committed two separate designs. The associated constant design is the one I proposed above, but it is a little verbose and unwieldy. However, it has the added benefit of allowing users to easily add custom output formats without needing to change the library.

As such, I wrote a different design using trait parameters instead of associated constants. This solution is much less verbose, more idiomatic, and retains the use of ImageFormatter::format, but would cause more breaking changes to the API since using the ImageFormatter::format would actually be provided by the Format trait. It would also not allow users to add custom output formats.

Aloxaf commented 3 years ago

This is, of course, just my opinion. If you think I should pursue cross-platform consistency for SVG output, I'll do that.

No, I think there is no need to do that.

With regards to the structure of the implementation, I have committed two separate designs. The associated constant design is the one I proposed above, but it is a little verbose and unwieldy. However, it has the added benefit of allowing users to easily add custom output formats without needing to change the library.

As such, I wrote a different design using trait parameters instead of associated constants. This solution is much less verbose, more idiomatic, and retains the use of ImageFormatter::format, but would cause more breaking changes to the API since using the ImageFormatter::format would actually be provided by the Format trait. It would also not allow users to add custom output formats.

Oh, I prefer the second one.

BTW, the ImageFormatter is designed for the pixel map output format, I am not sure if all of its fields are necessary for SVG output format or SVG needs more fields. In fact, my original idea is:

struct ImageFormatter;

struct SVGFormatter;

trait Formatter {
  fn format();
}

impl Formatter for ImageFormatter {}
impl Formatter for SVGFormatter {}

Maybe you can consider the API deisgn after finishing the main feature?

Spaceface16518 commented 3 years ago

BTW, the ImageFormatter is designed for the pixel map output format, I am not sure if all of its fields are necessary for SVG output format or SVG needs more fields.

Oh okay. I didn't think about this.


struct ImageFormatter;

struct SVGFormatter;

trait Formatter {

  fn format();

}

impl Formatter for ImageFormatter {}

impl Formatter for SVGFormatter {}

Ah I see, this is a good idea.

Maybe you can consider the API deisgn after finishing the main feature?

Yeah, I'll definitely polish the API afterwords but I just wanted some kind of skeleton to implement inside of to ease the integration process. I think I'll go with your design though. Thank you!