Skip to content

Commit 278bcb0

Browse files
mihaelatdumitrumagomez
authored andcommitted
Don't use m_shader cache and create a new one for each draw https://bugs.webkit.org/show_bug.cgi?id=298220
Reviewed by Carlos Garcia Campos. The usage of `m_shader` doesn't seem to be safe. It seems to me that a Gradient might be used by different threads. They both can check that the shader is not set and then try to create it. The first one creates it and then tries to use it and the second one will then create a new one and free the first one. I could see two threads that use `WebCore::Gradient::fill()` which ended up in `SkShaderBase::makeContext()` crash. This commit removes the `m_shader` cache and always creates a new shader for each draw. This is probably more expensive, but maybe not that much. If I remove the use of the shader cache, the crash goes away. * Source/WebCore/platform/graphics/Gradient.h: * Source/WebCore/platform/graphics/skia/GradientSkia.cpp: (Gradient::shader): Modified Canonical link: https://commits.webkit.org/299797@main
1 parent d330a5d commit 278bcb0

File tree

2 files changed

+2
-10
lines changed

2 files changed

+2
-10
lines changed

Source/WebCore/platform/graphics/Gradient.h

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -130,10 +130,6 @@ class Gradient : public RenderingResource {
130130
std::optional<GradientRendererCG> m_platformRenderer;
131131
#endif
132132

133-
#if USE(SKIA)
134-
sk_sp<SkShader> m_shader;
135-
#endif
136-
137133
};
138134

139135
WEBCORE_EXPORT WTF::TextStream& operator<<(WTF::TextStream&, const Gradient&);

Source/WebCore/platform/graphics/skia/GradientSkia.cpp

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@ namespace WebCore {
4040

4141
void Gradient::stopsChanged()
4242
{
43-
m_shader = { };
4443
}
4544

4645
inline SkScalar webCoreDoubleToSkScalar(double d)
@@ -101,9 +100,6 @@ static SkGradientShader::Interpolation toSkiaInterpolation(const ColorInterpolat
101100

102101
sk_sp<SkShader> Gradient::shader(float globalAlpha, const AffineTransform& gradientSpaceTransform)
103102
{
104-
if (m_shader)
105-
return m_shader;
106-
107103
Vector<SkColor4f, 8> colors;
108104
colors.reserveInitialCapacity(stops().size());
109105
Vector<SkScalar, 8> positions;
@@ -145,7 +141,7 @@ sk_sp<SkShader> Gradient::shader(float globalAlpha, const AffineTransform& gradi
145141
auto interpolation = toSkiaInterpolation(colorInterpolationMethod());
146142
SkMatrix matrix = gradientSpaceTransform;
147143

148-
m_shader = WTF::switchOn(
144+
auto shader = WTF::switchOn(
149145
m_data,
150146
[&](const LinearData& data) {
151147
SkPoint points[] = { SkPoint::Make(data.point0.x(), data.point0.y()), SkPoint::Make(data.point1.x(), data.point1.y()) };
@@ -170,7 +166,7 @@ sk_sp<SkShader> Gradient::shader(float globalAlpha, const AffineTransform& gradi
170166
return SkGradientShader::MakeSweep(data.point0.x(), data.point0.y(), colors.data(), nullptr, positions.data(), colors.size(), tileMode, 0, 360, interpolation, &matrix);
171167
});
172168

173-
return m_shader;
169+
return shader;
174170
}
175171

176172
void Gradient::fill(GraphicsContext& context, const FloatRect& rect)

0 commit comments

Comments
 (0)