Summary: | [Skia] Fix shadow behavior for both CSS and Canvas | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mike Reed <reed> | ||||||||
Component: | Layout and Rendering | Assignee: | Mike Reed <reed> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | brettw, commit-queue, eric, kbr, mdelaney7, sam, senorblanco, simon.fraser, webkit.review.bot | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Other | ||||||||||
OS: | OS X 10.5 | ||||||||||
Attachments: |
|
Description
Mike Reed
2011-04-28 08:04:34 PDT
Created attachment 91491 [details]
Patch
Attachment 91491 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/plat..." exit_code: 1
Source/WebCore/ChangeLog:9: Line contains tab character. [whitespace/tab] [5]
Source/WebCore/ChangeLog:10: Line contains tab character. [whitespace/tab] [5]
Source/WebCore/ChangeLog:11: Line contains tab character. [whitespace/tab] [5]
Source/WebCore/ChangeLog:12: Line contains tab character. [whitespace/tab] [5]
Source/WebCore/ChangeLog:13: Line contains tab character. [whitespace/tab] [5]
Source/WebCore/ChangeLog:14: Line contains tab character. [whitespace/tab] [5]
Source/WebCore/ChangeLog:15: Line contains tab character. [whitespace/tab] [5]
Source/WebCore/ChangeLog:16: Line contains tab character. [whitespace/tab] [5]
Source/WebCore/ChangeLog:17: Line contains tab character. [whitespace/tab] [5]
Source/WebCore/ChangeLog:18: Line contains tab character. [whitespace/tab] [5]
Source/WebCore/ChangeLog:19: Line contains tab character. [whitespace/tab] [5]
Source/WebCore/ChangeLog:20: Line contains tab character. [whitespace/tab] [5]
Total errors found: 12 in 5 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 91509 [details]
Patch
Bug title should make it obvious that this is a Skia-only change. Looks fine to me. I'm willing to rubber-stamp this. Comment on attachment 91509 [details]
Patch
Do we need updated test expectations?
Comment on attachment 91509 [details]
Patch
Looks fine to me assuming it's been tested. Do you want to update the bug synopsis and ChangeLog descriptions per Simon's feedback?
test expectations are updated, but only to remove expected failures. Basically this is fixing a previous break. Created attachment 91531 [details]
Patch
Comment on attachment 91531 [details]
Patch
OK.
Comment on attachment 91531 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=91531&action=review > Source/WebCore/platform/graphics/chromium/FontChromiumWin.cpp:-457 > - if (!alpha || windowsCanHandleDrawTextShadow(graphicsContext) || !windowsCanHandleTextDrawingWithoutShadow(graphicsContext)) { Since you're removing the calls to these functions, I think the only remaining caller is windowsCanHandleTextDrawing(). For clarity, we could also revert that function to be like it was pre-r83541, and remove the *Shadow() and *WithoutShadow() flavours. This could be done in a followup patch. > LayoutTests/platform/chromium/test_expectations.txt:2528 > +BUGCR63921 GPU : fast/canvas/canvas-strokePath-alpha-shadow.html = TEXT TIMEOUT Unless I'm missing something, these should remain GPU WIN LINUX. We shouldn't be running the canvas tests on GPU MAC. Comment on attachment 91531 [details] Patch Clearing flags on attachment: 91531 Committed r85264: <http://trac.webkit.org/changeset/85264> All reviewed patches have been landed. Closing bug. |