Bug 61893 - Switch ContainerNode to use IntPoint
Summary: Switch ContainerNode to use IntPoint
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Emil A Eklund
URL:
Keywords:
Depends on: 61949
Blocks: 60318
  Show dependency treegraph
 
Reported: 2011-06-01 16:35 PDT by Emil A Eklund
Modified: 2011-06-07 14:06 PDT (History)
4 users (show)

See Also:


Attachments
Patch (4.18 KB, patch)
2011-06-01 17:03 PDT, Emil A Eklund
no flags Details | Formatted Diff | Diff
Patch (4.23 KB, patch)
2011-06-07 11:41 PDT, Emil A Eklund
no flags Details | Formatted Diff | Diff
Patch for landing (4.19 KB, patch)
2011-06-07 12:51 PDT, Emil A Eklund
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Emil A Eklund 2011-06-01 16:35:04 PDT
ContainerNode uses x, y, w, h in a couple of places when dealing with render boxes and line layout. Convert this to points and sizes instead.
Comment 1 Emil A Eklund 2011-06-01 17:03:30 PDT
Created attachment 95690 [details]
Patch
Comment 2 Eric Seidel (no email) 2011-06-02 08:33:47 PDT
Comment on attachment 95690 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=95690&action=review

I think we should consider renaming the move() calls to moveBy() or something to make it clear what move(Point) is doing.  (I know rect and IntPoint use the same naming, so maybe that should be a separate change?)

> Source/WebCore/dom/ContainerNode.cpp:881
> +                point.move(box->location());

Bleh.  I'm liking generic move(IntPoint) less and less.  it's not clear if it sets or moves.

> Source/WebCore/dom/ContainerNode.cpp:907
> +        point.move(box->size());

move(Size) is OK, but we still might want to rename both of these to moveBy.

> Source/WebCore/platform/graphics/FloatPoint.h:107
> +        return IntPoint(m_x > other.m_x ? m_x : other.m_x,
> +            m_y > other.m_y ? m_y : other.m_y);

I don't think the wrapping his helpful here.
Comment 3 Emil A Eklund 2011-06-07 11:41:32 PDT
Created attachment 96271 [details]
Patch
Comment 4 Eric Seidel (no email) 2011-06-07 12:04:26 PDT
Comment on attachment 96271 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=96271&action=review

> Source/WebCore/platform/graphics/FloatPoint.h:125
> +        return FloatPoint(m_x > other.m_x ? m_x : other.m_x, m_y > other.m_y ? m_y : other.m_y);

This sounds like std::min/std::max....
Comment 5 Emil A Eklund 2011-06-07 12:08:06 PDT
Thanks, didn't realize we could use std::min/max in WebCore.
Comment 6 Emil A Eklund 2011-06-07 12:51:44 PDT
Created attachment 96286 [details]
Patch for landing
Comment 7 WebKit Review Bot 2011-06-07 14:06:39 PDT
Comment on attachment 96286 [details]
Patch for landing

Clearing flags on attachment: 96286

Committed r88264: <http://trac.webkit.org/changeset/88264>
Comment 8 WebKit Review Bot 2011-06-07 14:06:43 PDT
All reviewed patches have been landed.  Closing bug.