RESOLVED FIXED 23766
CanvasRenderingContext2D::setShadow needs else for other platforms 
https://bugs.webkit.org/show_bug.cgi?id=23766
Summary CanvasRenderingContext2D::setShadow needs else for other platforms
Scott Violet
Reported 2009-02-05 12:42:16 PST
The CMYK variant of CanvasRenderingContext2D::setShadow only updates the GraphicsContext if the platform is GC. There should be an else case for other platforms.
Attachments
v1 of patch (1.68 KB, patch)
2009-02-05 12:47 PST, Scott Violet 		eric: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Scott Violet
Comment 1 2009-02-05 12:47:51 PST
Created attachment 27356 [details] v1 of patch
Darin Adler
Comment 2 2009-02-05 12:51:40 PST
Comment on attachment 27356 [details] v1 of patch I don't like this trend of keeping CG as a special case when making all the other platforms share code. Can we find a way to make the code the same for all the platforms without hurting the rendering quality in CG?
Eric Seidel (no email)
Comment 3 2009-02-05 13:22:20 PST
Comment on attachment 27356 [details] v1 of patch Yeah, this is an OK solution for platforms w/o CYMK support, but it would be bad if used on platforms which did. I feel like this may need a FIXME to note that this should use a smarter color class some day. (We should be able to pass a CYMK color along to graphics context directly w/o having to convert it to RGB here.) The patch looks fine though.
Scott Violet
Comment 4 2009-02-05 13:22:50 PST
I'm not a WebKit expert by any means, so take this with a grain of salt. Could we do the following: . Change Color to wrap either an RGBA or CMYKA type ala a union. . Change rgb() such that if the type is cmyka we use makeRGBAFromCMYKA. . For platforms that support CMYK they could test the type, then convert as necessary.
Eric Seidel (no email)
Comment 5 2009-02-05 13:22:57 PST
Comment on attachment 27356 [details] v1 of patch Actually, never mind about the FIXME. When we fix this, we'll just remove the CG code, and everyone will use the path you just added... we'll just make that path not be lossy like it is today. :)
Dimitri Glazkov (Google)
Comment 6 2009-02-05 15:41:11 PST
Landed as http://trac.webkit.org/changeset/40693.
Note You need to log in before you can comment on or make changes to this bug.