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

vg/vgsvg: bump DPI to 96, introduce functional options #492

Merged
merged 1 commit into from Jan 17, 2019

Conversation

sbinet
Copy link
Member

@sbinet sbinet commented Jan 7, 2019

Fixes #491.

Please take a look.


// UseDPI sets the dots per inch of a canvas. It should only be
// used as an option argument when initializing a new canvas.
func UseDPI(dpi int) option {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason for integral DPI?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mainly for consistency with e.g. vg/vgimg.UseDPI.
I can switch to float64 if you prefer. (we're not super consistent on this front, sometimes the DPI field is float64 sometimes int...)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's revisit that in another issue.

@sbinet sbinet force-pushed the issue-491 branch 2 times, most recently from 14af1dc to d908fc5 Compare January 11, 2019 09:19
@chmike
Copy link
Contributor

chmike commented Jan 11, 2019

I suggested to change the DPI value because the current value is incorrect. This should be fixed ASAP because it is a bug.

It's OK to leave it as int for now because in reality we should be using the viewBox <svg> parameter instead of this DPI scaling. If the units of the input data are in Inches, we can define the <svg> tag like this: <svg width="4in" height="4in" viewBox="0 0 4 4"> and don’t need to multiply each value by vgsvg.DPI anymore. A point with coordinate 0,0 will be top left, and a point with coordinate 4,4 will be bottom right.

I suggest, we first fix the DPI value to get correct svg images, and I work on a PR to replace the DPI values with the viewBox parameter. Is this suggestion OK ?

@sbinet
Copy link
Member Author

sbinet commented Jan 16, 2019

ping?


// UseDPI sets the dots per inch of a canvas. It should only be
// used as an option argument when initializing a new canvas.
func UseDPI(dpi int) option {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's revisit that in another issue.

@sbinet sbinet merged commit 3cfdcb1 into gonum:master Jan 17, 2019
@sbinet sbinet deleted the issue-491 branch January 17, 2019 09:01
sbinet added a commit to sbinet-hep/hep that referenced this pull request Jan 17, 2019
This is a consequence of gonum/plot#492 bumping the DPI from 90 to 96.
sbinet added a commit to sbinet-hep/hep that referenced this pull request Jan 17, 2019
This is a consequence of gonum/plot#492 bumping the DPI from 90 to 96.
sbinet added a commit to go-hep/hep that referenced this pull request Jan 17, 2019
This is a consequence of gonum/plot#492 bumping the DPI from 90 to 96.
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

Successfully merging this pull request may close these issues.

Change vgsvg.DPI from 90 to 96
3 participants