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

png: rsvg-convert implementation #51

Open
ccoVeille opened this issue Mar 27, 2024 · 3 comments
Open

png: rsvg-convert implementation #51

ccoVeille opened this issue Mar 27, 2024 · 3 comments

Comments

@ccoVeille
Copy link
Contributor

ccoVeille commented Mar 27, 2024

There is something strange to me with current code

freeze/png.go

Lines 14 to 31 in 4fcacff

func libsvgConvert(doc *etree.Document, w, h float64, output string) error {
_, err := exec.LookPath("rsvg-convert")
if err != nil {
return err
}
svg, err := doc.WriteToBytes()
if err != nil {
return err
}
// rsvg-convert is installed use that to convert the SVG to PNG,
// since it is faster.
rsvgConvert := exec.Command("rsvg-convert", "-o", output)
rsvgConvert.Stdin = bytes.NewReader(svg)
err = rsvgConvert.Run()
return err
}

You are not using w and h variables.

So either, you should simplify the signature, or use them as parameter for rsvg-convert

as it supports it

USAGE:
    rsvg-convert [FLAGS] [OPTIONS] [FILE]...

FLAGS:
    -?, --help                  Prints help information
    -a, --keep-aspect-ratio     Preserve the aspect ratio
        --keep-image-data       Keep image data
        --no-keep-image-data    Do not keep image data
    -u, --unlimited             Allow huge SVG files
    -v, --version               Prints version information

OPTIONS:
    -l, --accept-language <languages>    Languages to accept, for example "es-MX,de,en" [default uses language from the
                                         environment]
    -b, --background-color <color>       Set the background color using a CSS color spec
    -i, --export-id <object id>          SVG id of object to export [default is to export all objects]
    -f, --format <format>                Output format [default: png]  [possible values: Png, Pdf, Ps, Eps, Svg]
        --left <length>                  Distance between left edge of page and the image [defaults to 0]
    -o, --output <output>                Output filename [defaults to stdout]
        --page-height <length>           Height of output media [defaults to the height of the SVG]
        --page-width <length>            Width of output media [defaults to the width of the SVG]
    -d, --dpi-x <number>                 Pixels per inch [default: 96]
    -p, --dpi-y <number>                 Pixels per inch [default: 96]
    -w, --width <length>                 Width [defaults to the width of the SVG]
    -h, --height <length>                Height [defaults to the height of the SVG]
    -s, --stylesheet <filename.css>      Filename of CSS stylesheet to apply
        --top <length>                   Distance between top edge of page and the image [defaults to 0]
    -z, --zoom <number>                  Zoom factor
    -x, --x-zoom <number>                Horizontal zoom factor
    -y, --y-zoom <number>                Vertical zoom factor

So the code would be something like this

	rsvgConvert := exec.Command("rsvg-convert", "-o", output, "-w", fmt.Sprintf("%.0f", w), "-h", fmt.Sprintf("%.0f", w))
@ccoVeille
Copy link
Contributor Author

BTW, what is the current status with rsvg-convert support.

I see nothing in the readme about it, the code does not suggest installing it to speed up PNG generation.

@maaslalani
Copy link
Member

Hey @ccoVeille, are you noticing issues with the --width and --height?

We do all the calculations in the SVG such that we don't need to specify the width and height during the conversion but let me know if you are seeing incorrect behaviour.

@ccoVeille
Copy link
Contributor Author

ccoVeille commented Mar 28, 2024

Hey @ccoVeille, are you noticing issues with the --width and --height?

We do all the calculations in the SVG such that we don't need to specify the width and height during the conversion but let me know if you are seeing incorrect behaviour.

Nope, I just noticed the signature was defining unused variables.

I was suggesting to either pass the variables to binary or remove them from the function signature.

I think it confirms they can be removed. If you think they should stay for signature consistency. You could use _, _ float64 but I don't think it's a good idea.

At first, I thought your code was relation on interfaces, so you had to get them in the signature.

It's up to you.

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

No branches or pull requests

2 participants