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

feat: Add baseColor to Shadow3DDecorator #3375

Merged

Conversation

Taormina
Copy link
Contributor

@Taormina Taormina commented Dec 2, 2024

Description

The Shadow3DDecorator is very useful, but the shadow defaults to being pure black. While this is fine in most cases, sometimes a designer might request something a little bit different, so this change to allow the base color of the shadow to be changed if so desired.

Checklist

  • I have followed the Contributor Guide when preparing my PR.
  • I have updated/added tests for ALL new/updated/fixed functionality.
  • I have updated/added relevant documentation in docs and added dartdoc comments with ///.
  • I have updated/added relevant examples in examples or docs.

Breaking Change?

  • Yes, this PR is a breaking change.
  • No, this PR is not a breaking change.

Related Issues

@Taormina
Copy link
Contributor Author

Taormina commented Dec 2, 2024

Screenshot 2024-12-02 at 2 00 49 PM

This screenshot is an example of a white baseColor.

@Taormina Taormina changed the title feat: add baseColor to Shadow3DDecorator Feat: add baseColor to Shadow3DDecorator Dec 2, 2024
@Taormina Taormina changed the title Feat: add baseColor to Shadow3DDecorator feat: Add baseColor to Shadow3DDecorator Dec 2, 2024
@Taormina
Copy link
Contributor Author

Taormina commented Dec 2, 2024

We can probably update the dynamically change shadow properties test as well, I just wasn't sure how to recreate the golden image exactly.

Copy link
Member

@spydon spydon left a comment

Choose a reason for hiding this comment

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

Looks good, would be good with a golden test though like you said.

Paint? _paint;
Paint _makePaint() {
final paint = Paint();
final color = Color.fromRGBO(0, 0, 0, opacity);
final color = baseColor.withAlpha((opacity * 255).toInt());
Copy link
Member

Choose a reason for hiding this comment

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

There is an extension on Color in Flame that does this that can be used instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

baseColor.withValues(alpha: opacity)?

Copy link
Member

Choose a reason for hiding this comment

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

Ah, it wasn't an extension, it was built-in:
https://api.flutter.dev/flutter/dart-ui/Color/withOpacity.html

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Screenshot 2024-12-02 at 3 07 52 PM

looks like withValues is the new withOpacity? but either way, much cleaner

Copy link
Member

Choose a reason for hiding this comment

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

Aah , I see!
Good to know.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@spydon it looks like it might be the new withOpacity, but it might only be on the beta channel right now, so let me get that aimed back at main Flutter channel and plop withOpacity back in.

Copy link
Member

@spydon spydon left a comment

Choose a reason for hiding this comment

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

Lgtm, thanks for your contribution!

@spydon spydon merged commit b5d7ee0 into flame-engine:main Dec 3, 2024
8 checks passed
@Taormina Taormina deleted the feat/base-color-shadow-3d-decorator branch January 7, 2025 01:22
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.

2 participants