[webkit-reviews] review denied: [Bug 6868] replace QMatrix with a new platform-independent class : [Attachment 6019] first cut -- after review I plan to re-read and do final refinements

[bugzilla-request-daemon at opendarwin.org]

Dave Hyatt <hyatt at apple.com> has denied Darin Adler <darin at apple.com>'s request
for review:
Bug 6868: replace QMatrix with a new platform-independent class
http://bugzilla.opendarwin.org/show_bug.cgi?id=6868

Attachment 6019: first cut -- after review I plan to re-read and do final
refinements
http://bugzilla.opendarwin.org/attachment.cgi?id=6019&action=edit

------- Additional Comments from Dave Hyatt <hyatt at apple.com>
I would prefer it if x() and y() remained on the rect class.  Much of the code
that uses rects operates in terms of x and y, and I think it creates an
inconsistency to eliminate these methods on rect while leaving all the calling
code still using x/y variable names and conventions.

Philosophically you have also mutated what was essentially a PixelRect class
into a PointRect class.  There have been assertions that old methods like
right/bottom or contains were confusing but they did have well-understood
semantics for what was essentially a rect operating on pixels and not on
points.  My biggest concern here is just making sure all the call sites that
used right/bottom   have been patched properly, since 1 pixel errors will be
really hard to spot with any of our regression tests.

This reworking of IntRect to be point-based and not pixel-based has also led to
methods like contains being a little strange, since contains is a question
being asked of a pixel.  
I don't really like the name containsPixel, and feel like this clarification
was almost only needed because the rect has been reworked to operate on points
instead of pixels.

That said, my main objection is the x/y removal, since even a point-based rect
is constructed with points that have x/y coordinates, and I think it's pretty
clear to clients of the rect that the x/y of a rect would refer to the top left
point's x/y.