[Webkit-unassigned] [Bug 27349] [WINCE] Add GraphicsContextWince implementation for WinCE.

[bugzilla-daemon at webkit.org]

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


Adam Roben (aroben) <aroben at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #33091|review?                     |review-
               Flag|                            |




--- Comment #16 from Adam Roben (aroben) <aroben at apple.com>  2009-07-20 11:17:32 PDT ---
(From update of attachment 33091)
> +#include "wtf/OwnPtr.h"

"wtf/OwnPtr.h" should be <wtf/OwnPtr.h> (with angle brackets instead of
quotes).

> +struct RotationTransform
> +{

The opening brace for type declarations (struct, class, enum) should be on the
same line as the type name. (Note that this differs from the rule for
functions.) (applies elsewhere)

> +    IntPoint origin()

This can be a const member function.

> +    IntRect mapRect(const IntRect& rect)
> +    {
> +        return m_transform.mapRect(rect);
> +    }
> +
> +    FloatRect mapRect(const FloatRect& rect)
> +    {
> +        return m_transform.mapRect(rect);
> +    }
> +
> +    IntPoint mapPoint(const IntPoint& point)
> +    {
> +        return m_transform.mapPoint(point);
> +    }
> +
> +    FloatPoint mapPoint(const FloatPoint& point)
> +    {
> +        return m_transform.mapPoint(point);
> +    }
> +
> +    FloatSize mapSize(const FloatSize& size)
> +    {
> +        double w, h;
> +        m_transform.map(size.width(), size.height(), w, h);
> +        return FloatSize(static_cast<float>(w), static_cast<float>(h));
> +    }

These can be const member functions.

> +    PassRefPtr<SharedBitmap> getTransparentLayerBitmap(IntRect& origRect, AlphaPaintType alphaPaint, RECT& bmpRect, bool checkClipBox, bool force)
> +    {
> +        if (m_opacity <= 0)
> +            return 0;
> +
> +        if (force || m_opacity < 1.)  {
> +            if (checkClipBox) {
> +                RECT clipBox;
> +                int clipType = GetClipBox(m_dc, &clipBox);
> +                if (clipType == SIMPLEREGION || clipType == COMPLEXREGION)
> +                    origRect.intersect(clipBox);
> +                if (origRect.isEmpty())
> +                    return 0;
> +            }
> +
> +            RefPtr<SharedBitmap> bmp = SharedBitmap::createInstance(alphaPaint == AlphaPaintNone, origRect.width(), origRect.height(), false);
> +            SetRect(&bmpRect, 0, 0, origRect.width(), origRect.height());
> +            if (bmp) {
> +                switch (alphaPaint) {
> +                case AlphaPaintNone:
> +                case AlphaPaintImage:
> +                    {
> +                        SharedBitmap::DCHolder dc(bmp.get());
> +                        if (dc.get()) {
> +                            BitBlt(dc.get(), 0, 0, origRect.width(), origRect.height(), m_dc, origRect.x(), origRect.y(), SRCCOPY);
> +                            if (bmp->is32bit() && (!m_bitmap || m_bitmap->is16bit())) {
> +                                // Set alpha channel
> +                                unsigned* pixels = (unsigned*)bmp->bytes();

Please use a C++-style cast here.

> +template <typename PixelType, bool Is16bit> void _rotateBitmap(SharedBitmap* destBmp, const SharedBitmap* sourceBmp, const RotationTransform& transform)

This can be marked "static" so it gets internal linkage.

> +{
> +    int destW = destBmp->width();
> +    int destH = destBmp->height();
> +    int sourceW = sourceBmp->width();
> +    int sourceH = sourceBmp->height();
> +    PixelType* dest = (PixelType*)destBmp->bytes();
> +    const PixelType* source = (const PixelType*)sourceBmp->bytes();

Please use C++-style casts here.

> +void rotateBitmap(SharedBitmap* destBmp, const SharedBitmap* sourceBmp, const RotationTransform& transform)

This can be marked "static" so it gets internal linkage.

> +void GraphicsContext::drawRect(const IntRect& rect)
> +{
> +    if (!m_data->m_opacity || paintingDisabled() || rect.width() <= 0 || rect.height() <= 0)
> +        return;

I thought you were going to use rect.isEmpty() instead of the width() and
height() checks?

> +#define EQUAL_ANGLE(a, b) (fabs((a) - (b)) < 1e-5)

An inline function would be nicer.

> +void GraphicsContext::setPlatformShouldAntialias(bool should)
> +{
> +    (void)should;
> +    notImplemented();
> +}
> +
> +void GraphicsContext::setLineDash(const DashArray&, float dashOffset)
> +{
> +    (void)dashOffset;
> +    notImplemented();
> +}

As I said before, you should either use UNUSED_PARAM from wtf/UnusedParam.h, or
omit the parameter names entirely.

This patch still has many instances of names like "transparentDc" instead of
"transparentDC", and still has the single-parameter-per-line style for some
function calls. Please fix those.

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