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

vgsvg: express font size in px units, not points #506

Closed
chmike opened this issue Jan 19, 2019 · 22 comments
Closed

vgsvg: express font size in px units, not points #506

chmike opened this issue Jan 19, 2019 · 22 comments

Comments

@chmike
Copy link
Contributor

chmike commented Jan 19, 2019

We need to properly scale the fonts size relative to the graphs size.

Currently, the size of the graph (lines, glyphs) is expressed in "user unit" (px), and the font size in point units (pt).

The size of text relative to the graph would vary with different dpi.

The suggested solution is to express font size in px, which is the same as without unit. This will require to define a scaling factor involving c.dpi.

This is with current master.

@kortschak
Copy link
Member

Can we not convert the user points to be effectively points as suggested in #505 (comment)?

@chmike
Copy link
Contributor Author

chmike commented Jan 19, 2019

Values for <path> can only be given in user units. We are thus constrained to work in user units.

By using the viewBox parameter, as you suggested, we define the user unit.
I did a small experience and set the vgsvg.DPI to 1, and the viewBox to inches: e.g.
<svg width="6in" height="4in" viewBox="0 0 6 4" ....

All the graphics and lines were perfectly scaled, but the text was huge. So I concluded that the pt size is independent of the user unit. Thus, if we want the graphics to look the same of a 90 and 96 dpi display, we have to convert the pt units to user units.

To verify that we do have this font scaling issue, here is an svg image with a framed text. If the dpi changes, I expect that the text won’t nicely fit the frame anymore.

<?xml version="1.0"?>
<!-- Generated by SVGo and Plotinum VG -->
<svg width="6in" height="4in"
	xmlns="http://www.w3.org/2000/svg"
	xmlns:xlink="http://www.w3.org/1999/xlink"
    viewBox="0 0 576 384" > <!-- for dpi=96: 576=6*96 & 384=4*96  -->
<g>
<text x="218" y="192" 
	style="font-family:'Times New Roman', Times, serif;font-weight:normal;font-style:normal;font-size:24pt">
    Hello world !</text>
<path d="M218,192 H388 V170 H218 V192" style="fill:none;stroke:#FF0000;stroke-width:1"/>
</g>
</svg>

Here is what I see with 96 dpi:

image

According to this reference, with 90 dpi, 1pt = 1.25px. This implies that we have 90/1.25=72ppi. With 96dpi, do we still have 72ppi, or do we have a different value ?

@chmike
Copy link
Contributor Author

chmike commented Jan 19, 2019

According to this source unfortunately in french, with 96dpi, 1pt = 1.3333..px. With 1.3333... = 90/72.

The expression to convert pt to px is then: px = pt*dpi/72.

We can pick dpi to be 90, 96 or even 1, it doesn’t matter. By picking the value 1, we can fully remove the need to call the method .Dots(c.dpi) which was my initial suggestion.

@kortschak
Copy link
Member

What I am suggesting is this...

<?xml version="1.0"?>
<!-- Generated by SVGo and Plotinum VG -->
<svg width="576pt" height="384pt"
        xmlns="http://www.w3.org/2000/svg"
        xmlns:xlink="http://www.w3.org/1999/xlink"
    viewBox="0 0 576 384" > <!-- for dpi=96: 576=6*96 & 384=4*96  -->
<g>
<text x="218" y="192" 
        style="font-family:'Times New Roman', Times, serif;font-weight:normal;font-style:normal;font-size:24pt">
    Hello world !</text>
<path d="M218,192 H388 V170 H218 V192" style="fill:none;stroke:#FF0000;stroke-width:1"/>
</g>
</svg>

Which removes the need to have a dpi value.

@chmike
Copy link
Contributor Author

chmike commented Jan 19, 2019

Sorry. My bad. Input parameters are in point units, not inches. The method Dots() divides by 72ppi and multiply by dpi.

I then understand your proposal and agree.

@kortschak
Copy link
Member

I wish it were that simple. I'll send a PR with that change, but it still has issues with text size that I don't understand at all; the text size is too large.

@chmike
Copy link
Contributor Author

chmike commented Jan 20, 2019

Could you push your changes in a branch so I can test and play with it ?

@kortschak
Copy link
Member

I've cc'd you in the PR, but #507.

@chmike
Copy link
Contributor Author

chmike commented Jan 20, 2019

I suggest that in the method FillText (if I remember well) you remove the pt scaling factor. The size of text should be specified as 12 and not 12pt. The effect will be that the text size will be specified in user units which are points.

@kortschak
Copy link
Member

I had thought I had tried that, but it seems I didn't. It looks like that fixes the problem. Thanks.

@chmike
Copy link
Contributor Author

chmike commented Jan 20, 2019

One thing that still worries me is that we specify the plot width and height in pt. We will get the same size everywhere if the ppi is the same everywhere. I’m not so sure of that anymore. What do you think about specifying the width and height of the svg in inches/cm as it was, and leave all the rest as it now is ?

@kortschak
Copy link
Member

The approach in #507 seems correct. Is there any reason to have a non-point size for the svg?

@chmike
Copy link
Contributor Author

chmike commented Jan 20, 2019

A point (pixel/dot) hasn’t the same size on different output devices. We saw it for the default px size (dpi). Thus the size in inches of a pt may also vary with output devices. vg assume that a point size is 1/72th of an inch, but it may not be true everywhere.

I think we should ensure that if the user specifies a 4x4inch plot, he really get a 4x4inch plot. I’m not sure this is what we have now. I would thus suggest to change the code so that we have

fmt.Fprintf(c.buf, `<?xml version="1.0"?>
<!-- Generated by SVGo and Plotinum VG -->
<svg width="%.*gin" height="%.*gin" viewBox="0 0 %.*g %.*g"
	xmlns="http://www.w3.org/2000/svg"
	xmlns:xlink="http://www.w3.org/1999/xlink">`+"\n",
		pr, c.w/vg.Inch,
		pr, c.h/vg.Inch,
		pr, c.w,
		pr, c.h,
	)

@kortschak
Copy link
Member

Thus the size in inches of a pt may also vary with output devices.

I don't believe this is true unless the resolution is insufficiently high (as noted here).

A point is defined as 1/72inch, if SVG doesn't respect that we have other problems, though it says it does.

@chmike
Copy link
Contributor Author

chmike commented Jan 20, 2019

Your references are convincing. Fine with me then. We have to wait for @sbinet to merge the PR.

@kortschak
Copy link
Member

Thanks for fleshing it out with me.

@chmike
Copy link
Contributor Author

chmike commented Jan 20, 2019

I tested the PR and I get exactly the same size for png and svg plots on my screen.

For text size, when using Times-Roman the text length is identical. But when I use Helvetica, the text length is not the same, and I see that the font is not exactly the same. I have the impression that the Helvetica font used to generate the png image is not the "real" Helvetica. But that is another problem. The Courier font yields the same text size.

@chmike
Copy link
Contributor Author

chmike commented Jan 23, 2019

Change merged

@chmike chmike closed this as completed Jan 23, 2019
@chmike chmike reopened this Jan 23, 2019
@chmike
Copy link
Contributor Author

chmike commented Jan 23, 2019

I’m worried that I don’t get the desired output when displaying the SVG with CodePen.
Here I try to display testdata/scatter_golden.svg. The font is too big.

https://codepen.io/anon/pen/QYwYym

When adding px after the font size, the problem is solved. It looks like a small fix is still needed.

Here is the result I get when adding px to each font size specification.

https://codepen.io/anon/pen/QYwYbQ

@chmike
Copy link
Contributor Author

chmike commented Jan 23, 2019

Should I create a new PR to fix that after #512 is fixed ?

@kortschak
Copy link
Member

Pleas take a look at #513.

@chmike
Copy link
Contributor Author

chmike commented Jan 30, 2019

Problem solved.

@chmike chmike closed this as completed Jan 30, 2019
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