Removing WindRule.h completely

Removing WindRule.h completely

Moving WindRule enum to GraphicsTypes.h with the same constant value as in skia

BUG=259814
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=179838

[f(malita), 2014-08-05 13:25:03]

Looks good, but we should also update the use sites to stop converting
explicitly and just cast instead. A few places I see:

GraphicsContext::clipPath()
GraphicsContext::canvasClip()
GraphicsContext::fillPath()
Path::windRule()
Path::setWindRule()
Path::contains()

[shreeramk, 2014-08-06 13:37:41]

On 2014/08/05 13:25:03, Florin Malita wrote:
> Looks good, but we should also update the use sites to stop converting
> explicitly and just cast instead. A few places I see:
> 
> GraphicsContext::clipPath()
> GraphicsContext::canvasClip()
> GraphicsContext::fillPath()
> Path::windRule()
> Path::setWindRule()
> Path::contains()

I didn't get you. What needs to be type casted??

[f(malita), 2014-08-06 14:25:45]

On 2014/08/06 13:37:41, shreeram.k wrote:
> On 2014/08/05 13:25:03, Florin Malita wrote:
> > Looks good, but we should also update the use sites to stop converting
> > explicitly and just cast instead. A few places I see:
> > 
> > GraphicsContext::clipPath()
> > GraphicsContext::canvasClip()
> > GraphicsContext::fillPath()
> > Path::windRule()
> > Path::setWindRule()
> > Path::contains()
> 
> I didn't get you. What needs to be type casted??

Those methods implement an explicit conversion WindRule -> SkPath::FillType, for
example  GraphicsContext::clipPath(const Path& pathToClip, WindRule clipRule):

    SkPath::FillType temporaryFillType = clipRule == RULE_EVENODD ?
SkPath::kEvenOdd_FillType : SkPath::kWinding_FillType;

Now that we're redefined WindRule to match FillType, we should be able to
convert directly:

    SkPath::FillType temporaryFillType =
static_cast<SkPath::FillType>(clipRule);

But it would be nice to hide the cast too, so I would add a helper to
SkiaUtils.h:

  inline SkPath::FillType WebCoreWindRuleToSkFillType(WindRule rule)
  {
      return static_cast<SkPath::FillType>(rule);
  }

and then use that in the methods mentioned above.

[shreeramk, 2014-08-07 05:36:57]

@Florin: PTAL.
I made the changes related to type cast. Should we handle hiding the cast in
this revision or in a separate revision?

[f(malita), 2014-08-07 15:40:17]

Thanks, almost l g t m :)

On 2014/08/07 05:36:57, shreeram.k wrote:
> @Florin: PTAL.
> I made the changes related to type cast. Should we handle hiding the cast in
> this revision or in a separate revision?

We can do that later if you prefer.

https://codereview.chromium.org/446543002/diff/20001/Source/platform/graphics...
File Source/platform/graphics/Path.cpp (right):

https://codereview.chromium.org/446543002/diff/20001/Source/platform/graphics...
Source/platform/graphics/Path.cpp:273: return
static_cast<WindRule>(m_path.getFillType());
Note that WindRule <-> FillType is not a bijection (the former has 2 values, the
latter has 4), so casting from FillType to WindRule is not safe in general.

Let's leave this conversion as before.

[shreeramk, 2014-08-08 04:38:58]

@Florin:
Updated according to comments.

Ok. I will handle, hiding the cast in a separate revision.

[f(malita), 2014-08-08 13:17:56]

lgtm

[commit-bot: I haz the power, 2014-08-08 13:18:59]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/shreeram.k@samsung.com/446543002/40001

[commit-bot: I haz the power, 2014-08-08 15:07:17]

Change committed as 179838