Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SVG support #6

Open
Aloxaf opened this issue Jul 15, 2019 · 6 comments
Open

SVG support #6

Aloxaf opened this issue Jul 15, 2019 · 6 comments
Labels
enhancement New feature or request

Comments

@Aloxaf
Copy link
Owner

Aloxaf commented Jul 15, 2019

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

@Aloxaf Aloxaf added the enhancement New feature or request label Jul 19, 2019
@Spaceface16518
Copy link

Spaceface16518 commented Nov 4, 2020

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
Copy link
Owner Author

Aloxaf commented Nov 4, 2020

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
Copy link

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
Copy link

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
Copy link
Owner Author

Aloxaf commented Nov 5, 2020

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
Copy link

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants