Bug 4462

QRectF: http://doc.trolltech.com/4.0/qrectf.html

Created attachment 3898 [details] Implements QRectF Implements QRectF, floating point QRect Modifies WebCore to have templated class QRectBase<T>, and separate classes QRect and QRectF. Same goes for QPoint and QSize Moves QPoint out of QPointArray Doesn't include modifications to xcodeproj

Created attachment 3903 [details] Implements QRectF Addresses following comments: < MacDome> Kompo_: 1. all the various F and non-F variants should be in the same KWQ file * Implementations of KWQ{Rect,Point,Size}Base<T> and Q{Rect,Point,Size} are in KWQRect.h KWQRect.mm * NOTE: Remember to delete KWQPoint.mm and KWQSize.{cpp,h} < MacDome> 2. we should use float and not double for QRectF, etc. * Changed < MacDome> Kompo_: I belive _KWQ_IOSTREAM_ is dead now * Removed Note also that cvs-create-patch --include-unknown seems to omit directory from the new files.. The new files go to WebCore/ForwardingHeaders/

Created attachment 3904 [details] Make KCanvas and KSVG2 to use QRectF Make KCanvas and KSVG2 to use QRectF and QPointF . Feedback wanted on are these changed points reasonable and if there is more points to change

Created attachment 3905 [details] Updated layout-tests for 3904 Layout-tests diffs that are result of 309. Difference is that some places now the fractional part is also printed. These include gradient points, bounding boxes etc.

Created attachment 3908 [details] Implements QRectF, QPointF, QSizeF So the classes should have still been in distinct files, only the base templates defined in the same file as the specializations. Implements KWQ*Base<T>, Q*, Q*F in KWQ*.h and KWQ*.mm.

Created attachment 3910 [details] Implements QRectF, QPointF, QSizeF addressses the points: - copyrights & years - corefoundation includes and ifdef __objc__ - inline constructors

Comment on attachment 3908 [details] Implements QRectF, QPointF, QSizeF This looks good. I'll look at landing this tomorrow/early this week.

Comment on attachment 3910 [details] Implements QRectF, QPointF, QSizeF Looks like the obsolete patch got the review+.

Comment on attachment 3910 [details] Implements QRectF, QPointF, QSizeF Ok, I've looked over this some more (while trying to land) and have run into some issues. 1. You inlined (moved to the header) a bunch of functions which were not previously inlined. I have not yet tested to see what the performance effects of that change are, but I can get some numbers for you. I would prefer that this change be as small as possible. Just changing the existing QSize, QRect, QPoint to QSizeBase<T> QRectBase<T> and QPointBase<T> and then having the smallest possible subclasses for the F and non-F varients. 2. Related to the first, you use .cpp style Foo::bar() { blah blah} syntax for declaring these extra functions in the headers. Looking through other pieces of WebCore, either functions are small enough to fit in the actual declaration, or they aren't defined in the headers. Was there some reason you chose this route? 3. The compile breaks w/ this change, due to: /Volumes/Stuff/Projects/WKOpen2/WebCore/../SVGSupport/kcanvas/device/quartz/KRe nderingDeviceQuartz.mm:127: error: no matching function for call to 'CGSize::CGSize(KWQSizeBase<int>)' /System/Library/Frameworks/ApplicationServices.framework/Frameworks/CoreGraphic s.framework/Headers/CGGeometry.h:24: note: candidates are: CGSize::CGSize() /System/Library/Frameworks/ApplicationServices.framework/Frameworks/CoreGraphic s.framework/Headers/CGGeometry.h:24: note: CGSize::CGSize(const CGSize&) Not quite sure why... except that I bet the compiler isn't willing to do the upcast from QSizeBase<int> to QSize. Would it be possible to make QSize and QSizeF typedefs instead of subclasses?

Created attachment 4399 [details] Implements QRectF, QPointF, QSizeF QRectF, QPointF, QSizeF are implemented by POD classes. Implementation is copied from non-F versions 1:1. This avoids upcast problems of inheriting from template class, as well as typedef / forward declaration as class problems using typedeffed template classes.

Created attachment 4400 [details] Make KSVG2 and KCanvas use floating-point versions of qrect, qpoint and qsize Makes KSVG2 and KCanvas use floating-point versions of qrect, qpoint and qsize where they can provide more precision to the rendering.

Created attachment 4401 [details] Updated SVGSupport/layout-tests Updated layout-tests to go with these patches. The layout-tests should differ just by having floating-point values in the various object properties.

Comment on attachment 4399 [details] Implements QRectF, QPointF, QSizeF This looks fine by me. But I'd like mjs or darin's approval before commiting.

Comment on attachment 4399 [details] Implements QRectF, QPointF, QSizeF r=me.

I have not forgotten you, I promise :) This will and this week.

Comment on attachment 4399 [details] Implements QRectF, QPointF, QSizeF I need to make a new patch. This isn't going to apply cleanly anymore.

Created attachment 5378 [details] Sync previous versions with the new code layout Implements QRectF, QPointF, QSizeF Make KSVG2 and KCanvas use floating-point versions of QRect, QPoint and QSize

Created attachment 5379 [details] layout tests to match the 5378 All differing layout tests from LayoutTest/svg/ I have checked the diff, but as it is so big, I'm not sure that it doesn't contain diff regions which are bugs, and not due to change of precision

Comment on attachment 5378 [details] Sync previous versions with the new code layout There seem to be some tabs in this patch. Also, as just a general remark, the most currently style guidelines require for "QRect& rect" instead of "QRect &rect". The tabs can be fixed when landing, and the & spacing issue is not a show stopper. I found toRect() a bit odd. It's not easy to tell from the name fo the method what sort of rounding rules it actually uses. Currently it just does a float->int cast. In certain circumstances (like dirty rects), you don't actually want to just cast, but rather find the minimal enclosing integer rect. (x2 = floor(x1), y2 = floor(y1), width2 = ciel(x1+width1)-x2, height2 = ciel(y1+height1)-y2) There may be other small issues. I mostly skimmed the patch just now.

Comment on attachment 5378 [details] Sync previous versions with the new code layout One more comment: It would be nice to be able to test some of these SVG functional changes. For example, now that we are storing fractional bounding box information for gradients and patterns, those should be testable.

Created attachment 5441 [details] Fixed the toRect() and some tabs. Fixed the toRect() and some tabs. Didn't fix the & and some tabs, there'd be inconceistency inside the files.. maybe fix them in a different patch?

Created attachment 5443 [details] LayoutTests/svg diff for the patch. Also includes new test case new testcase is svg/common/fractional-rects.svg, which is copy of svg/common/rounded-rects.svg. You can check how the output differs..

Comment on attachment 5441 [details] Fixed the toRect() and some tabs. I think this is good enough to land. r=me.

I made some small style corrections (removeing {} for one line ifs, changing "Type &var" to "Type& var") and fixed one small error with enclosingRect (- was used instead of +), that's it.

Thanks again kimmo. Sorry this took so long to land.