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

re-rasterize fast text shadows directly with the given shadow color #2393

Merged
merged 3 commits into from Feb 8, 2018

Conversation

@lsalzman
Copy link
Contributor

lsalzman commented Feb 7, 2018

It seems that in the wild people are expecting unblurred text shadows to be precisely the same as if the text is drawn with the shadow color. So long as we have to deal with subpixel anti-aliasing and preblending, it is not really possible to reuse the subpixel AA masks and satisfy those constraints. Thus, we have to go back to the old method of just re-rasterizing the text shadow AA masks with the font color being exactly the shadow color, or the preblending just gets messed up.

This fixes issue #2384 and also the downstream Gecko bug https://bugzilla.mozilla.org/show_bug.cgi?id=1434029


This change is Reviewable

@Gankra
Copy link
Contributor

Gankra commented Feb 7, 2018

Just checking, this manages to do something special to color glyphs? (Emoji)

@Gankra
Copy link
Contributor

Gankra commented Feb 7, 2018

Also could you make shadow-atomic strict again?

@lsalzman
Copy link
Contributor Author

lsalzman commented Feb 7, 2018

This is why we still need to keep the shadow flag around on TextRunPrimitiveCpu, essentially to disambiguate that one remaining case. Otherwise, we wouldn't need it.

@glennw
Copy link
Member

glennw commented Feb 7, 2018

@lsalzman shadow-ordering is failing on mac only, for some reason. It was previously running on linux only, so it's probably fine to re-enable that as a platform specific test.

@lsalzman
Copy link
Contributor Author

lsalzman commented Feb 8, 2018

Okay, back to linux-only for now then.

@glennw
Copy link
Member

glennw commented Feb 8, 2018

@bors-servo
Copy link
Contributor

bors-servo commented Feb 8, 2018

📌 Commit e67c642 has been approved by glennw

@bors-servo
Copy link
Contributor

bors-servo commented Feb 8, 2018

Testing commit e67c642 with merge b777142...

bors-servo added a commit that referenced this pull request Feb 8, 2018
re-rasterize fast text shadows directly with the given shadow color

It seems that in the wild people are expecting unblurred text shadows to be precisely the same as if the text is drawn with the shadow color. So long as we have to deal with subpixel anti-aliasing and preblending, it is not really possible to reuse the subpixel AA masks and satisfy those constraints. Thus, we have to go back to the old method of just re-rasterizing the text shadow AA masks with the font color being exactly the shadow color, or the preblending just gets messed up.

This fixes issue #2384 and also the downstream Gecko bug https://bugzilla.mozilla.org/show_bug.cgi?id=1434029

<!-- 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/2393)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Feb 8, 2018

☀️ Test successful - status-appveyor, status-taskcluster, status-travis
Approved by: glennw
Pushing b777142 to master...

@bors-servo bors-servo merged commit e67c642 into servo:master Feb 8, 2018
3 checks passed
3 checks passed
Taskcluster (pull_request) TaskGroup: success
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.