[Skia] Implement support for shadows that ignore transforms #25178
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
GeorgesStavracas
added
the
Layout and Rendering
|
EWS run on previous version of this PR (hash a63db86)
|
GeorgesStavracas
force-pushed
the
eng/Skia-Implement-support-for-untransformed-shadows
branch
from
a63db86
to
bc9d51d
Compare
GeorgesStavracas
added
WPE WebKit
|
EWS run on previous version of this PR (hash bc9d51d)
|
nikolaszimmermann
approved these changes
Feb 29, 2024
|
|
||
| // Ignoring the CTM is practically equal as applying the inverse of | ||
| // the CTM when post-processing the drop shadow. | ||
| if (const std::optional<SkMatrix>& inverse = ctm.inverse()) { |
There was a problem hiding this comment.
Any particular reason for converting directly to SkMatrix? IMHO AffineTransform is more readable.
There was a problem hiding this comment.
It's because the methods mapHomogeneousPoints and mapRadius only exist in SkMatrix, unless I've missed something?
| // the CTM when post-processing the drop shadow. | ||
| if (const std::optional<SkMatrix>& inverse = ctm.inverse()) { | ||
| SkPoint3 p = SkPoint3::Make(offset.width(), offset.height(), 0); | ||
| inverse->mapHomogeneousPoints(&p, &p, 1); |
There was a problem hiding this comment.
Makes sense! You need to transform all lengths to be in identity space - as you intent to remove the CTM.
carlosgcampos
approved these changes
Feb 29, 2024
GeorgesStavracas
force-pushed
the
eng/Skia-Implement-support-for-untransformed-shadows
branch
from
bc9d51d
to
e124da8
Compare
|
EWS run on current version of this PR (hash e124da8)
|
carlosgcampos
added
the
unsafe-merge-queue
https://bugs.webkit.org/show_bug.cgi?id=270175 Reviewed by Carlos Garcia Campos and Nikolas Zimmermann. When creating the drop shadow filter, if state.shadowsIgnoreTransforms() is true, render the drop shadow with trasformed coordinates and blur radius, by adjusting them against the CTM inverse. This is enough to make various canvas tests happy. Some of them are still failing with off-by-one colors, and it's not clear why. * Source/WebCore/platform/graphics/skia/GraphicsContextSkia.cpp: (WebCore::GraphicsContextSkia::createDropShadowFilterIfNeeded const): Canonical link: https://commits.webkit.org/275492@main
webkit-commit-queue
force-pushed
the
eng/Skia-Implement-support-for-untransformed-shadows
branch
from
e124da8
to
caf3994
Compare
|
Committed 275492@main (caf3994): https://commits.webkit.org/275492@main Reviewed commits have been landed. Closing PR #25178 and removing active labels. |
webkit-commit-queue
removed
the
unsafe-merge-queue
GeorgesStavracas
deleted the
eng/Skia-Implement-support-for-untransformed-shadows
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
caf3994
e124da8
🧪 wpe-wk2🧪 ios-wk2🧪 api-wpe🧪 ios-wk2-wpt🧪 gtk-wk2🛠 tv🧪 api-gtk🛠 tv-sim🛠 watch🛠 watch-sim