Bug: Context2d issues with units

[eKoopmans]

Hi, I'm following up on scaling issues revealed in this fiddle (text is strangely spaced when using default jsPDF units): https://jsfiddle.net/eKoopmans/egm94jqh/

This comes from writing each word separately, and specifying the x/y coordinates of each. When using an HTML canvas, units are expected to always be in pixels (e.g. fillText). However the jsPDF Context2d gets pretty confused with units:

• (I've only focused on text methods so far)
• .font sets the PDF font size to the same numeric value, regardless of what units were specified (problem - should be scaled based on specified units (e.g. 12px) and document units; related problem - the regex doesn't correctly identify the units)
• .fillText receives x and y as pixels
• it adjusts the y value using getBaseline, which uses the current font size scaled based on the document units (problem - should be scaled a second time into pixels to match y)
• x and y are passed on to .putText as still mostly pixels
• .putText modifies x and y based on canvas transformations (fine...?), then sends to API.text (problem - these should be scaled from pixels to document unit before sending)
• API.text expects x and y in document units (not pixels), and scales everything to pt
• since pt is the default unit (scaleFactor = 1), it's the only unit that ends up looking correct

So, there's 3-4 problems so far, but I think a larger audit is necessary of all the assumptions being made about units in Context2d. It seems to me the best approach is to keep everything internally as pixels within Context2d, and make sure to adjust correctly whenever interacting with methods external to Context2d.

[Uzlopak]

I agree with your findings. Scaling is an issue. Without blaming anyone, context2d was previously just passing the units to jspdf without rescaling the values depending on the used unit system. Scaling was made by setting the unit in the initialization of the jspdf instance.

So by digging deeper, we find those issues ;).

Tbh, it was kind of a debugging mess to fix font syntax recognition, till i found the used regex on SO (maybe modified it too, I forgot). So I was kind of happy, that the font syntax is atleast working in pt.

putText is the internal method preparing the text. So you say we should first rescale px to pt and then scale to document units?

[eKoopmans]

Yeah, x * (72/96) / pdf.internal.scaleFactor on the way out of putText (when calling API.text).

The other option would be to fix x on the way into .fillText, and represent everything in terms of the document units from the start. That might actually be the more consistent approach with the rest of jsPDF.

One question though, once a PDF document is created are people allowed to change the base units, or is it fixed? If it can change, it might be safer to keep it as pixels and only convert on the way out...

Edit: Fixed the calculation, should be / scaleFactor not * scaleFactor.

[Uzlopak]

I think I programmed methods to change the base units.

[Uzlopak]

.font sets the PDF font size to the same numeric value, regardless of what units were specified (problem - should be scaled based on specified units (e.g. 12px) and document units; related problem - the regex doesn't correctly identify the units)

Realized that the regex is not giving the fontUnit but giving the lineHeight.

fixed that with #2419

but more stuff has to be done.....

[Uzlopak]

What if instead of doing heavy lifting by doing all the recalculations we just set a transformationmatrix at the beginning and reset it at the end?

[DharanBro]

Do we have a way to access the current document's units(pt or px etc..)? in case if we need to do some calculations based on it.

[HackbrettXXX]

You cannot access the unit directly, but you can get the current scale factor, which is derived from the unit: pdf.internal.scaleFactor. See also https://github.com/MrRio/jsPDF/blob/master/src/jspdf.js#L3109

[github-actions[bot]]

This issue is stale because it has been open 90 days with no activity. It will be closed soon. Please comment/reopen if this issue is still relevant.

[HackbrettXXX]

@eKoopmans could you check if this is still an issue on the current master branch?