Bug 81486

Summary: [BlackBerry] Use GraphicsContext::fillPath() and strokePath instead of drawPath() in RenderThemeBlackBerry
Product: WebKit Reporter: Leo Yang <leo.yang>
Component: WebKit BlackBerryAssignee: Leo Yang <leo.yang>
Status: RESOLVED FIXED    
Severity: Normal CC: charles.wei, robin.webkit, rwlbuis, staikos, tonikitoo, webkit.review.bot, zimmermann
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 73144    
Attachments:
Description Flags
Patch
rwlbuis: review-
Patch v2 none

Leo Yang
Reported 2012-03-18 20:52:53 PDT
RenderThemeBlackBerry.cpp is now using GraphicsContext::drawPath().
Attachments
Patch (4.05 KB, patch)
2012-03-18 21:03 PDT, Leo Yang 		rwlbuis: review-
DetailsFormatted DiffDiff
Patch v2 (4.63 KB, patch)
2012-03-31 20:04 PDT, Leo Yang 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Leo Yang
Comment 1 2012-03-18 21:03:03 PDT
Created attachment 132537 [details] Patch
Rob Buis
Comment 2 2012-03-19 07:02:06 PDT
Comment on attachment 132537 [details] Patch We should work on getting rid of using drawPath, it can be replaced by using fillPath/strokePath. This means RenderThemeBlackBerry has to be adapted to use those instead.
Leo Yang
Comment 3 2012-03-19 22:52:48 PDT
(In reply to comment #2) > (From update of attachment 132537 [details]) > We should work on getting rid of using drawPath, it can be replaced by using fillPath/strokePath. This means RenderThemeBlackBerry has to be adapted to use those instead. If we replace drawPath with fillPath and strokePath, there is some overhead: 826 if (paintingDisabled()) 827 return; 828 829 SkPath path = *pathToFill.platformPath(); 830 if (!isPathSkiaSafe(getCTM(), path)) 831 return; 832 833 const GraphicsContextState& state = m_state; 834 path.setFillType(state.fillRule == RULE_EVENODD ? 835 SkPath::kEvenOdd_FillType : SkPath::kWinding_FillType); This block of code will be unnecessarily called twice.
Leo Yang
Comment 4 2012-03-26 20:08:27 PDT
(In reply to comment #3) > (In reply to comment #2) > > (From update of attachment 132537 [details] [details]) > > We should work on getting rid of using drawPath, it can be replaced by using fillPath/strokePath. This means RenderThemeBlackBerry has to be adapted to use those instead. > > If we replace drawPath with fillPath and strokePath, there is some overhead: > > 826 if (paintingDisabled()) > 827 return; > 828 > 829 SkPath path = *pathToFill.platformPath(); > 830 if (!isPathSkiaSafe(getCTM(), path)) > 831 return; > 832 > 833 const GraphicsContextState& state = m_state; > 834 path.setFillType(state.fillRule == RULE_EVENODD ? > 835 SkPath::kEvenOdd_FillType : SkPath::kWinding_FillType); > > This block of code will be unnecessarily called twice. @rob: what do you think? I can move drawPath implementation to a separated new file or RenderThemeBlackBerry if you agree.
Leo Yang
Comment 5 2012-03-31 19:54:03 PDT
Change bug title
Leo Yang
Comment 6 2012-03-31 20:04:41 PDT
Created attachment 134985 [details] Patch v2 Substituting drawPath() by fillPath() and strokePath()
Rob Buis
Comment 7 2012-04-01 05:57:00 PDT
Comment on attachment 134985 [details] Patch v2 Looks fine.
Leo Yang
Comment 8 2012-04-01 06:03:37 PDT
Thanks for your review.
WebKit Review Bot
Comment 9 2012-04-01 06:40:49 PDT
Comment on attachment 134985 [details] Patch v2 Clearing flags on attachment: 134985 Committed r112814: <http://trac.webkit.org/changeset/112814>
WebKit Review Bot
Comment 10 2012-04-01 06:40:54 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.