Bug 77912

Summary: [CG] Remove old CG specific shadow setting code from canvas
Product: WebKit Reporter: Matthew Delaney <mdelaney7>
Component: CanvasAssignee: Matthew Delaney <mdelaney7>
Status: RESOLVED FIXED    
Severity: Normal CC: cmarrin, mdelaney7, mitz
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch simon.fraser: review+

Description Matthew Delaney 2012-02-06 16:24:08 PST
CanvasRenderingContext2D has an old setShadows method that sets CMYKa shadows for CG in a hardcoded manner. This hardcoded path is useless now since it'll get overwritten on the CG context via the normal shadow setting route and thus it should be ripped out.

This function is currently hit by two canvas layout tests which both still pass after removing the hardcoded CG-specific path. (fast/canvas/shadow-offset-7.html AND fast/canvas/canvas-overloads-setShadow.html).
Comment 1 Matthew Delaney 2012-02-06 17:13:31 PST
Created attachment 125734 [details]
Patch
Comment 2 Simon Fraser (smfr) 2012-02-07 00:42:46 PST
Comment on attachment 125734 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=125734&action=review

> Source/WebCore/ChangeLog:4
> +        Removing old CG shadow code.

This needs to be a bit more detailed. Why was the old code added? Why is it OK to remove it now?
Comment 3 Matthew Delaney 2012-02-07 10:08:36 PST
Created attachment 125868 [details]
Patch
Comment 4 Simon Fraser (smfr) 2012-02-07 12:12:12 PST
Comment on attachment 125868 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=125868&action=review

> Source/WebCore/ChangeLog:8
> +        The CG specific shadow offset hack was added in http://trac.webkit.org/changeset/34317
> +        but has since moved down into GraphicsContextCG::setLegacyShadow that is called
> +        by applyShadow() thus the shadow hack is redundant here if we call applyShadow()

It's very hard to parse this run-on sentence.

> Source/WebCore/ChangeLog:12
> +        The CG specific shadow setting block of code is redundant since any setting we
> +        do here to the CGContext will get overwritten later by any subsequent shadow setting
> +        or unsetting since the values in the State object are what's push down into CG.

And this one.
Comment 5 Matthew Delaney 2012-02-07 13:53:03 PST
Committed r106988: <http://trac.webkit.org/changeset/106988>