Open nthykier opened 3 years ago
Would this still create a group and propagate the fill
to the parent for the following?
<rect fill="#f00" x="10" y="10" width="10" height="10"/>
<rect fill="red" x="20" y="20" width="10" height="10"/>
Would this still create a group and propagate the
fill
to the parent for the following?[...]
Neither the old nor the new code gets this consistently right because that is governed by another part of the code. For reference, the new code gets your particular example correct. However, I would expect both the old and the new code to fail that test with e.g. cyan
vs aqua
vs #0ff
.
For that to (always) work, then we would also have to change:
newColorValue = convertColor(oldColorValue)
oldBytes = len(oldColorValue)
newBytes = len(newColorValue)
if oldBytes > newBytes:
element.setAttribute(attr, newColorValue)
numBytes += (oldBytes - len(element.getAttribute(attr)))
Notably, the if oldBytes > newBytes:
which should be rewritten as if oldBytes >= newBytes and oldColorValue != newColorValue:
Yep, you're right, as long as we convert consistently (even if it's not shorter but the same length) we should be fine optimization-wise. We should probably add a few testcases for these border cases as well...
Remaining questions I'm mulling about:
--disable-simplify-colors
. You already adjusted the description, but I figure the name is not really suitable anymore. Maybe we should consider renaming it (keeping the old name as fall-back as we did for other preferences), e.g. --disable-shorten-colors
?Ok, I have:
--disable-shorten-colors
as an alias of --disable-simplify-colors
As for mixing color codes with color names - scour is here to write short svg files, not beautiful ones. :)
Also, if your beef with this is about consistency, then we are trading one consistency (form) for another (shortest length). :smile:
As someone who independently developed a rough version of this, I'd find this functionality useful.
Could a Scour maintainer chime in to note what could move this PR forward?
Improve the code for rewriting colors into recognising that some colors are shorter by name. This commit enables scour to rewrite
rgb(255, 0, 0)
intored
which is slightly shorter than#f00
(ditto fortan
andpink
).When the color name ties in length with the hexcode, then scour will leave it as-is if the input file used a variant of same length (e.g.
blue
,cyan
andaqua
will be left as-is). But if scour is rewriting the color code, it will prefer the hex code variant.Signed-off-by: Niels Thykier niels@thykier.net