Bug 3672 - GW: KWQRect -- CGRect and other small additions
Summary: GW: KWQRect -- CGRect and other small additions
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 412
Hardware: Mac OS X 10.4
: P3 Normal
Assignee: Dave Hyatt
URL:
Keywords:
Depends on:
Blocks: 3250
  Show dependency treegraph
 
Reported: 2005-06-23 02:48 PDT by Eric Seidel (no email)
Modified: 2005-06-24 13:39 PDT (History)
0 users

See Also:


Attachments
KWQRect -- CGRect and other small additions (3.98 KB, patch)
2005-06-23 02:51 PDT, Eric Seidel (no email)
darin: review-
Details | Formatted Diff | Diff
Formatting oversights now fixed. (3.71 KB, patch)
2005-06-23 12:10 PDT, Eric Seidel (no email)
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2005-06-23 02:48:47 PDT
KWQRect -- CGRect and other small additions

including:
QRect (const QPoint &tl, const QPoint &br)
QRect::top()
QRect::left()
QRect::topLeft()
QRect::bottomRight()
QRect::contiains(QRect)
QRect::normalize()

I'm not 100% sure of the correctness of all these changes... only due to the possible off-by-one error 
associated with QRect.  QRect has this funny +1 width/height property, and I'm not sure that I'm 
correctly adding/removing that +1 when converting to/from CGRect, etc.  Hopefully someone who has 
worked with QRect a bit longer than I can comment more.

That said, the code is otherwise tested and in use locally.
Comment 1 Eric Seidel (no email) 2005-06-23 02:51:29 PDT
Created attachment 2576 [details]
KWQRect -- CGRect and other small additions
Comment 2 Darin Adler 2005-06-23 07:12:28 PDT
Comment on attachment 2576 [details]
KWQRect -- CGRect and other small additions

Looks good. A few comments:

The QRect contructor that takes two points is too long to be inline. Please
move it into the .mm file.

Why did you make left() and top() non-inline? They shouldn't be changed unless
there's a reason.

In QRect::normalize, you should use "-w" rather than "(w * -1)" and "-h" rather
than "(h * -1)".

No need for the extra blank line after operator CGRect. Better to keep the
file's format consistent, even in such a trivial detail.
Comment 3 Eric Seidel (no email) 2005-06-23 12:10:00 PDT
Created attachment 2603 [details]
Formatting oversights now fixed.
Comment 4 Darin Adler 2005-06-23 22:55:52 PDT
Comment on attachment 2603 [details]
Formatting oversights now fixed.

r=me