Bug 77912 - [CG] Remove old CG specific shadow setting code from canvas
Summary: [CG] Remove old CG specific shadow setting code from canvas
Alias: None
Product: WebKit
Classification: Unclassified
Component: Canvas (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Matthew Delaney
Depends on:
Reported: 2012-02-06 16:24 PST by Matthew Delaney
Modified: 2012-02-07 13:53 PST (History)
3 users (show)

See Also:

Patch (1.53 KB, patch)
2012-02-06 17:13 PST, Matthew Delaney
no flags Details | Formatted Diff | Diff
Patch (3.05 KB, patch)
2012-02-07 10:08 PST, Matthew Delaney
simon.fraser: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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]
Comment 2 Simon Fraser (smfr) 2012-02-07 00:42:46 PST
Comment on attachment 125734 [details]

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]
Comment 4 Simon Fraser (smfr) 2012-02-07 12:12:12 PST
Comment on attachment 125868 [details]

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>