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

Consider running tests with resvg-js #1656

Open
yisibl opened this issue Feb 22, 2022 · 7 comments
Open

Consider running tests with resvg-js #1656

yisibl opened this issue Feb 22, 2022 · 7 comments

Comments

@yisibl
Copy link
Contributor

yisibl commented Feb 22, 2022

resvg(resvg-js) is pretty much the best supported rendering library for SVG today, and a detailed test suite is available here. If playwright could be removed, this would greatly speed up CI and local testing.

image

@TrySound
Copy link
Member

Cool! Would you like to work this?

@yisibl
Copy link
Contributor Author

yisibl commented Feb 22, 2022

Cool! Would you like to work this?

Can you make a list of what can be adjusted?

@TrySound
Copy link
Member

@yisibl
Copy link
Contributor Author

yisibl commented Feb 26, 2022

My initial look at the test looks like there are quite a few changes needed.
Also, I was unable to successfully download the W3C test file locally due to network reasons.

I'm sorry, I don't have time at the moment, probably until April. If you have some ideas, you can make some adjustments first.

Kreeg pushed a commit to Kreeg/svgo that referenced this issue May 16, 2022
@Kreeg
Copy link
Contributor

Kreeg commented May 16, 2022

@yisibl @TrySound hello! I started to move regression tests to resvg. Now it has 15 mismatched icons, I've uploaded it with pngs and svgs here https://drive.google.com/file/d/1EacnF5KKYRvs5ZTts_SNRM4J5UJ5-fGU/view?usp=sharing
You can check current code (imcoplete at the time) here https://github.com/Kreeg/svgo/tree/move-regression-tests-to-resvg.
I can't tell if this is svgo or resvg issue. For example, color-prop-05-t.svg is a light green color square. But the optimized one is the red square! If you open this file in Inkscape it also green and red, but the chrome and firefox renders this as red colored, safari renders as green.
изображение

изображение

I guess it's resvg issue for the other 14 mismatches. @yisibl can you check that?

@yisibl
Copy link
Contributor Author

yisibl commented May 16, 2022

@Kreeg Thank you for your excellent work!

color-prop-05-t.svg

I think Safari and resvg-js are rendering correctly, and it should be green here. This means that fill='currentColor' should inherit the color set in the color property.

svgo does not apply color values correctly after compression:

<svg baseProfile="tiny" viewBox="0 0 480 360" xmlns="http://www.w3.org/2000/svg">
    ...
    <path color="red" d="M120 60h150v150H120z" fill="currentColor" font-family="SVGFreeSansASCII,sans-serif" font-size="18" />
    ...
</svg>

text-ws-03-t.svg

<tref> Deprecated, It is recommended to remove from the test.
Doc: https://developer.mozilla.org/en-US/docs/Web/SVG/Element/tref

types-basic-02-f.svg

resvg does not currently support CSS units(e.g. 20PX) in uppercase.

text-fonts-202-t.svg & text-fonts-02-t.svg

resvg is not rendered correctly font-weight="lighter"
Doc: https://developer.mozilla.org/en-US/docs/Web/CSS/font-weight#common_weight_name_mapping

struct-dom-07-f.svg

Uses JS to dynamically modify fill to green.

var use = document.getElementById("use-elm");
var firstElementChild = use.instanceRoot.firstChild;

support for instanceRoot was dropped in Chrome and Opera in 2014/15. https://bugs.chromium.org/p/chromium/issues/detail?id=313438

So, this is an obsolete test case and this test case will not work in modern browsers. We should exclude it.

@Kreeg
Copy link
Contributor

Kreeg commented May 18, 2022

Cool! Thanks for the answers @yisibl!
But what about painting-marker and filters-overview svgs?

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
@Kreeg @yisibl @TrySound and others