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

Text AA reftests failing on some linux configurations #1941

Closed
kvark opened this issue Oct 25, 2017 · 12 comments
Closed

Text AA reftests failing on some linux configurations #1941

kvark opened this issue Oct 25, 2017 · 12 comments
Labels

Comments

@kvark
Copy link
Member

@kvark kvark commented Oct 25, 2017

Reftests introduced by #1855 fail on my Fedora-26 machine:

Linux carbon 4.13.4-200.fc26.x86_64 #1 SMP

Reftest log: wr-text-aa.zip
cc @glennw

@kvark kvark added the type: bug label Oct 25, 2017
@glennw
Copy link
Member

@glennw glennw commented Oct 25, 2017

Could you check what FreeType version you have?

@kvark
Copy link
Member Author

@kvark kvark commented Oct 25, 2017

2.7.1-9

@glennw
Copy link
Member

@glennw glennw commented Mar 16, 2018

I investigated this briefly today, as it's occurring on my Ubuntu 17.10 machine too. I confirmed that Freetype is 2.8.0 on my machine, and 2.6.5 on the build machines.

My guess is it's related to https://www.freetype.org/freetype2/docs/subpixel-hinting.html, but I tried to disable that and it didn't seem to fix the problem. There might be something additional needed, or I may not have disabled it correctly.

@glennw
Copy link
Member

@glennw glennw commented Apr 6, 2018

We've now switched off travis builds completely, meaning the TC machines are the only ones running CI tests.

I propose that we update that system image to a more recent version of Freetype, possibly by just updating the distro, if that's easiest.

We'd need to update the reference images, but it should be much simpler going forward (right now, I have an Ubuntu 16.04 VM that I use to generate reference images when needed).

Thoughts @staktrace @kvark ?

@glennw
Copy link
Member

@glennw glennw commented Apr 6, 2018

Would that be reasonably simple to do?

@staktrace
Copy link
Contributor

@staktrace staktrace commented Apr 6, 2018

I have no objections. It shouldn't be too hard to do either. We just need to generate a new docker image with the desired version of freetype. The steps involved would be:

  1. Copy the Dockerfile that's pasted in .taskcluster.yml and make changes to install whatever version of freetype you want
  2. Build the docker image from the Dockerfile and publish it to the docker hub
  3. Modify .taskcluster.yml to use that docker image (instead of staktrace/webrender-test:latest)
  4. Push the commit to github and let taskcluster run the CI job using the new image. See what fails
  5. Update reference images for the failures and add them to your commit
  6. Make PR with updated .taskcluster.yml and reference images

I'm happy to do this since I already have docker installed and an account on docker hub where I can push images. However I need to know which version of freetype you want and where to get it. Upgrading the distro from 16.04 to a newer version is possible but carries more risk, so I'd prefer to just add an apt source or download a tarball from somewhere to install a new freetype.

@glennw
Copy link
Member

@glennw glennw commented Apr 8, 2018

If you have the time to do the update, that would be great! Otherwise, I can look into it later this week.

I'd suggest we update to FreeType 2.8. This looks like it has a PPA for Ubuntu with that version - http://ubuntuhandbook.org/index.php/2017/06/install-freetype-2-8-in-ubuntu-16-04-17-04/.

@glennw
Copy link
Member

@glennw glennw commented Apr 18, 2018

@staktrace Did you have a chance to look into whether the PPA above is suitable for what we need?

@staktrace
Copy link
Contributor

@staktrace staktrace commented Apr 19, 2018

@glennw Ah sorry I missed your previous message with the versions you wanted. I'll give this a shot today or tomorrow.

@staktrace
Copy link
Contributor

@staktrace staktrace commented Apr 19, 2018

@glennw I made a new docker image which installs the freetype from the PPA as per the instructions you linked to. I'm not really sure how to confirm that the new freetype is actually being used, but when I run WR CI on it, there are 6 reftest failures, so I assume that's working as intended.

The six reftest failures can be seen at https://tools.taskcluster.net/groups/R0_3fs2mRqWtAsDiKq6KAQ/tasks/R0_3fs2mRqWtAsDiKq6KAQ/runs/0/logs/public%2Flogs%2Flive.log

Reftests with unexpected results:
	reftests/text/alpha-transform.yaml == reftests/text/alpha-transform.png
	reftests/text/subpixel-rotate.yaml == reftests/text/subpixel-rotate.png
	reftests/text/subpixel-skew.yaml == reftests/text/subpixel-skew.png
	reftests/text/blurred-shadow-local-clip-rect.yaml == reftests/text/blurred-shadow-local-clip-rect-ref.png
	reftests/text/two-shadows.yaml == reftests/text/two-shadows.png
	reftests/text/shadow-transforms.yaml == reftests/text/shadow-transforms.png

Can you take a look and make sure these differences are expected? If so I can make a patch that updates .taskcluster.yml and the six reference images.

@glennw
Copy link
Member

@glennw glennw commented Apr 19, 2018

These are the exact 6 tests that fail on my local machine. I verified in the reftest analyzer that they look like the same differences I see visually.

So let's update those references and land it, thanks! 🎉

@glennw
Copy link
Member

@glennw glennw commented Apr 20, 2018

A new test was added today that also falls into this category - raster-space.yaml - so that one would need to be updated too.

bors-servo added a commit that referenced this issue Apr 20, 2018
Switch Linux CI to a docker image with freetype2.8

Fixes #1941

r? @glennw or anybody else

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/2678)
<!-- Reviewable:end -->
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
3 participants
You can’t perform that action at this time.