RESOLVED FIXED 3674
GW: Add CGPoint and other small fixes to KWQPoint*
https://bugs.webkit.org/show_bug.cgi?id=3674
Summary GW: Add CGPoint and other small fixes to KWQPoint*
Eric Seidel (no email)
Reported 2005-06-23 03:10:19 PDT
Add CGPoint and other small fixes to KWQPoint* including adding: QPoint::setX(int) QPoint::setY(int) QPoint::isNull() QPointArray::QPointArray(QRect) QPointArray::boundingRect() QPointArray::copy() QPointArray::point(int, int *, int *)
Attachments
Adds CGPoint support, as well as various other small fixes (4.32 KB, patch)
2005-06-23 03:12 PDT, Eric Seidel (no email)
darin: review-
Formatting oversights now fixed. (4.33 KB, patch)
2005-06-23 12:49 PDT, Eric Seidel (no email)
darin: review-
addressed Darin's concerns (4.33 KB, patch)
2005-06-30 03:21 PDT, Eric Seidel (no email)
eric: review-
Addressed SadEagle's boundingRect() concern. (4.43 KB, patch)
2005-07-05 17:21 PDT, Eric Seidel (no email)
mjs: review+
Eric Seidel (no email)
Comment 1 2005-06-23 03:12:01 PDT
Created attachment 2578 [details] Adds CGPoint support, as well as various other small fixes Please pardon the few additional white-space changes. I ran a "re-indent" command over the entire file to replace tabs w/ spaces... it added a few spaces where there were not before.
Darin Adler
Comment 2 2005-06-23 07:19:24 PDT
Comment on attachment 2578 [details] Adds CGPoint support, as well as various other small fixes The operator * implementation does not match our coding style. The "{" should be on a second line. The implementation of isNull() has some extra punctuation and spaces in it. Should just be "return xCoord == 0 && yCoord == 0;". There are extra spaces in the setPoints call inside the QPointArray constructor that takes a QRect. I know that matches the existing constructor, but it does not match our coding cuidelines. In QPointArray::copy, it would be better to use a static_cast rather than a C-style cast. The boundingRect and point functions have the "{" o the same line as the function declaration, but it should be on the next line. The call to QMemArray<QPoint>::at(index) inside QPointArray::point, should just be a call to at(index).
Eric Seidel (no email)
Comment 3 2005-06-23 12:49:30 PDT
Created attachment 2604 [details] Formatting oversights now fixed.
Darin Adler
Comment 4 2005-06-23 22:57:42 PDT
Comment on attachment 2604 [details] Formatting oversights now fixed. Forward declaration of class QRect should be at the top of the file. No space after "-=" in operator -= declaration. When importing things in the KWQ directory, don't use forwarding headers. Hence it should be: #import "KWQRect.h" Otherwise, looks fine.
Eric Seidel (no email)
Comment 5 2005-06-30 03:21:17 PDT
Created attachment 2713 [details] addressed Darin's concerns
Maks Orlovich
Comment 6 2005-07-05 12:12:22 PDT
Is it just me, or does this never use minX > 0, minY > 0 for the top-left corner of the rectangle?
Eric Seidel (no email)
Comment 7 2005-07-05 17:21:14 PDT
Created attachment 2824 [details] Addressed SadEagle's boundingRect() concern.
Eric Seidel (no email)
Comment 8 2005-07-05 17:22:57 PDT
Comment on attachment 2713 [details] addressed Darin's concerns Replaced patch to fix SadEagle's boundingRect() concerns.
Maciej Stachowiak
Comment 9 2005-07-06 22:23:12 PDT
r=me
Note You need to log in before you can comment on or make changes to this bug.