WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
213403
[CG] REGRESSION (
r256892
): Luminance SVG mask is not applied when accelerated drawing is enabled
https://bugs.webkit.org/show_bug.cgi?id=213403
Summary
[CG] REGRESSION (r256892): Luminance SVG mask is not applied when accelerated...
Said Abou-Hallawa
Reported
2020-06-19 13:30:30 PDT
Created
attachment 402317
[details]
test case Open the attached test case. Result: A 200x200 green rectangle Expected: A 200x200 green rectangle with a 100x100 white hole in the middle.
Attachments
test case
(490 bytes, text/html)
2020-06-19 13:30 PDT
,
Said Abou-Hallawa
no flags
Details
Patch
(3.52 KB, patch)
2020-06-19 13:41 PDT
,
Said Abou-Hallawa
no flags
Details
Formatted Diff
Diff
Patch
(3.48 KB, patch)
2020-06-19 14:17 PDT
,
Said Abou-Hallawa
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Said Abou-Hallawa
Comment 1
2020-06-19 13:34:21 PDT
This is a regression of
r256892
. Before this change void ImageBuffer::genericConvertToLuminanceMask() { auto srcPixelArray = getUnmultipliedImageData(luminanceRect); ... } RefPtr<Uint8ClampedArray> ImageBuffer::getUnmultipliedImageData(const IntRect& rect, IntSize* pixelArrayDimensions, CoordinateSystem coordinateSystem) const { if (context().isAcceleratedContext()) flushContext(); } After this change: void ConcreteImageBuffer::convertToLuminanceMask() override { if (auto* backend = ensureBackendCreated()) { flushDrawingContext(); backend->convertToLuminanceMask(); } } And ImageBufferBackend::convertToLuminanceMask() does not do any flushing. Notice flushDrawingContext() flushes only the drawing items in the display list if they exist. If the ImageBuffer is backed by an IOSurface, convertToLuminanceMask() will act on non up-to-date backend. So we need to call ConcreteImageBuffer::flushContext() instead.
Said Abou-Hallawa
Comment 2
2020-06-19 13:38:52 PDT
<
rdar://problem/64489419
>
Said Abou-Hallawa
Comment 3
2020-06-19 13:41:47 PDT
Created
attachment 402318
[details]
Patch
Darin Adler
Comment 4
2020-06-19 14:00:54 PDT
Comment on
attachment 402318
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=402318&action=review
> Source/WebCore/platform/graphics/ConcreteImageBuffer.h:183 > + const_cast<ConcreteImageBuffer&>(*this).flushContext();
This is not a const member function — why is the const_cast needed?
Simon Fraser (smfr)
Comment 5
2020-06-19 14:10:06 PDT
Comment on
attachment 402318
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=402318&action=review
> Source/WebCore/platform/graphics/ConcreteImageBuffer.h:184 > backend->convertToLuminanceMask();
Why doesn't the backend flush inside convertToLuminanceMask()?
Said Abou-Hallawa
Comment 6
2020-06-19 14:17:09 PDT
Created
attachment 402324
[details]
Patch
Said Abou-Hallawa
Comment 7
2020-06-19 14:36:42 PDT
Comment on
attachment 402318
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=402318&action=review
>> Source/WebCore/platform/graphics/ConcreteImageBuffer.h:183 >> + const_cast<ConcreteImageBuffer&>(*this).flushContext(); > > This is not a const member function — why is the const_cast needed?
Copy/paste mistake. Fixed.
>> Source/WebCore/platform/graphics/ConcreteImageBuffer.h:184 >> backend->convertToLuminanceMask(); > > Why doesn't the backend flush inside convertToLuminanceMask()?
ConcreteImageBuffer::flushContext() may call DisplayList::ImageBuffer::flushDrawingContext() which replays back the display items before calling ImageBufferBackend::flushContext(). If I move the call to flushContext() to ImageBufferBackend::convertToLuminanceMask(), replaying the display items back will not be possible.
EWS
Comment 8
2020-06-19 17:00:06 PDT
Committed
r263297
: <
https://trac.webkit.org/changeset/263297
> All reviewed patches have been landed. Closing bug and clearing flags on
attachment 402324
[details]
.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug