Bug 44994 - Add length and related operations to FloatPoint and FloatSize
Summary: Add length and related operations to FloatPoint and FloatSize
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Kenneth Russell
URL:
Keywords:
Depends on:
Blocks: 44729
  Show dependency treegraph
 
Reported: 2010-08-31 14:50 PDT by Kenneth Russell
Modified: 2010-09-01 10:10 PDT (History)
5 users (show)

See Also:


Attachments
Patch (3.72 KB, patch)
2010-08-31 14:54 PDT, Kenneth Russell
cmarrin: review-
kbr: commit-queue-
Details | Formatted Diff | Diff
Revised patch (4.48 KB, patch)
2010-08-31 15:48 PDT, Kenneth Russell
cmarrin: review+
kbr: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kenneth Russell 2010-08-31 14:50:48 PDT
For some geometric algorithms it is useful to treat FloatPoint as a vector on the 2D plane and query its length or the square of its length. It is also useful to be able to compute the distance or squared distance between two points. We should add length computations to FloatPoint and FloatSize for these purposes.
Comment 1 Kenneth Russell 2010-08-31 14:54:19 PDT
Created attachment 66116 [details]
Patch

From the ChangeLog:

Added length and squared length operations to both FloatPoint and FloatSize, and added set(x, y) operation to FloatPoint. These changes have been tested with new code to be added later.
Comment 2 Chris Marrin 2010-08-31 15:27:15 PDT
Comment on attachment 66116 [details]
Patch

...
> Index: WebCore/platform/graphics/FloatPoint.h
> ===================================================================
> --- WebCore/platform/graphics/FloatPoint.h	(revision 66532)
> +++ WebCore/platform/graphics/FloatPoint.h	(working copy)
> @@ -80,6 +80,11 @@ public:
>  
>      void setX(float x) { m_x = x; }
>      void setY(float y) { m_y = y; }
> +    void set(float x, float y)
> +    {
> +        m_x = x;
> +        m_y = y;
> +    }
>      void move(float dx, float dy)
>      {
>          m_x += dx;
> @@ -91,6 +96,12 @@ public:
>          m_y *= sy;
>      }
>  
> +    float length() const;
> +    float lengthSquared() const
> +    {
> +        return m_x * m_x + m_y * m_y;
> +    }
> +
>  #if PLATFORM(CG)
>      FloatPoint(const CGPoint&);
>      operator CGPoint() const;
> Index: WebCore/platform/graphics/FloatSize.cpp

Need a casting operator between FloatPoint and FloatPoint3D (here and in FloatPoint3D).

normalize() and dot() would be useful here as well, and would make the classes more consistent. For completeness you could add cross() as well, although I've never seen it used for a 2D point.
Comment 3 Kenneth Russell 2010-08-31 15:34:34 PDT
(In reply to comment #2)
> Need a casting operator between FloatPoint and FloatPoint3D (here and in FloatPoint3D).

I don't think this is a good idea. Providing such a casting operator would allow implicit casts and assignments to succeed that could lead to programming errors. FloatPoint3D already has a constructor taking FloatPoint which sets the z component to zero. We could add an xy() method on FloatPoint3D returning FloatPoint (or six methods like xy(), xz(), etc.).

> normalize() and dot() would be useful here as well, and would make the classes more consistent. For completeness you could add cross() as well, although I've never seen it used for a 2D point.

I can add normalize() and dot(). Cross product is an operation which is only defined on three-dimensional vectors.
Comment 4 Kenneth Russell 2010-08-31 15:48:38 PDT
Created attachment 66127 [details]
Revised patch

Added normalize() and dot product operations to FloatPoint based on review feedback.
Comment 5 Chris Marrin 2010-08-31 16:08:02 PDT
(In reply to comment #3)
> (In reply to comment #2)
> > Need a casting operator between FloatPoint and FloatPoint3D (here and in FloatPoint3D).
> 
> I don't think this is a good idea. Providing such a casting operator would allow implicit casts and assignments to succeed that could lead to programming errors. FloatPoint3D already has a constructor taking FloatPoint which sets the z component to zero. We could add an xy() method on FloatPoint3D returning FloatPoint (or six methods like xy(), xz(), etc.).

I agree that the ctor is probably sufficient.

> 
> > normalize() and dot() would be useful here as well, and would make the classes more consistent. For completeness you could add cross() as well, although I've never seen it used for a 2D point.
> 
> I can add normalize() and dot(). Cross product is an operation which is only defined on three-dimensional vectors.

I have seen 2D cross product implemented. It returns a scalar value which is the magnitude of resultant vector. Apparently (and I never did the math so I can't verify this), it's also twice the area of the triangle formed by the two vectors. But I agree, I don't think this is important.

So just normalize and dot will fill out the class nicely.
Comment 6 Kenneth Russell 2010-08-31 17:07:39 PDT
(In reply to comment #5)
> So just normalize and dot will fill out the class nicely.

normalize and dot have been added per above. Please re-review.
Comment 7 Kenneth Russell 2010-08-31 17:38:48 PDT
Committed r66560: <http://trac.webkit.org/changeset/66560>