54365 2011-02-13 14:22:35 -0800 GraphicsContext: Add clip(IntRect) overload for Qt 2011-02-14 13:54:21 -0800 1 1 1 Unclassified WebKit Layout and Rendering 528+ (Nightly build) All All RESOLVED FIXED Performance, Qt, QtTriaged P2 Normal --- 1 kling webkit-unassigned darin oldest_to_newest 350457 0 kling 2011-02-13 14:22:35 -0800 Qt has special code paths for clipping to integer rectangles. Add a GraphicsContext::clip() overload to allow us to take advantage of the optimized code. 350458 1 82278 kling 2011-02-13 14:24:29 -0800 Created attachment 82278 Proposed patch 350500 2 82278 darin 2011-02-13 18:17:33 -0800 Comment on attachment 82278 Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=82278&action=review > Source/WebCore/platform/graphics/GraphicsContext.h:326 > +#if PLATFORM(QT) > + void clip(const IntRect&); > +#endif I think we should add this cross-platform. For the other platforms we can the graphics context code convert the rectangle to floating point. You could put the version for all other platforms into GraphicsContextQt.cpp inside an #if !PLATFORM(QT) block. We really don’t want to start putting PLATFORM(QT) if statements into call sites! 350504 3 kling 2011-02-13 18:36:39 -0800 (In reply to comment #2) > (From update of attachment 82278 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=82278&action=review > > > Source/WebCore/platform/graphics/GraphicsContext.h:326 > > +#if PLATFORM(QT) > > + void clip(const IntRect&); > > +#endif > > I think we should add this cross-platform. For the other platforms we can the graphics context code convert the rectangle to floating point. You could put the version for all other platforms into GraphicsContextQt.cpp inside an #if !PLATFORM(QT) block. Fair enough. The reason I didn't do it was because most (all?) call sites automatically started using the new overload since they were using the implicit FloatRect(IntRect) constructor before. 350832 4 82354 kling 2011-02-14 12:53:24 -0800 Created attachment 82354 Proposed patch Updated patch making GraphicsContext::clip(IntRect) cross-port per Darin's suggestion. 350835 5 82354 darin 2011-02-14 13:02:02 -0800 Comment on attachment 82354 Proposed patch Seems OK. I’d like to see the patch that makes use of this ASAP, though, because until then this is dead code. 350856 6 kling 2011-02-14 13:42:58 -0800 (In reply to comment #5) > (From update of attachment 82354 [details]) > Seems OK. I’d like to see the patch that makes use of this ASAP, though, because until then this is dead code. It's already in place, some example users: RenderBlock::paintColumnContents() RenderBox::pushContentsClip() RenderBoxModelObject::paintFillLayerExtended() RenderLayer::beginTransparencyLayers() ScrollView::paint() They were previously calling clip() with implicitly constructed FloatRects. 350865 7 82354 kling 2011-02-14 13:54:07 -0800 Comment on attachment 82354 Proposed patch Clearing flags on attachment: 82354 Committed r78503: <http://trac.webkit.org/changeset/78503> 350866 8 kling 2011-02-14 13:54:21 -0800 All reviewed patches have been landed. Closing bug. 82278 2011-02-13 14:24:29 -0800 2011-02-14 12:53:24 -0800 Proposed patch bug-54365.diff text/plain 2150 kling ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCA0YWZiZjE5Li4yZjM3NzZiIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDUgKzEsMjAg QEAKIDIwMTEtMDItMTMgIEFuZHJlYXMgS2xpbmcgIDxrbGluZ0B3ZWJraXQub3JnPgogCisgICAg ICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIEdyYXBoaWNzQ29udGV4 dDogQWRkIGNsaXAoSW50UmVjdCkgb3ZlcmxvYWQgZm9yIFF0CisgICAgICAgIGh0dHBzOi8vYnVn cy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD01NDM2NQorCisgICAgICAgIFF0IGhhcyBzcGVj aWFsIGNvZGUgcGF0aHMgZm9yIGNsaXBwaW5nIHRvIGludGVnZXIgcmVjdGFuZ2xlcy4KKyAgICAg ICAgQWRkIGEgR3JhcGhpY3NDb250ZXh0OjpjbGlwKCkgb3ZlcmxvYWQgdG8gYWxsb3cgdXMgdG8g dGFrZQorICAgICAgICBhZHZhbnRhZ2Ugb2YgdGhlIG9wdGltaXplZCBjb2RlLgorCisgICAgICAg ICogcGxhdGZvcm0vZ3JhcGhpY3MvR3JhcGhpY3NDb250ZXh0Lmg6CisgICAgICAgICogcGxhdGZv cm0vZ3JhcGhpY3MvcXQvR3JhcGhpY3NDb250ZXh0UXQuY3BwOgorICAgICAgICAoV2ViQ29yZTo6 R3JhcGhpY3NDb250ZXh0OjpjbGlwKToKKworMjAxMS0wMi0xMyAgQW5kcmVhcyBLbGluZyAgPGts aW5nQHdlYmtpdC5vcmc+CisKICAgICAgICAgUmV2aWV3ZWQgYnkgRGlyayBTY2h1bHplLgogCiAg ICAgICAgIENhbnZhczogMmQuZmlsbFN0eWxlLnBhcnNlLnN5c3RlbS5odG1sIGZhaWxzCmRpZmYg LS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9HcmFwaGljc0NvbnRleHQu aCBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL0dyYXBoaWNzQ29udGV4dC5oCmlu ZGV4IGFkNWNlZGMuLmVlMTQ3YjggMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3Jt L2dyYXBoaWNzL0dyYXBoaWNzQ29udGV4dC5oCisrKyBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3Jt L2dyYXBoaWNzL0dyYXBoaWNzQ29udGV4dC5oCkBAIC0zMjEsNiArMzIxLDEwIEBAIG5hbWVzcGFj ZSBXZWJDb3JlIHsKICAgICAgICAgdm9pZCBzZXRJbWFnZUludGVycG9sYXRpb25RdWFsaXR5KElu dGVycG9sYXRpb25RdWFsaXR5KTsKICAgICAgICAgSW50ZXJwb2xhdGlvblF1YWxpdHkgaW1hZ2VJ bnRlcnBvbGF0aW9uUXVhbGl0eSgpIGNvbnN0OwogCisjaWYgUExBVEZPUk0oUVQpCisgICAgICAg IHZvaWQgY2xpcChjb25zdCBJbnRSZWN0Jik7CisjZW5kaWYKKwogICAgICAgICB2b2lkIGNsaXAo Y29uc3QgRmxvYXRSZWN0Jik7CiAgICAgICAgIHZvaWQgYWRkUm91bmRlZFJlY3RDbGlwKGNvbnN0 IFJvdW5kZWRJbnRSZWN0Jik7CiAgICAgICAgIHZvaWQgYWRkSW5uZXJSb3VuZGVkUmVjdENsaXAo Y29uc3QgSW50UmVjdCYsIGludCB0aGlja25lc3MpOwpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNv cmUvcGxhdGZvcm0vZ3JhcGhpY3MvcXQvR3JhcGhpY3NDb250ZXh0UXQuY3BwIGIvU291cmNlL1dl YkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvcXQvR3JhcGhpY3NDb250ZXh0UXQuY3BwCmluZGV4IGJm MjgyNmMuLjM4Y2RkMjYgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBo aWNzL3F0L0dyYXBoaWNzQ29udGV4dFF0LmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9y bS9ncmFwaGljcy9xdC9HcmFwaGljc0NvbnRleHRRdC5jcHAKQEAgLTc2OSw2ICs3NjksMTQgQEAg Q29udGV4dFNoYWRvdyogR3JhcGhpY3NDb250ZXh0Ojpjb250ZXh0U2hhZG93KCkKICAgICByZXR1 cm4gJm1fZGF0YS0+c2hhZG93OwogfQogCit2b2lkIEdyYXBoaWNzQ29udGV4dDo6Y2xpcChjb25z dCBJbnRSZWN0JiByZWN0KQoreworICAgIGlmIChwYWludGluZ0Rpc2FibGVkKCkpCisgICAgICAg IHJldHVybjsKKworICAgIG1fZGF0YS0+cCgpLT5zZXRDbGlwUmVjdChyZWN0LCBRdDo6SW50ZXJz ZWN0Q2xpcCk7Cit9CisKIHZvaWQgR3JhcGhpY3NDb250ZXh0OjpjbGlwKGNvbnN0IEZsb2F0UmVj dCYgcmVjdCkKIHsKICAgICBpZiAocGFpbnRpbmdEaXNhYmxlZCgpKQo= 82354 2011-02-14 12:53:24 -0800 2011-02-14 13:54:07 -0800 Proposed patch bug-54365-v2.diff text/plain 2915 kling ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCBiYTIyNDdhLi5kNTc1YzkyIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMjAg QEAKKzIwMTEtMDItMTQgIEFuZHJlYXMgS2xpbmcgIDxrbGluZ0B3ZWJraXQub3JnPgorCisgICAg ICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIEdyYXBoaWNzQ29udGV4 dDogQWRkIGNsaXAoSW50UmVjdCkgb3ZlcmxvYWQgZm9yIFF0CisgICAgICAgIGh0dHBzOi8vYnVn cy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD01NDM2NQorCisgICAgICAgIFF0IGhhcyBzcGVj aWFsIGNvZGUgcGF0aHMgZm9yIGNsaXBwaW5nIHRvIGludGVnZXIgcmVjdGFuZ2xlcy4KKyAgICAg ICAgQWRkIGEgR3JhcGhpY3NDb250ZXh0OjpjbGlwKCkgb3ZlcmxvYWQgdG8gYWxsb3cgdXMgdG8g dGFrZQorICAgICAgICBhZHZhbnRhZ2Ugb2YgdGhlIG9wdGltaXplZCBjb2RlLgorCisgICAgICAg ICogcGxhdGZvcm0vZ3JhcGhpY3MvR3JhcGhpY3NDb250ZXh0LmNwcDoKKyAgICAgICAgKFdlYkNv cmU6OkdyYXBoaWNzQ29udGV4dDo6Y2xpcCk6CisgICAgICAgICogcGxhdGZvcm0vZ3JhcGhpY3Mv R3JhcGhpY3NDb250ZXh0Lmg6CisgICAgICAgICogcGxhdGZvcm0vZ3JhcGhpY3MvcXQvR3JhcGhp Y3NDb250ZXh0UXQuY3BwOgorICAgICAgICAoV2ViQ29yZTo6R3JhcGhpY3NDb250ZXh0OjpjbGlw KToKKwogMjAxMS0wMi0xMyAgTmljbyBXZWJlciAgPHRoYWtpc0BjaHJvbWl1bS5vcmc+CiAKICAg ICAgICAgUmV2aWV3ZWQgYnkgQW5kZXJzIENhcmxzc29uLgpkaWZmIC0tZ2l0IGEvU291cmNlL1dl YkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvR3JhcGhpY3NDb250ZXh0LmNwcCBiL1NvdXJjZS9XZWJD b3JlL3BsYXRmb3JtL2dyYXBoaWNzL0dyYXBoaWNzQ29udGV4dC5jcHAKaW5kZXggMTZlZmE3MC4u MmQwYmQxZSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvR3Jh cGhpY3NDb250ZXh0LmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9H cmFwaGljc0NvbnRleHQuY3BwCkBAIC01NTAsNiArNTUwLDEzIEBAIHZvaWQgR3JhcGhpY3NDb250 ZXh0OjpkcmF3SW1hZ2VCdWZmZXIoSW1hZ2VCdWZmZXIqIGltYWdlLCBDb2xvclNwYWNlIHN0eWxl Q29sb3JTCiAgICAgICAgIGltYWdlLT5kcmF3KHRoaXMsIHN0eWxlQ29sb3JTcGFjZSwgRmxvYXRS ZWN0KGRlc3QubG9jYXRpb24oKSwgRmxvYXRTaXplKHR3LCB0aCkpLCBGbG9hdFJlY3Qoc3JjLmxv Y2F0aW9uKCksIEZsb2F0U2l6ZSh0c3csIHRzaCkpLCBvcCwgdXNlTG93UXVhbGl0eVNjYWxlKTsK IH0KIAorI2lmICFQTEFURk9STShRVCkKK3ZvaWQgR3JhcGhpY3NDb250ZXh0OjpjbGlwKGNvbnN0 IEludFJlY3QmIHJlY3QpCit7CisgICAgY2xpcChGbG9hdFJlY3QocmVjdCkpOworfQorI2VuZGlm CisKIHZvaWQgR3JhcGhpY3NDb250ZXh0OjphZGRSb3VuZGVkUmVjdENsaXAoY29uc3QgUm91bmRl ZEludFJlY3QmIHJlY3QpCiB7CiAgICAgaWYgKHBhaW50aW5nRGlzYWJsZWQoKSkKZGlmZiAtLWdp dCBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL0dyYXBoaWNzQ29udGV4dC5oIGIv U291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvR3JhcGhpY3NDb250ZXh0LmgKaW5kZXgg YWQ1Y2VkYy4uNzY5NTA1MSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3Jh cGhpY3MvR3JhcGhpY3NDb250ZXh0LmgKKysrIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3Jh cGhpY3MvR3JhcGhpY3NDb250ZXh0LmgKQEAgLTMyMSw2ICszMjEsNyBAQCBuYW1lc3BhY2UgV2Vi Q29yZSB7CiAgICAgICAgIHZvaWQgc2V0SW1hZ2VJbnRlcnBvbGF0aW9uUXVhbGl0eShJbnRlcnBv bGF0aW9uUXVhbGl0eSk7CiAgICAgICAgIEludGVycG9sYXRpb25RdWFsaXR5IGltYWdlSW50ZXJw b2xhdGlvblF1YWxpdHkoKSBjb25zdDsKIAorICAgICAgICB2b2lkIGNsaXAoY29uc3QgSW50UmVj dCYpOwogICAgICAgICB2b2lkIGNsaXAoY29uc3QgRmxvYXRSZWN0Jik7CiAgICAgICAgIHZvaWQg YWRkUm91bmRlZFJlY3RDbGlwKGNvbnN0IFJvdW5kZWRJbnRSZWN0Jik7CiAgICAgICAgIHZvaWQg YWRkSW5uZXJSb3VuZGVkUmVjdENsaXAoY29uc3QgSW50UmVjdCYsIGludCB0aGlja25lc3MpOwpk aWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvcXQvR3JhcGhpY3ND b250ZXh0UXQuY3BwIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvcXQvR3JhcGhp Y3NDb250ZXh0UXQuY3BwCmluZGV4IGJmMjgyNmMuLjM4Y2RkMjYgMTAwNjQ0Ci0tLSBhL1NvdXJj ZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL3F0L0dyYXBoaWNzQ29udGV4dFF0LmNwcAorKysg Yi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9xdC9HcmFwaGljc0NvbnRleHRRdC5j cHAKQEAgLTc2OSw2ICs3NjksMTQgQEAgQ29udGV4dFNoYWRvdyogR3JhcGhpY3NDb250ZXh0Ojpj b250ZXh0U2hhZG93KCkKICAgICByZXR1cm4gJm1fZGF0YS0+c2hhZG93OwogfQogCit2b2lkIEdy YXBoaWNzQ29udGV4dDo6Y2xpcChjb25zdCBJbnRSZWN0JiByZWN0KQoreworICAgIGlmIChwYWlu dGluZ0Rpc2FibGVkKCkpCisgICAgICAgIHJldHVybjsKKworICAgIG1fZGF0YS0+cCgpLT5zZXRD bGlwUmVjdChyZWN0LCBRdDo6SW50ZXJzZWN0Q2xpcCk7Cit9CisKIHZvaWQgR3JhcGhpY3NDb250 ZXh0OjpjbGlwKGNvbnN0IEZsb2F0UmVjdCYgcmVjdCkKIHsKICAgICBpZiAocGFpbnRpbmdEaXNh YmxlZCgpKQo=