Bug 3674

Summary: GW: Add CGPoint and other small fixes to KWQPoint*
Product: WebKit Reporter: Eric Seidel (no email) <eric>
Component: New BugsAssignee: Eric Seidel (no email) <eric>
Status: RESOLVED FIXED    
Severity: Enhancement    
Priority: P4    
Version: 412   
Hardware: Mac   
OS: OS X 10.4   
Bug Depends on:    
Bug Blocks: 3250    
Attachments:
Description Flags
Adds CGPoint support, as well as various other small fixes
darin: review-
Formatting oversights now fixed.
darin: review-
addressed Darin's concerns
eric: review-
Addressed SadEagle's boundingRect() concern. mjs: review+

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