Skip to content

Conversation

@Hixie
Copy link
Contributor

@Hixie Hixie commented May 7, 2018

@Hixie Hixie changed the title Factor our common Paint-building code used with BoxShadow Disable shadows when taking goldens May 7, 2018
@Hixie
Copy link
Contributor Author

Hixie commented May 7, 2018

Fixes #17262

@tvolkert
Copy link
Contributor

tvolkert commented May 8, 2018

Bots aren't happy - among the failures is "MergeableMaterial paints shadows", so it seems like some tests need updating.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why * 2?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make the difference more obvious. It was just a guess as to what would look good, I'm happy to change it to 1.0 if you like.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what I was figuring. 2.0 sgtm

Copy link
Contributor

@tvolkert tvolkert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

(modulo existing tests that need updating)

@tvolkert
Copy link
Contributor

tvolkert commented May 8, 2018

Also, see discussion in #17366

@Hixie Hixie force-pushed the shadows branch 3 times, most recently from 76c7485 to 0bdca4d Compare May 8, 2018 20:18
@tvolkert
Copy link
Contributor

LGTM

@tvolkert tvolkert merged commit ca94bfd into flutter:master May 10, 2018
DaveShuckerow pushed a commit to DaveShuckerow/flutter that referenced this pull request May 14, 2018
@Hixie Hixie deleted the shadows branch May 16, 2018 00:43
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants