yWorks / svg2pdf.js

A javascript-only SVG to PDF conversion utility that runs in the browser. Brought to you by yWorks - the diagramming experts
MIT License
649 stars 98 forks source link

wrong export from Highcharts svg with outlined text/ #192

Open inc198107 opened 2 years ago

inc198107 commented 2 years ago

When export from chart with outlined text (nested tSpan used) cintent is duplicated. screenshot

after some debugging svg2pdf library i am founded cause of described bug, it is part of code in textchunk.ts that set x coordinate of tSpan (string 111 - 112 ) image after described changes everything works fine screenshot after string commented

(sorry for my weak english)

Angular 10, Highcharts 9.2.1 , jsPDF 2.4.0, svg2pdf 2.2.0 Chrome 91.0.4472.124 (Official Build) (64-bit)

that used svg:

`

Created with Highcharts 9.1.2 -- POWERPOWEROTHEROTHERGASGASData Set 1 Ad-hoc/UMM Count (Affected asset)Top Level`
HackbrettXXX commented 2 years ago

Thanks for reporting this bug. Could you reduce your SVG to only what's necessary to reproduce the bug?

inc198107 commented 2 years ago

It is little bit hard, because svg generated by Highcharts. But problematic part is here : <text x="5" data-z-index="1" y="16" style="color:#000000;font-size:11px;font-weight:bold;fill:#000000;"> <tspan class="highcharts-text-outline" fill="#FFFFFF" stroke="#FFFFFF" stroke-width="2px" stroke-linejoin="round">POWER<tspan x="5" y="16">​</tspan></tspan>POWER</text></g><g class="highcharts-label highcharts-data-label highcharts-data-label-color-undefined" data-z-index="1" transform="translate(0,-9999)" opacity="0"><text x="5" data-z-index="1" y="16" style="color:#000000;font-size:11px;font-weight:bold;fill:#000000;"><tspan class="highcharts-text-outline" fill="#FFFFFF" stroke="#FFFFFF" stroke-width="2px" stroke-linejoin="round" style="">OTHER<tspan x="5" y="16">​</tspan></tspan>OTHER</text></g><g class="highcharts-label highcharts-data-label highcharts-data-label-color-undefined" data-z-index="1" transform="translate(436,263)"><text x="5" data-z-index="1" y="16" style="color:#000000;font-size:11px;font-weight:bold;fill:#000000;"><tspan class="highcharts-text-outline" fill="#FFFFFF" stroke="#FFFFFF" stroke-width="2px" stroke-linejoin="round">GAS<tspan x="5" y="16">​</tspan></tspan>GAS</text>

HackbrettXXX commented 2 years ago

I've reduced it a bit further (and fixed it):

<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 500 500">
  <g>
    <text x="5" y="16" style="color:#000000;font-size:11px;font-weight:bold;fill:#000000;">
      <tspan fill="#FFFFFF" stroke="red" stroke-width="2px" stroke-linejoin="round">POWER
        <tspan x="5" y="16">​</tspan>
      </tspan>
      POWER
    </text>
  </g>
</svg>

This renders to image

Could you create a pull request? Although we have to test the suggested fix against other text examples. I'm pretty sure, adding the text width there happens for a reason.

inc198107 commented 2 years ago

ok

inc198107 commented 2 years ago

Unfortunately, I don't have permission to create a new branch in your repository, so I can't make a pull request.

HackbrettXXX commented 2 years ago

In order to create a pull request you have to fork the repository: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/creating-a-pull-request

inc198107 commented 2 years ago

Thanks a lot. I will do it in few days

пн, 13 дек. 2021 г. в 11:07, Lukas Holländer @.***>:

In order to create a pull request you have to fork the repository: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/creating-a-pull-request

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/yWorks/svg2pdf.js/issues/192#issuecomment-992251195, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEEJ4DXGJ4FFAFUY3MG457TUQWZTXANCNFSM5HZ3BURA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

karolkolodziej commented 2 years ago

Hi! Any updates on this issue? Based on this issue reported in Highcharts I was able to reproduce that in your online playground as:

<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 300 150">
  <text x="5" data-z-index="1" y="16" style="color: rgb(0, 0, 0); font-size: 11px; font-weight: bold; fill: rgb(0, 0, 0);">
    <tspan class="highcharts-text-outline" fill="red" stroke="red" stroke-width="2px" stroke-linejoin="round" style="">29.9<tspan x="5" y="16">&ZeroWidthSpace;</tspan>
    </tspan>29.9
  </text>
</svg>

image

HackbrettXXX commented 2 years ago

There is a PR (#198), but @inc198107 hasn't responded in a while. @karolkolodziej any assistance moving the PR forward would be very welcome.

inc198107 commented 2 years ago

Sorry I had no time to recheck this previously. But now, as you now, we have war with Russian nazi(I am from Ukraine), houpfully after our victory I will return to this task

пт, 8 апр. 2022 г., 14:47 Lukas Holländer @.***>:

There is a PR (#198 https://github.com/yWorks/svg2pdf.js/pull/198), but @inc198107 https://github.com/inc198107 hasn't responded in a while. @karolkolodziej https://github.com/karolkolodziej any assistance moving the PR forward would be very welcome.

— Reply to this email directly, view it on GitHub https://github.com/yWorks/svg2pdf.js/issues/192#issuecomment-1092777900, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEEJ4DTMEPLNB2BYO5L2OGTVEAMGNANCNFSM5HZ3BURA . You are receiving this because you were mentioned.Message ID: @.***>

karolkolodziej commented 2 years ago

Thank you for the quick response.

A few comments on the issue and the PR:

Perhaps before calculating currentTextX you should check if an element of &ZeroWidthSpace is being introduced. Pseudocode:

if (textNodeContext === '&ZeroWidthSpace') {
   currentTextX = x
} else {
   currentTextX = x + textMeasure.width + textMeasure.length * charSpace
}

Sorry I can't help more but I'm not entirely familiar with that library and also I'm not sure how to test it...

inc198107 commented 2 years ago

Thanks for the feedback!

пт, 8 апр. 2022 г., 16:29 Karol Kołodziej @.***>:

Thank you for the quick response.

A few comments on the issue and the PR:

  • There are some tests failing with this approach (replacing the calculation with currentTextX = x.
  • This approach(☝️) seems to work in our case but overall it doesn't look like a rounding/float imprecision issue it looks more like an actual error with the calculation.
  • I was able to simplify the code giving such results even more:
29.9 29.9

Perhaps before calculating currentTextX you should check if an element of &ZeroWidthSpace is being introduced. Pseudocode:

if (textNodeContext === '&ZeroWidthSpace') {

currentTextX = x

} else {

currentTextX = x + textMeasure.width + textMeasure.length * charSpace

}

Sorry I can't help more but I'm not entirely familiar with that library and also I'm not sure how to test it...

— Reply to this email directly, view it on GitHub https://github.com/yWorks/svg2pdf.js/issues/192#issuecomment-1092861909, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEEJ4DXFPR76MJ7RBNWSBI3VEAYEJANCNFSM5HZ3BURA . You are receiving this because you were mentioned.Message ID: @.***>

HackbrettXXX commented 2 years ago

@karolkolodziej thanks for doing the tests and for the additional insight.