SVG icon indicating copy to clipboard operation
SVG copied to clipboard

text-anchor middle not rendering correctly

Open Stoffelche opened this issue 3 years ago • 16 comments

Description

I have a simple SVG, where the text with text-anchor middle is rendered incorrectly

Example data

<svg xmlns="http://www.w3.org/2000/svg" >
	<rect fill="#0669b2" stroke="none" stroke-width="0" width="120" height="60"  />
	<svg width="120" height="60">
		<text font-size="11pt" fill="white" text-anchor="middle" transform="matrix(1 0 0 1 60 30)" >
			<tspan dy="0.3em">Some text</tspan>
		</text>
	</svg>
</svg>

SVG.Net renders: grafik but the correct rendering should be: grafik

(don't look at the image paddings they are just screenshots, one from your SVG Viewer the other from a browser). The point is that, SVG.Net starts the text in the middle position but does not position the the text horizontally centered.

Used Versions

Master

Stoffelche avatar Dec 04 '20 17:12 Stoffelche

I found, that when I initialize _xAnchor to 0 in the second ctor of TextDrawingState
public TextDrawingState(TextDrawingState parent, SvgTextBase element)
then my sample renders correctly. Then I tried another sample, which has more nodes and connections inbetween (the samples are from svg rendered by joint.js). And with those I experienced the following strange behavior:
When I format th xml with line breaks and tab-indents, the problem with the text not being aligned to middle occurs again.
However, if i delete all these \r, \n, \t, then the text is aligned correctly, after my change mentioned above.
Also, when I tried to debug the formatted xml, I found that the white chars are meddled somehow with the text-attributes of the tspan elements.
The formatting does not have any effect on the rendering in browsers, so I suspect, there is something wrong with respect to handing of it, in the svg.net code as well.

Stoffelche avatar Dec 17 '20 08:12 Stoffelche

Thanks for the detailed description!

tebjan avatar Dec 18 '20 02:12 tebjan

If you want to look into the problem with the formatted xml:
TestSVG.zip I have uploaded a zip with the sources for the svg with and without formatting

Stoffelche avatar Dec 18 '20 08:12 Stoffelche

May also be related to #742.

mrbean-bremen avatar Jan 01 '21 19:01 mrbean-bremen

The fix for the original problem (in the constructor) looks good to me (this was clearly forgotten). The whitespace problem seems to be an independent bug.

mrbean-bremen avatar Jan 01 '21 20:01 mrbean-bremen

Did you fix it in the master branch?
I tried pushing a new branch with this fix but did not succeed

Stoffelche avatar Jan 04 '21 12:01 Stoffelche

Concerning the whitespace problem: I found that it is triggered by the xml:space="preserve" in the text-element.
But this does not change rendering in browsers, so it should not have any impact in the rendering.

Stoffelche avatar Jan 04 '21 12:01 Stoffelche

Did you fix it in the master branch?

No, not yet - can do this later today. Thanks for the investigation!

mrbean-bremen avatar Jan 04 '21 15:01 mrbean-bremen

@Stoffelche - I added a PR, can you please have a look if this is ok for you?

mrbean-bremen avatar Jan 04 '21 20:01 mrbean-bremen

The PR is fine.

Stoffelche avatar Jan 05 '21 08:01 Stoffelche

What about the xml:space="preserve" problem? I tried to find out, what the problem is, but it looks complicated. I found that text-anchor is applied in FlushPath, but this is only applied to the tspan-element, not to the content-text before and after the tspan. The content in this case are the whitespace chars, which are preserved. I think the text-anchor should be applied to all the content inside the text-element and not only the tspan

Stoffelche avatar Jan 05 '21 08:01 Stoffelche

The problem is demonstrated with the following modified example:

<svg xmlns="http://www.w3.org/2000/svg" >
	<rect fill="#0669b2" stroke="none" stroke-width="0" width="120" height="60"  />
	<svg width="120" height="60">
		<text font-size="11pt" fill="white" text-anchor="middle" transform="matrix(1 0 0 1 60 30)" >
			ab
			<tspan>inner</tspan>
			cd
		</text>
	</svg>
</svg>

The text "ab inner cd" should be rendered as a whole with text-anchor middle as can be verified in a browser. But svg.net only shifts the tspan-element somehow to the left, when applying the text-anchor and does not shift the both content-elements, resulting in overlaps of the text.

Stoffelche avatar Jan 05 '21 08:01 Stoffelche

Thanks for the investigation - I will have a look in the evening, and merge the PR.

mrbean-bremen avatar Jan 05 '21 10:01 mrbean-bremen

Another sample, which is not correctly rendered:

<svg xmlns="http://www.w3.org/2000/svg" >
	<rect fill="#0669b2" stroke="none" stroke-width="0" width="120" height="60"  />
	<svg width="120" height="60">
		<text font-size="11pt" fill="white" text-anchor="middle" transform="matrix(1 0 0 1 60 30)" >
			<tspan>sp1</tspan>
			<tspan>sp2</tspan>
			<tspan>sp3</tspan>
		</text>
	</svg>
</svg>

Stoffelche avatar Jan 05 '21 12:01 Stoffelche

Having it investigated further I found various problems: Firstly TextAnchor property is implemented as inherit-attribute, which means that the tspan elements inherit it from the text. This is incorrect IMO because not the individual elements should be shifted to middle but the whole resulting path, similar to the transform-matrix being applied to the path as a whole.

While trying to fix it, I found that there is some confusing handling of the _path field. First it is being created during the call of the Bounds property, during which the pathes of the children are added. Then it is re-used during the render-process, where it already has the children's path added. But during the render-process, with the call of RenderChildren, the children are rendered explicitly, too, resulting in the three tspan elements of the example above being rendered twice. This can be verified, by skipping the RenderChildren call.

Also, I think it is not a good idea to have the pathes of the children added to the path (path being in effect the cached _path,) in the Bounds property. Either the path should be cloned in the Bounds property before adding the children's pathes. Or the children's pathes should already be added int the Path() method, in which case one could rely on them being included in the cached path, making RenderChildren definitely unnecessary.

If one sticks to the concept of _path containing only the content nodes and not the child elements' pathes, then at least the Bounds values should be cached, as they are needed again during rendering, in order for the shift needed for the TextAnchor-based transformation.

The whole concept of doing so much work during a simple Bounds-property call seems not very good design to me. Instead there should first be some kind of prepare-method to recursively create the cached _path etc.

Are there any comments on how to solve this?

Stoffelche avatar Jan 09 '21 13:01 Stoffelche

@H1Gdev , @wieslawsoltes - you have been working on related code, any insights here?

mrbean-bremen avatar Jan 21 '21 20:01 mrbean-bremen