Bug 179290

Summary: Move code that maps a CompositeOperator and BlendMode to a CGBlendMode into a helper function
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: New BugsAssignee: Simon Fraser (smfr) <simon.fraser>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch darin: review+

Simon Fraser (smfr)
Reported 2017-11-04 16:27:21 PDT
Move code that maps a CompositeOperator and BlendMode to a CGBlendMode into a helper function
Attachments
Patch (8.51 KB, patch)
2017-11-04 16:28 PDT, Simon Fraser (smfr) 		no flags
DetailsFormatted DiffDiff
Patch (87.16 KB, patch)
2017-11-04 18:46 PDT, Simon Fraser (smfr) 		no flags
DetailsFormatted DiffDiff
Patch (8.51 KB, patch)
2017-11-04 18:47 PDT, Simon Fraser (smfr) 		darin: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Simon Fraser (smfr)
Comment 1 2017-11-04 16:28:25 PDT
Created attachment 326050 [details] Patch
Simon Fraser (smfr)
Comment 2 2017-11-04 18:46:38 PDT
Created attachment 326056 [details] Patch
Simon Fraser (smfr)
Comment 3 2017-11-04 18:47:26 PDT
Created attachment 326057 [details] Patch
Darin Adler
Comment 4 2017-11-04 19:35:17 PDT
Comment on attachment 326057 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=326057&action=review > Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp:140 > +static CGBlendMode compositeAndBlendToCGBlendMode(CompositeOperator compositeOperator, BlendMode blendMode) Since this is C++ I don’t think we really need to restate what the arguments are in the function name. I think this should be called something like selectCGBlendMode. I think it’s a little strange that it ignores the composite operator when the blend mode is something other than normal. It’s hard to understand why that’s OK. I’m sure there is a good reason. I would love to have a comment explaining it.
Simon Fraser (smfr)
Comment 5 2017-11-04 20:14:13 PDT
(In reply to Darin Adler from comment #4) > Comment on attachment 326057 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=326057&action=review > > > Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp:140 > > +static CGBlendMode compositeAndBlendToCGBlendMode(CompositeOperator compositeOperator, BlendMode blendMode) > > Since this is C++ I don’t think we really need to restate what the arguments > are in the function name. I think this should be called something like > selectCGBlendMode. I'll change it. > I think it’s a little strange that it ignores the composite operator when > the blend mode is something other than normal. It’s hard to understand why > that’s OK. I’m sure there is a good reason. I would love to have a comment > explaining it. Yeah. I'm not sure why we distinguish between CompositeOperator and BlendMode throughout the WebKit code; it seems like some spec purity modeled after https://www.w3.org/TR/compositing-1. CG smooshes them into one enum.
Simon Fraser (smfr)
Comment 6 2017-11-04 22:40:55 PDT
https://trac.webkit.org/changeset/224465/webkit
Radar WebKit Bug Importer
Comment 7 2017-11-15 12:10:54 PST
<rdar://problem/35567092>
Note You need to log in before you can comment on or make changes to this bug.