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

[Feature] Add DropShadowPanel Support for Grid & Border content with Translucent Brushes & Round Corners #3607

Closed
seanocali opened this issue Dec 4, 2020 · 9 comments
Labels
Milestone

Comments

@seanocali
Copy link

seanocali commented Dec 4, 2020

Describe the problem this feature would solve

DropShadowPanel only supports obtaining an alpha mask from certain types of content (Image, Shape, TextBlock, and ImageEx). If you try to use content such as a Grid or Border that has either a background brush or border brush with rounded corners, you do not get a matching drop shadow. You can substitute a Rectangle to get rounded corners, however Rectangles don't stretch and must be a fixed size which can introduce UI design difficulties. Also if you want a translucent brush then the reduced opaqueness you set on the content will be applied to the drop shadow. Having semi-opaque content while also having a clearly defined dropshadow is basically not possible. On top of this, if you add an AcrylicBrush it fails and reverts to a fallback solid color.

Describe the solution

I've created an extended control which doesn't affect the existing functionality for Image, Shape, TextBlock, and ImageEx. When the content is a Grid or Border, a matching CompositionRoundedRectangleGeometry is generated. This is used to create a SpriteShape, which is added to a ShapeVisual, which is used to create a CompositionSurfaceBrush to provide the dropshadow mask. The Size, Fill, Stroke, and CornerRadius are mapped and updated accordingly.

The same support can be added for StackPanel, RelativePanel, etc. I'm more than happy to submit a pull request.

Additional context & Screenshots

Here's a screenshot of what my extended version of DropShadowPanel can do that the original cannot:
Screenshot 2020-12-04 142631

@seanocali seanocali added the feature request 📬 A request for new changes to improve functionality label Dec 4, 2020
@ghost
Copy link

ghost commented Dec 4, 2020

Hello, 'seanocali! Thanks for submitting a new feature request. I've automatically added a vote 👍 reaction to help get things started. Other community members can vote to help us prioritize this feature in the future!

@Kyaa-dost
Copy link
Contributor

@seanocali Thanks for the feature request and for taking an initiative in this area. I really like the idea as it seems more reliable but let's see whats other community members have to say about this and if approved then you can proceed to submit the PR 😊

@mdtauk
Copy link

mdtauk commented Dec 7, 2020

It would be good if there was a mode added which would "cutout" the area above the shadow, so the shadow only shows outside of the shape. This would allow a semi-transparent fill to not show the shadowed area below.

It could take the CompositionRoundedRectangleGeometry generated, and use it to mask out the shadow

image

@Kyaa-dost
Copy link
Contributor

Thanks for the input @mdtauk!

@seanocali any insight on the advice above? ⬆️

@seanocali
Copy link
Author

@mdtauk Yeah I like that idea.

@seanocali
Copy link
Author

Also note my solution just takes the Top-Left CornerRadius and applies it to the rectangle's X and Y radii. Having 4 independent corners could be done with a Path instead of a Rectangle but I haven't dove into that yet.

@mdtauk
Copy link

mdtauk commented Dec 8, 2020

Also note my solution just takes the Top-Left CornerRadius and applies it to the rectangle's X and Y radii. Having 4 independent corners could be done with a Path instead of a Rectangle but I haven't dove into that yet.

Is there any idea of what (if any) performance implications there may be implementing it?

Are there ways to optimise DropDownShadows applied to paths, like cloning the data instead of constructing a new shape?

@seanocali
Copy link
Author

I haven't done tests, but it wouldn't surprise me if creating a new shape is less expensive than calling GetAlphaMask() on an element in the visual tree.

Speaking of performance the DropShadowPanel is currently riddled with redundant casting. For example:

 if (Content is Image)
      {
          mask = ((Image)Content).GetAlphaMask();
      }

should be

 if (Content is Image image)
      {
          mask = image.GetAlphaMask();
      }

@michael-hawker
Copy link
Member

FYI @JustinXinLiu @Sergio0694

@michael-hawker michael-hawker added this to the 7.1 milestone Aug 11, 2021
@ghost ghost added the in progress 🚧 label Aug 24, 2021
ghost pushed a commit that referenced this issue Aug 27, 2021
## Fixes #3122 #3607 #3516

_Also implements #3693 for the new DropShadow._

FYI @seanocali as this is a different implementation approach (which is simpler to use outside of the DropShadowPanel we've been working on) but should hopefully achieve the same result.

This PR adds attached shadows which can easily be attached to any FrameworkElement without needing to modify the layout like DropShadowPanel does today. They can also be shared using a resource, added to the style of an element, and animated! All the things! 🎉

## PR Type

What kind of change does this PR introduce?

<!-- Please uncomment one or more options below that apply to this PR. -->

<!-- - Bugfix -->
- Feature by @Ryken100 and integrated/extended by @michael-hawker 
<!-- - Code style update (formatting) -->
<!-- - Refactoring (no functional changes, no api changes) -->
<!-- - Build or CI related changes -->
<!-- - Documentation content changes -->
<!-- - Sample app changes -->
<!-- - Other... Please describe: -->

## What is the current behavior?

DropShadowPanel is clunky and requires modifying how you layout your app.

## What is the new behavior?

Just attach a shadow and be done! (DropShadowPanel is deprecated.)

## PR Checklist

- [x] Composition Only Shadow Support? (with Target)
- [x] Add XML Docs
- [x] Animation Support to Explicit Animation System?
- [x] Bug can't use `AttachedCardShadow` directly with `Border`?

Please check if your PR fulfills the following requirements: <!-- and remove the ones that are not applicable to the current PR -->

- [ ] Tested code with current [supported SDKs](../#supported)
- [ ] Pull Request has been submitted to the documentation repository [instructions](../blob/main/Contributing.md#docs). Link: <!-- docs PR link -->
- [ ] Sample in sample app has been added / updated (for bug fixes / features)
  - [ ] Icon has been created (if new sample) following the [Thumbnail Style Guide and templates](https://github.com/CommunityToolkit/WindowsCommunityToolkit-design-assets)
- [ ] New major technical changes in the toolkit have or will be added to the [Wiki](https://github.com/CommunityToolkit/WindowsCommunityToolkit/wiki) e.g. build changes, source generators, testing infrastructure, sample creation changes, etc...
- [ ] Tests for the changes have been added (for bug fixes / features) (if applicable)
- [ ] Header has been added to all new source files (run _build/UpdateHeaders.bat_)
- [ ] Contains **NO** breaking changes

<!-- If this PR contains a breaking change, please describe the impact and migration path for existing applications below.
Please note that breaking changes are likely to be rejected within minor release cycles or held until major versions. -->

## Other information

<!-- Please add any other information that might be helpful to reviewers. -->
@ghost ghost locked as resolved and limited conversation to collaborators Nov 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants