Created attachment 197510 [details] Patch

Created attachment 197641 [details] Patch

Comment on attachment 197641 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=197641&action=review Is there measurable performance impact for the code paths that now do extra work for non-blend cases? I’m guessing no. What does this do for graphics back ends other than CG? > Source/WebCore/ChangeLog:16 > + (GraphicsContext): Would be nice to remove this bogus change log line and/or fix the script that added it. > LayoutTests/css3/compositing/effect-background-blend-mode-color.html:5 > + if (window.testRunner) { > + window.testRunner.overridePreference("WebKitAcceleratedCompositingEnabled", "1"); > + } Extra braces not needed here. Why test only in this mode? Don’t we want to test both modes?

Created attachment 197724 [details] Patch

Thanks for the review, (In reply to comment #3) > (From update of attachment 197641 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=197641&action=review > > Is there measurable performance impact for the code paths that now do extra work for non-blend cases? I’m guessing no. For non-blend cases, |fillRect| already calls the |setCompositeOperation| method with |BlendModeNormal| and this patch preserves this behaviour, so I'm guessing there shouldn't be a performance impact. > > What does this do for graphics back ends other than CG? These changes eventually imply calling the |setPlatformCompositeOperation| with the appropriate blend mode; for other graphics back ends, it doesn't have any effect yet, I'll add appropriate bugs for each of them, good point. > > > Source/WebCore/ChangeLog:16 > > + (GraphicsContext): > > Would be nice to remove this bogus change log line and/or fix the script that added it. Agree. > > > LayoutTests/css3/compositing/effect-background-blend-mode-color.html:5 > > + if (window.testRunner) { > > + window.testRunner.overridePreference("WebKitAcceleratedCompositingEnabled", "1"); > > + } > > Extra braces not needed here. Why test only in this mode? Don’t we want to test both modes? Agreed, we should test both modes, I've added another test file with accelerated compositing disabled. Also, could you please commit this when you have some time? Thanks

Comment on attachment 197724 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=197724&action=review > LayoutTests/css3/compositing/effect-background-blend-mode-color.html:7 > + if (window.testRunner) > + window.testRunner.overridePreference("WebKitAcceleratedCompositingEnabled", "1"); > + > + if (window.testRunner) > + window.testRunner.dumpAsText(true); Make this one conditional statement. > LayoutTests/css3/compositing/effect-background-blend-mode-color2.html:7 > + if (window.testRunner) > + window.testRunner.overridePreference("WebKitAcceleratedCompositingEnabled", "0"); > + > + if (window.testRunner) > + window.testRunner.dumpAsText(true); Same here.

Comment on attachment 197724 [details] Patch Clearing flags on attachment: 197724 Committed r149010: <http://trac.webkit.org/changeset/149010>

All reviewed patches have been landed. Closing bug.