Bug 3674

Summary:

GW: Add CGPoint and other small fixes to KWQPoint*

Product:

WebKit

Reporter:

Eric Seidel (no email) <eric>

Component:

New Bugs

Assignee:

Eric Seidel (no email) <eric>

Status:

RESOLVED FIXED

Severity:

Enhancement

Priority:

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+

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-	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
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

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.