[Webkit-unassigned] [Bug 28122] [Haiku] Adding some simple graphic files (IntPoint, =?UTF-8?Q?=20IntRect=E2=80=A6?=)

[bugzilla-daemon at webkit.org]

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


Eric Seidel <eric at webkit.org> changed:

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




--- Comment #2 from Eric Seidel <eric at webkit.org>  2009-08-09 07:46:50 PDT ---
(From update of attachment 34407)
What does the color struct look like?  Could you just use a COMPILE_ASSERT to
assert identical layout with the WebCore:RGBA32 type and not need to call
make_color, etc?

See how I did things in ColorSkia (where we just cast directly).  Your current
solution is *totally fine*.  I just wanted to bring the opportunity of casting
directly to your attention.

Why are the +1 needed for one way conversion:
 38     , m_size(rect.Width() + 1, rect.Height() + 1)

and -1 is not needed the other way:
 44     return BRect(BPoint(x(), y()), BSize(width(), height()));

That at least needs a comment.

It seems if BPoint is a float point you don't want to have it implicitly
constructed.
 IntPoint::IntPoint(const BPoint& point)
 37     : m_x(static_cast<int>(point.x))
 38     , m_y(static_cast<int>(point.y))
 39 {
 40 }

See how CGPoint is handled.

Yeah, it seems that BRect, BPoint, BSize should only map to the Float* variants
(similar to how CG* works).  Why a FIXME here, but not before?
42 IntRect::operator BRect() const
 43 {
 44     // FIXME: subtract 1 from height and width?
 45     return BRect(BPoint(x(), y()), BSize(width(), height()));
 46 }

Parts of this would be easy to approve.  I think you need to double-check how
you do the B* to Float*/Int* conversions.  Since it seems that B* are all
floating-point values, you can follow exactly what the CG* types do, since
those are all floating point values.

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