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

[Bug]: Missing text position in SVG #9489

Open
7 tasks done
more-strive opened this issue Nov 9, 2023 · 15 comments
Open
7 tasks done

[Bug]: Missing text position in SVG #9489

more-strive opened this issue Nov 9, 2023 · 15 comments
Assignees

Comments

@more-strive
Copy link

CheckList

  • I agree to follow this project's Code of Conduct
  • I have read and followed the Contributing Guide
  • I have read and followed the Issue Tracker Guide
  • I have searched and referenced existing issues and discussions
  • I am filing a BUG report.
  • I have managed to reproduce the bug after upgrading to the latest version
  • I have created an accurate and minimal reproduction

Version

6.0.0-beta9

In What environments are you experiencing the problem?

Chrome

Node Version (if applicable)

18.16.0

Link To Reproduction

https://codesandbox.io/p/sandbox/fabric-vanillajs-sandbox-forked-wqznkq?file=%2Fsrc%2Findex.ts%3A11%2C23

Steps To Reproduce

  1. The page.svg of the sample file in assets

Expected Behavior

Text displayed in the specified position

Actual Behavior

Text in the bottom left corner of the canvas

Error Message & Stack Trace

No response

@ShaMan123
Copy link
Contributor

@ShaMan123
Copy link
Contributor

can you reduce th svg to <5 text elements
too big to debug

@more-strive
Copy link
Author

page-text-0
removed other text, with 5 elements remaining

@ShaMan123
Copy link
Contributor

@gloriousjob
Copy link
Contributor

    <svg id="svg" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" version="1.1" width="841.92pt" height="595.32pt" viewBox="0 0 841.92 595.32">
        <text xml:space="preserve" transform="matrix(1 0 -0 1 0 595.32)" font-size="5.4" font-family="Dotum"><tspan y="-422.26" x="222.02 225.14 228.26">135</tspan></text>
    </svg>

I simplified the svg to the above and see the issue (or at least one of them). It looks like the text element should be able to handle tspan subelements but I don't see where Text.fromElement is handling tspan subelements. It looks like the tspan should be used to place the location of the text. This doesn't appear to be in v5 either, at least looking at the code. https://www.w3.org/TR/SVG11/text.html#TSpanElement
https://www.w3.org/TR/SVG2/text.html#TextElement

I'm also confused on why Text.fromElement is replacing dx and dy with 0 instead of reading what's in the svg. Similar with top and left from options. It looks like the v5 code would read top and left from options and set to 0 if they weren't populated but the v6 code just sets to 0.

@gloriousjob
Copy link
Contributor

I found this test suite: http://fabricjs.com/test/svg_import/text.html
If this was working at some point, it doesn't seem to be now... any idea when it was good?

@asturur
Copy link
Member

asturur commented Nov 12, 2023

tspan in svg import aren't supported, while should be supported in output.

@asturur
Copy link
Member

asturur commented Nov 12, 2023

Supporting TSPAN is on my old TODO list, maybe as soon as 6.0 is out i ll do it.
Is not hard and we have 'dy' in text that was built to support some math notations but is also good for tspans where appropriate.

@asturur asturur self-assigned this Nov 12, 2023
@asturur
Copy link
Member

asturur commented Nov 12, 2023

duplicate of #1280

@gloriousjob
Copy link
Contributor

gloriousjob commented Nov 14, 2023

@asturur I'm willing to help with this. I'm just not sure what the idea would be. When a tspan appears, should that be handled in the same FabricText or do you think there may be cases where we'd be forced to use separate FabricTexts? Or is this something where we can get fancy with styling to solve the problem?

Something else I noticed is that the svg text supports arrays for the x value (and other values), so that each letter may be handled in the array (but the code assumes only the single value case). One example provided is rotate, so that you see the first couple of letters in a different rotation than the last ones. The array case is actually what's driving my same vs separate question above.

@more-strive
Copy link
Author

The original file is a PDF file, and the SVG obtained through MuPDF API conversion is uncertain whether MuPDF can remove tspan

@ShaMan123
Copy link
Contributor

@more-strive do you mind sharing an example of using MuPDF API to convert PDF to SVG?

@more-strive
Copy link
Author

This is the official command line
https://mupdf.readthedocs.io/en/latest/mutool-convert.html
image

@asturur
Copy link
Member

asturur commented Nov 20, 2023

@asturur I'm willing to help with this. I'm just not sure what the idea would be. When a tspan appears, should that be handled in the same FabricText or do you think there may be cases where we'd be forced to use separate FabricTexts? Or is this something where we can get fancy with styling to solve the problem?

Something else I noticed is that the svg text supports arrays for the x value (and other values), so that each letter may be handled in the array (but the code assumes only the single value case). One example provided is rotate, so that you see the first couple of letters in a different rotation than the last ones. The array case is actually what's driving my same vs separate question above.

I can expand on this, because i thought about it for long time, but i need time to sit down and write everything.

@asturur
Copy link
Member

asturur commented Nov 20, 2023

So if you have seen the text.fromElement

    const textContent = (element.textContent || '')
      .replace(/^\s+|\s+$|\n+/g, '')
      .replace(/\s+/g, ' ');

We get the textContent of the Text element and we trash everything that is an html/svg tag.

What we would like to do is to parse the childNodes of that text element, divide textNodes from other kind of nodes ( opefully the others are only tspan ) and start moving tspan attributes to style properties.
This requires keeping track to what index of text a tspan starts and end using graphemes and not simple chars, so it requires also to know the full text string anyway

That would be the basic implementation.

There will be issues, like TSPAN used to look like text lines that will be a single line that gets displaced.
There are other issues like the fact that if i m not wrong text posiont after a tspan continues after the last tspan. ( i have to check the specs for layouting https://www.w3.org/TR/2006/CR-SVGMobile12-20060810/text.html#TextLayout ).

There are different level of efforts that can bring us to a nicer support and get to mostly complete support with time.

  • style attributes support that aren't geometric ( color, fontFamily, fontSize.... )
  • position support where it makes sense ( dx/dy/x/y/rotate)
  • what else is left

There are also some decisions to take, like style does not support x/y but supports dx/dy so eventually a map between x with dx and y with dy needs to be arranged.

I'm sure you can help but i would really do some feature development instead of just ts migrations and api changes, so i m looking forward to start it

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

No branches or pull requests

4 participants