| Summary: | [skia] feConvolveMatrix should use accelerated path | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Stephen White <senorblanco> | ||||||||||||||
| Component: | SVG | Assignee: | Stephen White <senorblanco> | ||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||
| Severity: | Normal | CC: | dino, jamesr, junov, rniwa, webkit.review.bot, zimmermann | ||||||||||||||
| Priority: | P2 | ||||||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||||||
| Hardware: | Unspecified | ||||||||||||||||
| OS: | Unspecified | ||||||||||||||||
| Bug Depends on: | 112906 | ||||||||||||||||
| Bug Blocks: | |||||||||||||||||
| Attachments: |
|
||||||||||||||||
|
Description
Stephen White
2013-03-20 12:44:41 PDT
Created attachment 194099 [details]
Patch
Created attachment 194100 [details]
Fix copyright date
Created attachment 194105 [details]
Patch
James, would you mind taking a look? Comment on attachment 194105 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=194105&action=review Hm, ok. I don't think I understand this very well but seems reasonable. > Source/WebCore/platform/graphics/filters/skia/FEConvolveMatrixSkia.cpp:43 > + case WebCore::EDGEMODE_UNKNOWN: > + case WebCore::EDGEMODE_DUPLICATE: > + default: > + return SkMatrixConvolutionImageFilter::kClamp_TileMode; this looks weird. why not do switch () { case ..._WRAP: return ...; case ..._NONE: return ...; default: return ....; } ? Comment on attachment 194105 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=194105&action=review >> Source/WebCore/platform/graphics/filters/skia/FEConvolveMatrixSkia.cpp:43 >> + return SkMatrixConvolutionImageFilter::kClamp_TileMode; > > this looks weird. why not do > > switch () { > case ..._WRAP: > return ...; > case ..._NONE: > return ...; > default: > return ....; > } > > ? I'd prefer to call out the DUPLICATE edge mode explicitly, since this one is the one that really maps to Clamp. I'll remove the default case entirely, since it shouldn't be necessary. UNKNOWN is a weird one, that's not in the spec (perhaps just there as a placeholder for parsing?) but unfortunately I still seed to do something with it. Created attachment 194228 [details]
Patch for landing
Comment on attachment 194228 [details] Patch for landing Clearing flags on attachment: 194228 Committed r146454: <http://trac.webkit.org/changeset/146454> All reviewed patches have been landed. Closing bug. Re-opened since this is blocked by bug 112906 Created attachment 194245 [details]
Patch for landing
Comment on attachment 194245 [details] Patch for landing Rejecting attachment 194245 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=webkit-commit-queue.appspot.com', '--bot-id=gce-cq-03', 'validate-changelog', '--non-interactive', 194245, '--port=chromium-xvfb']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue /mnt/git/webkit-commit-queue/LayoutTests/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive). Full output: http://webkit-commit-queue.appspot.com/results/17292022 Created attachment 194246 [details]
Patch for landing
Comment on attachment 194246 [details] Patch for landing Clearing flags on attachment: 194246 Committed r146469: <http://trac.webkit.org/changeset/146469> All reviewed patches have been landed. Closing bug. Why was this patch landed without any new expected results? Please rebaseline results for at least one platform in the patch. Otherwise we have no way of knowing what the expected results is. http://trac.webkit.org/changeset/146492 didn't refer to either this bug or http://trac.webkit.org/changeset/146469. |