search

scour-project / scour

Scour - An SVG Optimizer / Cleaner
Apache License 2.0
780 stars 59 forks source link

font-size="2em" is removed #209

Open JoKalliauer opened 6 years ago

JoKalliauer commented 6 years ago

Processiong File:Envelope_string_art.svg with

$ scour input.svg.txt output.svg.txt

removes font-size="2em"

input.svg.txt 

<?xml version="1.0" encoding="UTF-8"?>
<svg color-interpolation-filters="sRGB" fill-rule="evenodd" font-size="12" stroke-linecap="square" stroke-miterlimit="3" viewBox="0 0 1647.8 1154.6" xmlns="http://www.w3.org/2000/svg">
 <text transform="translate(1024.2 -1055.9)" x="6.6" y="1113.9" font-family="Microsoft YaHei UI" font-size="2em" font-weight="700">(半原创)截止2016年松江新城轨道交通系统线路图 <tspan x="182.4" dy="1em" font-size="1em">作者</tspan>：QFA7301 <tspan x="177.1" dy="1em" font-size="1em">素材</tspan>：<tspan font-size="1em">Daniel</tspan><tspan font-size="1em">129</tspan><tspan x="60.2" dy="1em" font-size="1em">感谢</tspan><tspan font-size="1em">Daniel</tspan><tspan font-size="1em">129</tspan>提供的上海地铁svg文件</text>
 <text transform="translate(1007.6 -225.3)" x="19.1" y="1125.9" font-family="Microsoft YaHei UI" font-size="2em" font-weight="700">T2线运营方式：T2线至锦昔路后续接T1线往北至新桥站 <tspan x="142" dy="1em" font-size="1em">T</tspan>1线运营方式：沿即有T1线行驶 <tspan x="24.1" dy="1em" font-size="1em">内部环线</tspan>：锦昔路-泰晤士小镇北站-三新北路-松卫公路</text>
</svg>

input

The output is: output.svg.txt

<?xml version="1.0" encoding="UTF-8"?>
<svg color-interpolation-filters="sRGB" fill-rule="evenodd" font-size="12" stroke-linecap="square" stroke-miterlimit="3" viewBox="0 0 1647.8 1154.6" xmlns="http://www.w3.org/2000/svg">
 <text transform="translate(1024.2 -1055.9)" x="6.6" y="1113.9" font-family="Microsoft YaHei UI" font-weight="700">(半原创)截止2016年松江新城轨道交通系统线路图 <tspan x="182.4" dy="1em" font-size="1em">作者</tspan>：QFA7301 <tspan x="177.1" dy="1em" font-size="1em">素材</tspan>：<tspan font-size="1em">Daniel</tspan><tspan font-size="1em">129</tspan><tspan x="60.2" dy="1em" font-size="1em">感谢</tspan><tspan font-size="1em">Daniel</tspan><tspan font-size="1em">129</tspan>提供的上海地铁svg文件</text>
 <text transform="translate(1007.6 -225.3)" x="19.1" y="1125.9" font-family="Microsoft YaHei UI" font-weight="700">T2线运营方式：T2线至锦昔路后续接T1线往北至新桥站 <tspan x="142" dy="1em" font-size="1em">T</tspan>1线运营方式：沿即有T1线行驶 <tspan x="24.1" dy="1em" font-size="1em">内部环线</tspan>：锦昔路-泰晤士小镇北站-三新北路-松卫公路</text>
</svg>

output

so font-size="2em" is removed in both <text>-tags

Copyright License: [{{self|cc-by-sa-3.0,2.5,2.0,1.0|GFDL}}](https://creativecommons.org/licenses/by-sa/3.0/deed) Author: [{{Ut|QFA7301}}](https://commons.wikimedia.org/wiki/User_talk:QFA7301), [{{Uc|Daniel129}}](https://commons.wikimedia.org/wiki/User_talk:Daniel129) Source: https://upload.wikimedia.org/wikipedia/commons/6/61/2016SongjiangTram.svg Details: [File:2016SongjiangTram.svg](https://commons.wikimedia.org/wiki/File:2016SongjiangTram.svg
nthykier commented 4 years ago

This and #210 have the same underlying cause, namely that removeUnusedAttributesOnParent prunes the relevant attribute causing the bug. Looking at that function, it seems like it is only concerned about whether the attribute is overwritten later and not whether the current node make use of the attribute itself.

@Ede123 Do you have any input for how this should be corrected? I note that removeUnusedAttributesOnParent operations on a lot of attributes and some of them have broader scope than just text.

Ede123 commented 4 years ago

Yeah, that function seems dangerous...

I'm afraid we won't be able to fix this easily as long as we don't add proper inheritance rules for all attributes to Scour (i.e. we actually know which attributes apply to which elements).

We might be able to get away with treating text elements special as a workaround (i.e. check if there are non-empty text-nodes before removing styles from the parent text element) but I'm afraid this could become ugly code-wise while potentially leading to missed optimizations.

P.S. One more odd thing I also noticed in removeUnusedAttributesOnParent:
There's a if len(childElements) <= 1: in there that I don't understand - why would we want to not optimize parents with one child?

JoKalliauer commented 4 years ago

Is there an option to dissable removeUnusedAttributesOnParent?

E.g. https://tools.wmflabs.org/svgworkaroundbot/ uses scour to make workarounds for librsvg, this script is used for SVGWorkaroundBot on Commons. So the Bot ideally should be able to process files without human review, therefore it needs a save option, with "minimal file-changes".

nthykier commented 4 years ago

Yeah, that function seems dangerous...

I'm afraid we won't be able to fix this easily as long as we don't add proper inheritance rules for all attributes to Scour (i.e. we actually know which attributes apply to which elements).

[...]

What would this take? Looking at the attribute for all elements above the current one until a match is found (or we reach the document root)?

P.S. One more odd thing I also noticed in removeUnusedAttributesOnParent: There's a if len(childElements) <= 1: in there that I don't understand - why would we want to not optimize parents with one child?

No clue; I think this is the first time I am working with this function, but it does seem weird glance and would do well with a comment if there is a valid reason.