Summary: | [Skia] Implement GraphicsContext::fillRoundedRect() using SkCanvas::drawRRect() | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Florin Malita <fmalita> | ||||||||||
Component: | Layout and Rendering | Assignee: | Florin Malita <fmalita> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | caryclark, dglazkov, eric.carlson, feature-media-reviews, junov, reed, senorblanco, webkit.review.bot | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Florin Malita
2013-01-08 12:42:43 PST
Created attachment 181747 [details]
Patch
Comment on attachment 181747 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=181747&action=review > Source/WebCore/platform/graphics/skia/GraphicsContextSkia.cpp:833 > + SkVector radii[4]; > + radii[SkRRect::kUpperLeft_Corner] = SkPoint::Make(topLeft.width(), topLeft.height()); > + radii[SkRRect::kUpperRight_Corner] = SkPoint::Make(topRight.width(), topRight.height()); > + radii[SkRRect::kLowerRight_Corner] = SkPoint::Make(bottomRight.width(), bottomRight.height()); > + radii[SkRRect::kLowerLeft_Corner] = SkPoint::Make(bottomLeft.width(), bottomLeft.height()); Looks a little strange to be assigning an SkPoint to an SkVector (even though they're the same thing). Since it's a typedef anyway, could you use SkVector::Make() here instead? Created attachment 181761 [details]
Patch
Thanks Stephen, updated.
How about radii[SkRRect::kUpperLeft_Corner].set(topLeft.width(), topLeft.height()); ... No need to make and assign, when you can set. PS -- if we cared about warnings, we might want to promote width/height to floats (or SkScalars), since they are ints here. Not a request, just a note. Created attachment 181774 [details]
Patch
Updated, thanks Mike.
Comment on attachment 181774 [details]
Patch
Looks good. r=me
Comment on attachment 181774 [details] Patch Attachment 181774 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/15771255 New failing tests: fast/forms/validation-message-appearance.html Created attachment 181902 [details]
Patch for landing
Comment on attachment 181902 [details] Patch for landing Clearing flags on attachment: 181902 Committed r139191: <http://trac.webkit.org/changeset/139191> All reviewed patches have been landed. Closing bug. |