Bug 81486 - [BlackBerry] Use GraphicsContext::fillPath() and strokePath instead of drawPath() in RenderThemeBlackBerry
: [BlackBerry] Use GraphicsContext::fillPath() and strokePath instead of drawPa...
Status: RESOLVED FIXED
: WebKit
WebKit BlackBerry
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To:
:
:
:
: 73144
  Show dependency treegraph
 
Reported: 2012-03-18 20:52 PST by
Modified: 2012-04-01 06:40 PST (History)


Attachments
Patch (4.05 KB, patch)
2012-03-18 21:03 PST, Leo Yang
rwlbuis: review-
Review Patch | Details | Formatted Diff | Diff
Patch v2 (4.63 KB, patch)
2012-03-31 20:04 PST, Leo Yang
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2012-03-18 20:52:53 PST
RenderThemeBlackBerry.cpp is now using GraphicsContext::drawPath().
------- Comment #1 From 2012-03-18 21:03:03 PST -------
Created an attachment (id=132537) [details]
Patch
------- Comment #2 From 2012-03-19 07:02:06 PST -------
(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.
------- Comment #3 From 2012-03-19 22:52:48 PST -------
(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.
------- Comment #4 From 2012-03-26 20:08:27 PST -------
(In reply to comment #3)
> (In reply to comment #2)
> > (From update of attachment 132537 [details] [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.
------- Comment #5 From 2012-03-31 19:54:03 PST -------
Change bug title
------- Comment #6 From 2012-03-31 20:04:41 PST -------
Created an attachment (id=134985) [details]
Patch v2

Substituting drawPath() by fillPath() and strokePath()
------- Comment #7 From 2012-04-01 05:57:00 PST -------
(From update of attachment 134985 [details])
Looks fine.
------- Comment #8 From 2012-04-01 06:03:37 PST -------
Thanks for your review.
------- Comment #9 From 2012-04-01 06:40:49 PST -------
(From update of attachment 134985 [details])
Clearing flags on attachment: 134985

Committed r112814: <http://trac.webkit.org/changeset/112814>
------- Comment #10 From 2012-04-01 06:40:54 PST -------
All reviewed patches have been landed.  Closing bug.