Bug 179290 - Move code that maps a CompositeOperator and BlendMode to a CGBlendMode into a helper function
Summary: Move code that maps a CompositeOperator and BlendMode to a CGBlendMode into a...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-11-04 16:27 PDT by Simon Fraser (smfr)
Modified: 2017-11-15 12:10 PST (History)
3 users (show)

See Also:


Attachments
Patch (8.51 KB, patch)
2017-11-04 16:28 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Patch (87.16 KB, patch)
2017-11-04 18:46 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Patch (8.51 KB, patch)
2017-11-04 18:47 PDT, Simon Fraser (smfr)
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2017-11-04 16:27:21 PDT
Move code that maps a CompositeOperator and BlendMode to a CGBlendMode into a helper function
Comment 1 Simon Fraser (smfr) 2017-11-04 16:28:25 PDT
Created attachment 326050 [details]
Patch
Comment 2 Simon Fraser (smfr) 2017-11-04 18:46:38 PDT
Created attachment 326056 [details]
Patch
Comment 3 Simon Fraser (smfr) 2017-11-04 18:47:26 PDT
Created attachment 326057 [details]
Patch
Comment 4 Darin Adler 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.
Comment 5 Simon Fraser (smfr) 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.
Comment 6 Simon Fraser (smfr) 2017-11-04 22:40:55 PDT
https://trac.webkit.org/changeset/224465/webkit
Comment 7 Radar WebKit Bug Importer 2017-11-15 12:10:54 PST
<rdar://problem/35567092>