Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add support for blurred rounded rectangle. #665
Add support for blurred rounded rectangle. #665
Changes from 7 commits
5740fec
242a4c9
cf50b87
62c1003
7b766cc
af8057a
a9cf8e3
a1e508b
3df93a7
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not because I expect you to do this, but because it's important that it's raised, but is it at all plausible for the corner radii to be different? I.e. make this function accept a
RoundedRect
- which despite the docs, can have different radii on each corner since linebender/kurbo#166I don't have a good understanding of the underlying maths to make this work
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about this as well but seems tricky at least.
If it would be dependent on the (continuous) distance alone it would be fine in my opinion but the parameters (e.g squircle shape) derived from the corner radius would introduce dicontinuities at the quadrant boundaries, probably visible at larger filter lengths. We could derive shared parameters for the whole shape with some tradeoff in quality but without larger adjustments to the approach.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah - that's fine. I've not thought about it deeply, and I don't think that it is a necessary feature.
Thought it would be important to at least ensure that it is discussed, as much for when Raph comes through to gut-check this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW, this definitely is a necessary feature for full CSS box-shadow support. Not sure how common they are in the wild, but the spec certainly allows authors to specify such shadows.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should be able to emulate it with multiple rounded rectangles, each in their own clip layer. That means that you would be responsible for the behaviour in the degerenate cases. I don't think these clip layers would need to be nested.
I think it's possible you would get conflation artifacts, though...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/DioxusLabs/blitz/blob/main/packages/blitz/src/renderer/multicolor_rounded_rect.rs
we implemented the maths here for rounded-rect, you might be able to find some of it helpful
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just rounded rect can be done without too much difficulty in a distance field context by, basically, setting the radius by quadrant. But in the blurred case you will definitely see discontinuities depending on the exact parameters. I think it is possible to fix, but will require some doing.
Of course another possibility is to render the rounded rect as a vector shape (as in the code Jon linked) and blur it, but (a) that's going to be slower than doing it in a shader, even with heavier math, and (b) we don't have the infrastructure in place. It would be nice to have though, as you do need that for text shadows.
I don't recommend using clips for this, it adds complexity and possible performance issues, as well as potential conflation artifacts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I quickly tested how the artifacts look like and there are indeed visible discontinuities at larger kernel sizes (see figure below). It seems to be primarily caused by the
scale
factor. Using a fixed or interpolated value would be an option here. But I would postpone it for now as followup.