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

Ensure fills with transparent colors and patterns are rendered as expected #3120

Merged
merged 2 commits into from
Sep 2, 2016

Conversation

lucaswoj
Copy link
Contributor

@lucaswoj lucaswoj commented Aug 31, 2016

This PR fixes #3107 by making FillStyleLayer#isHidden aware of fill-pattern.

Launch Checklist

@lucaswoj
Copy link
Contributor Author

cc @dpieri @jfirebaugh @mourner @mollymerp

@jfirebaugh
Copy link
Contributor

jfirebaugh commented Aug 31, 2016

LGTM, although I question whether isHidden should be checking any properties other than visibility.

@lucaswoj
Copy link
Contributor Author

LGTM, although I question whether isHidden should be checking any properties other than visibility.

That's a fair point. I'd be happy to refactor to make it so. Should we go for it?

@jfirebaugh
Copy link
Contributor

If it doesn't regress tests or benchmarks, I say go for it.

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Sep 1, 2016

The latest commit removes all opacity tests from the StyleLayer#isHidden method. This is unlikely to cause performance regressions in core styles because we don't often have 0 opacity layers in core styles. It's possible a 3rd party style may see a perf regression due to this change.

@lucaswoj lucaswoj merged commit 900abd5 into master Sep 2, 2016
@lucaswoj lucaswoj deleted the fill-transparent-pattern branch September 2, 2016 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

0.23.0 breaks some fill layers that use fill-pattern
2 participants