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

Replace SVGs with equivalent PNGs in the csswg tests #12963

Open
Manishearth opened this issue Aug 21, 2016 · 4 comments
Open

Replace SVGs with equivalent PNGs in the csswg tests #12963

Manishearth opened this issue Aug 21, 2016 · 4 comments
Labels

Comments

@Manishearth
Copy link
Member

@Manishearth Manishearth commented Aug 21, 2016

A lot of the CSS tests use SVGs as references or as objects to be manipulated.

$ git grep "[a-zA-Z]\.svg"| wc -l
    7606

The references won't run in Servo at all, which makes the test useless in Servo. This also can often lead to false positives when the tested thing doesn't display at all (my multiple backgrounds PR had a bug which cased a lot of tests to pass because of this).

I suspect getting SVG to work in Servo will take a lot of time. We should do that eventually, but I'd like working CSS tests before that.

It occurred to me that we could make the wpt harness replace svgs with pngs (via some flag) using some library. Servo can still mimesniff them and display as pngs, and the reftests will work.

This could make actual SVG tests pass by accident, but that's not important till we actually start supporting SVG (at which point we can revert this change).

If we don't want to do this, we could also just manually replace everything with PNGs, though this requires a more careful audit, and there's probably a reason that all the tests use SVG (PNG might lead to fuzziness?)

cc @SimonSapin @jgraham @gsnedders

@gsnedders
Copy link
Contributor

@gsnedders gsnedders commented Aug 21, 2016

Many of the tests that use SVG as objects are specifically testing SVGs being embedded in specific places.

The complexity of replacing SVGs with bitmapped images is going to be colour spaces, I think. Need to have them match,

If there's tests that can easily just have SVGs replaced with images upstream we should just do that.

@frewsxcv frewsxcv added the A-testing label Aug 21, 2016
@Manishearth
Copy link
Member Author

@Manishearth Manishearth commented Aug 22, 2016

specifically testing SVGs being embedded in specific places.

Right, but Servo doesn't care about these right now 😄. Since it's a flag, other browsers can continue using SVGs.

If there's tests that can easily just have SVGs replaced with images upstream we should just do that.

This was what I was originally thinking about, but the tests aren't easy to edit anymore. Converting on-the-fly lets them stay as editable svgs, but converts them for servo.

@nox
Copy link
Member

@nox nox commented Oct 4, 2017

@jgraham @gsnedders Are we going to do this?

@gsnedders
Copy link
Contributor

@gsnedders gsnedders commented Oct 5, 2017

@nox I'm still not opposed to it where SVG isn't being tested, but I'd suggest doing this against upstream once #16175 is resolved.

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

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.