Closed
Bug 1297031
Opened 8 years ago
Closed 8 years ago
TEST-UNEXPECTED-FAIL in mask-composite-2c.html while CSS masking is enabled
Categories
(Core :: Layout, defect)
Core
Layout
Tracking
()
RESOLVED
FIXED
mozilla51
Tracking | Status | |
---|---|---|
firefox51 | --- | fixed |
People
(Reporter: bmo, Assigned: ethlin)
References
Details
Attachments
(1 file, 3 obsolete files)
(deleted),
patch
|
Details | Diff | Splinter Review |
TRY: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a730d5de4528&selectedJob=26088958
REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/test/build/tests/reftest/tests/layout/reftests/w3c-css/submitted/masking/mask-composite-2c.html == file:///builds/slave/test/build/tests/reftest/tests/layout/reftests/w3c-css/submitted/masking/mask-composite-2-ref.html | image comparison, max difference: 255, number of differing pixels: 10000
Regressed by:
https://hg.mozilla.org/integration/mozilla-inbound/rev/91667ea2a01e
Reporter | ||
Updated•8 years ago
|
Reporter | ||
Updated•8 years ago
|
Assignee | ||
Comment 1•8 years ago
|
||
Skia's drawBitmap() uses different way(drawBitmapAsMask) to handle kAlpha_8_SkColorType and that causes wrong blending result. So I force the 'AutoPaintSetup' use group method while the color type is kAlpha_8_SkColorType.
Attachment #8784307 -
Flags: review?(jmuizelaar)
Updated•8 years ago
|
Attachment #8784307 -
Flags: review?(jmuizelaar) → review?(lsalzman)
Comment 2•8 years ago
|
||
Comment on attachment 8784307 [details] [diff] [review]
force use group for Alpha type
Review of attachment 8784307 [details] [diff] [review]:
-----------------------------------------------------------------
The basic idea of this patch is fine given the constraints that xfermodes with A8 bitmaps just don't work at all in Skia.
Mainly I just have some nitpicking that can be easily fixed before we land this:
This check: bitmap.colorType() == kAlpha_8_SkColorType
That currently applies to all xfermodes, but the default interpretation of mask blitting in Skia is OP_OVER. So in the very common case of OP_OVER, we don't need to force a group, as Skia will do the right thing.
So this check would be better: bitmap.colorType() == kAlpha_8_SkColorType && aOptions.mCompositionOp != CompositionOp::OP_OVER
That way we don't pay a speed hit for the common case unnecessarily.
Other nitpick: forceNeedGroup/aForceNeedGroup is awkward wording. I would prefer the simpler forceGroup/aForceGroup.
Attachment #8784307 -
Flags: review?(lsalzman) → review-
Assignee | ||
Comment 3•8 years ago
|
||
Thanks for the review. I addressed the above comments to this patch.
Attachment #8784307 -
Attachment is obsolete: true
Attachment #8784655 -
Flags: review?(lsalzman)
Updated•8 years ago
|
Attachment #8784655 -
Flags: review?(lsalzman) → review+
Assignee | ||
Comment 4•8 years ago
|
||
Change to r=lsalzman.
Attachment #8784655 -
Attachment is obsolete: true
Assignee | ||
Comment 5•8 years ago
|
||
Assignee | ||
Comment 6•8 years ago
|
||
Rebase to inbound.
Attachment #8784697 -
Attachment is obsolete: true
https://hg.mozilla.org/integration/mozilla-inbound/rev/e78454e242072bbcada626874f2900bbe1c3e647
Bug 1297031 - Force to use group in DrawTargetSkia::DrawSurface when colorType is kAlpha_8_SkColorType. r=lsalzman
Comment 8•8 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox51:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in
before you can comment on or make changes to this bug.
Description
•