Bug 3674 - GW: Add CGPoint and other small fixes to KWQPoint*
Summary: GW: Add CGPoint and other small fixes to KWQPoint*
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 412
Hardware: Mac OS X 10.4
: P4 Enhancement
Assignee: Eric Seidel (no email)
URL:
Keywords:
Depends on:
Blocks: 3250
  Show dependency treegraph
 
Reported: 2005-06-23 03:10 PDT by Eric Seidel (no email)
Modified: 2005-07-06 23:40 PDT (History)
0 users

See Also:


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-
Details | Formatted Diff | Diff
Formatting oversights now fixed. (4.33 KB, patch)
2005-06-23 12:49 PDT, Eric Seidel (no email)
darin: review-
Details | Formatted Diff | Diff
addressed Darin's concerns (4.33 KB, patch)
2005-06-30 03:21 PDT, Eric Seidel (no email)
eric: review-
Details | Formatted Diff | Diff
Addressed SadEagle's boundingRect() concern. (4.43 KB, patch)
2005-07-05 17:21 PDT, Eric Seidel (no email)
mjs: 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 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 *)
Comment 1 Eric Seidel (no email) 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.
Comment 2 Darin Adler 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).
Comment 3 Eric Seidel (no email) 2005-06-23 12:49:30 PDT
Created attachment 2604 [details]
Formatting oversights now fixed.
Comment 4 Darin Adler 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.
Comment 5 Eric Seidel (no email) 2005-06-30 03:21:17 PDT
Created attachment 2713 [details]
addressed Darin's concerns
Comment 6 Maks Orlovich 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?  
 
Comment 7 Eric Seidel (no email) 2005-07-05 17:21:14 PDT
Created attachment 2824 [details]
Addressed SadEagle's boundingRect() concern.
Comment 8 Eric Seidel (no email) 2005-07-05 17:22:57 PDT
Comment on attachment 2713 [details]
addressed Darin's concerns

Replaced patch to fix SadEagle's boundingRect() concerns.
Comment 9 Maciej Stachowiak 2005-07-06 22:23:12 PDT
r=me