[Webkit-unassigned] [Bug 23147] Introduce Skia to WebKit

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=23147





------- Comment #4 from sam at webkit.org  2009-01-06 14:19 PDT -------
(From update of attachment 26470)
I have only reviewed up to GradientSkia.  I am not sure what GdkSkia is for,
and therefore, it could use explanation in the ChangeLog.


> +static const double deg2rad = 0.017453292519943295769; // pi/180

This should already be defined in WTF/MathExtras.h

> +AffineTransform::AffineTransform(const SkMatrix& matrix) : m_transform(matrix)

The : m_transform(matrix) should be on the next line.

> +
> +void AffineTransform::map(double x, double y, double *x2, double *y2) const

* on the wrong side.

> +IntRect AffineTransform::mapRect(const IntRect& src) const
> +{
> +    SkRect  dst;

Extra space.

> +
> +AffineTransform &AffineTransform::scale(double sx, double sy)

& on the wrong side.

> +AffineTransform &AffineTransform::translate(double tx, double ty)

& on the wrong side.

> +AffineTransform &AffineTransform::shear(double sx, double sy)

& on the wrong side.

> +AffineTransform &AffineTransform::operator*=(const AffineTransform& m2)

& on the wrong side.

> +
> +double AffineTransform::a() const
> +{
> +    return SkScalarToDouble(m_transform.getScaleX());
> +}
> +void AffineTransform::setA(double a)

Missing newline.

> +double AffineTransform::b() const
> +{
> +    return SkScalarToDouble(m_transform.getSkewY());
> +}
> +void AffineTransform::setB(double b)

Missing newline.

> +double AffineTransform::c() const
> +{
> +    return SkScalarToDouble(m_transform.getSkewX());
> +}
> +void AffineTransform::setC(double c)

Missing newline.

> +double AffineTransform::d() const
> +{
> +    return SkScalarToDouble(m_transform.getScaleY());
> +}
> +void AffineTransform::setD(double d)

Missing newline.

> +double AffineTransform::e() const
> +{
> +    return SkScalarToDouble(m_transform.getTranslateX());
> +}
> +void AffineTransform::setE(double e)

Missing newline.

> +double AffineTransform::f() const
> +{
> +    return SkScalarToDouble(m_transform.getTranslateY());
> +}
> +void AffineTransform::setF(double f)

Missing newline.

> +
> +// Determine the total number of stops needed, including pseudo-stops at the
> +// ends as necessary.
> +static size_t total_stops_needed(const Gradient::ColorStop* stopData, size_t count)

Please use camelCase.

> +{
> +    const Gradient::ColorStop* stop = stopData;
> +    size_t count_used = count;
> +    if (count < 1 || stop->stop > 0.0)
> +      count_used++;
> +    stop += count - 1;
> +    if (count < 2 || stop->stop < 1.0)
> +      count_used++;
> +    return count_used;

Indentation seems wrong here.

> +}
> +
> +// Collect sorted stop position and color information into the pos and colors 
> +// buffers, ensuring stops at both 0.0 and 1.0.  The buffers must be large
> +// enough to hold information for all stops, including the new endpoints if
> +// stops at 0.0 and 1.0 aren't already included.
> +static void fill_stops(const Gradient::ColorStop* stopData,
> +                       size_t count, SkScalar* pos, SkColor* colors)

Please use camelCase.

> +SkShader* Gradient::platformGradient()
> +{
> +    if (m_gradient)
> +        return m_gradient;
> +
> +    // TODO: This and compareStops() are also in Gradient.cpp and

This should be a FIXME.

> +    // CSSGradientValue.cpp; probably should refactor in WebKit.
> +    if (!m_stopsSorted) {
> +        if (m_stops.size())
> +            std::stable_sort(m_stops.begin(), m_stops.end(), compareStops);
> +        m_stopsSorted = true;
> +    }
> +    size_t count_used = total_stops_needed(m_stops.data(), m_stops.size());

Please use camelCase.

> +    ASSERT(count_used >= 2);
> +    ASSERT(count_used >= m_stops.size());
> +
> +    // FIXME: Why is all this manual pointer math needed?!
> +    SkAutoMalloc storage(count_used * (sizeof(SkColor) + sizeof(SkScalar)));
> +    SkColor* colors = (SkColor*)storage.get();
> +    SkScalar* pos = (SkScalar*)(colors + count_used);

We prefer c++ style casts.

> +
> +    fill_stops(m_stops.data(), m_stops.size(), pos, colors);
> +
> +    if (m_radial) {
> +        // TODO(mmoss) CSS radial Gradients allow an offset focal point (the

This should be a FIXME, preferably with a bug #.


-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.