163988 2016-10-25 14:44:20 -0700 [Win][Direct2D] Implement GraphicsContext::releaseWindowsContext. 2016-10-27 15:51:08 -0700 1 1 1 Unclassified WebKit WebCore Misc. WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED P2 Normal --- 163898 1 pvollan pvollan achristensen bfulgham commit-queue oldest_to_newest 1244455 0 pvollan 2016-10-25 14:44:20 -0700 This is needed to draw native controls. 1244466 1 292828 pvollan 2016-10-25 14:53:34 -0700 Created attachment 292828 Patch 1244467 2 commit-queue 2016-10-25 14:55:36 -0700 Attachment 292828 did not pass style-queue: ERROR: Source/WebCore/platform/graphics/win/GraphicsContextDirect2D.cpp:255: Use the class HWndDC instead of calling GetDC to avoid potential memory leaks. [runtime/leaky_pattern] [5] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style. 1244630 3 292828 bfulgham 2016-10-25 22:07:58 -0700 Comment on attachment 292828 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=292828&action=review Looks good! After this lands I'll incorporate into my patch. > Source/WebCore/platform/graphics/win/GraphicsContextDirect2D.cpp:248 > + HRESULT hr = platformContext()->QueryInterface(__uuidof(ID2D1GdiInteropRenderTarget), (void**)&gdiRenderTarget); I have a patch to reuse the GDI Interop target. I'll update it to include this code path. > Source/WebCore/platform/graphics/win/GraphicsContextDirect2D.cpp:252 > + platformContext()->BeginDraw(); I also have a patch with an RIAA class that will get rid of some of this boilerplate. 1245233 4 293049 pvollan 2016-10-27 13:31:51 -0700 Created attachment 293049 Patch 1245287 5 293049 bfulgham 2016-10-27 15:07:12 -0700 Comment on attachment 293049 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=293049&action=review Very nice! This should get rid of at least one expensive copy! > Source/WebCore/platform/graphics/win/GraphicsContextDirect2D.cpp:252 > + D2D1_BITMAP_PROPERTIES bitmapProperties = D2D1::BitmapProperties(D2D1::PixelFormat(DXGI_FORMAT_B8G8R8A8_UNORM, D2D1_ALPHA_MODE_IGNORE)); Use 'auto' here, please. > Source/WebCore/platform/graphics/win/GraphicsContextDirect2D.cpp:255 > + HRESULT hr = platformContext()->CreateBitmap(pixelData.size(), pixelData.buffer(), pixelData.bytesPerRow(), &bitmapProperties, &bitmap); We should probably ASSERT(SUCCEEDED(hr)) here. 1245310 6 pvollan 2016-10-27 15:42:04 -0700 Committed r208013: <https://trac.webkit.org/changeset/208013> 1245314 7 293049 sabouhallawa 2016-10-27 15:49:31 -0700 Comment on attachment 293049 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=293049&action=review > Source/WebCore/platform/graphics/win/GraphicsContextDirect2D.cpp:263 > + if (!didBeginDraw()) > + hr = platformContext()->EndDraw(); I think this condition is wrong. It can be the opposite: if (didBeginDraw()) hr = platformContext()->EndDraw(); But I think this is not accurate either. Since you need to endDraw() only if you beginDraw(), I think you need to track this by flag in the beginDraw if-statetmen abovet. Otherwise you may cause unbalanced draw begin-end nesting. 1245317 8 293049 bfulgham 2016-10-27 15:51:08 -0700 Comment on attachment 293049 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=293049&action=review >> Source/WebCore/platform/graphics/win/GraphicsContextDirect2D.cpp:263 >> + hr = platformContext()->EndDraw(); > > I think this condition is wrong. It can be the opposite: > > if (didBeginDraw()) > hr = platformContext()->EndDraw(); > > But I think this is not accurate either. Since you need to endDraw() only if you beginDraw(), I think you need to track this by flag in the beginDraw if-statetmen abovet. Otherwise you may cause unbalanced draw begin-end nesting. This whole issue will be fixed by Bug 164005. 292828 2016-10-25 14:53:34 -0700 2016-10-27 13:31:48 -0700 Patch bug-163988-20161025145418.patch text/plain 2446 pvollan SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDIwNzg0NikKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE1IEBACisyMDE2LTEwLTI1ICBQZXIgQXJu ZSBWb2xsYW4gIDxwdm9sbGFuQGFwcGxlLmNvbT4KKworICAgICAgICBbV2luXVtEaXJlY3QyRF0g SW1wbGVtZW50IEdyYXBoaWNzQ29udGV4dDo6cmVsZWFzZVdpbmRvd3NDb250ZXh0LgorICAgICAg ICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTYzOTg4CisKKyAgICAg ICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgVGhpcyBtZXRob2QgaXMg bmVlZGVkIHRvIGRyYXcgbmF0aXZlIGNvbnRyb2xzLgorCisgICAgICAgICogcGxhdGZvcm0vZ3Jh cGhpY3Mvd2luL0dyYXBoaWNzQ29udGV4dERpcmVjdDJELmNwcDoKKyAgICAgICAgKFdlYkNvcmU6 OkdyYXBoaWNzQ29udGV4dDo6cmVsZWFzZVdpbmRvd3NDb250ZXh0KToKKwogMjAxNi0xMC0yNSAg QnJhZHkgRWlkc29uICA8YmVpZHNvbkBhcHBsZS5jb20+CiAKICAgICAgICAgSW5kZXhlZERCIDIu MDogU3VwcG9ydCBJREJPYmplY3RTdG9yZSBvcGVuS2V5Q3Vyc29yLgpJbmRleDogU291cmNlL1dl YkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3Mvd2luL0dyYXBoaWNzQ29udGV4dERpcmVjdDJELmNwcAo9 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy93aW4vR3JhcGhp Y3NDb250ZXh0RGlyZWN0MkQuY3BwCShyZXZpc2lvbiAyMDc3ODMpCisrKyBTb3VyY2UvV2ViQ29y ZS9wbGF0Zm9ybS9ncmFwaGljcy93aW4vR3JhcGhpY3NDb250ZXh0RGlyZWN0MkQuY3BwCSh3b3Jr aW5nIGNvcHkpCkBAIC0yMzUsNiArMjM1LDQwIEBAIHZvaWQgR3JhcGhpY3NDb250ZXh0OjpkcmF3 TmF0aXZlSW1hZ2UoY28KIAogdm9pZCBHcmFwaGljc0NvbnRleHQ6OnJlbGVhc2VXaW5kb3dzQ29u dGV4dChIREMgaGRjLCBjb25zdCBJbnRSZWN0JiBkc3RSZWN0LCBib29sIHN1cHBvcnRBbHBoYUJs ZW5kLCBib29sIG1heUNyZWF0ZUJpdG1hcCkKIHsKKyAgICBib29sIGNyZWF0ZWRCaXRtYXAgPSBt YXlDcmVhdGVCaXRtYXAgJiYgKCFtX2RhdGEtPm1faGRjIHx8IGlzSW5UcmFuc3BhcmVuY3lMYXll cigpKTsKKyAgICBpZiAoIWNyZWF0ZWRCaXRtYXApIHsKKyAgICAgICAgbV9kYXRhLT5yZXN0b3Jl KCk7CisgICAgICAgIHJldHVybjsKKyAgICB9CisKKyAgICBpZiAoIWhkYyB8fCBkc3RSZWN0Lmlz RW1wdHkoKSkKKyAgICAgICAgcmV0dXJuOworCisgICAgQ09NUHRyPElEMkQxR2RpSW50ZXJvcFJl bmRlclRhcmdldD4gZ2RpUmVuZGVyVGFyZ2V0OworICAgIEhSRVNVTFQgaHIgPSBwbGF0Zm9ybUNv bnRleHQoKS0+UXVlcnlJbnRlcmZhY2UoX191dWlkb2YoSUQyRDFHZGlJbnRlcm9wUmVuZGVyVGFy Z2V0KSwgKHZvaWQqKikmZ2RpUmVuZGVyVGFyZ2V0KTsKKyAgICBBU1NFUlQoaHIgPT0gU19PSyk7 CisKKyAgICBpZiAoIWRpZEJlZ2luRHJhdygpKQorICAgICAgICBwbGF0Zm9ybUNvbnRleHQoKS0+ QmVnaW5EcmF3KCk7CisKKyAgICBIREMgZHN0SGRjID0gbnVsbHB0cjsKKyAgICBociA9IGdkaVJl bmRlclRhcmdldC0+R2V0REMoRDJEMV9EQ19JTklUSUFMSVpFX01PREVfQ09QWSwgJmRzdEhkYyk7 CisgICAgQVNTRVJUKGhyID09IFNfT0spOworCisgICAgRDJEMTo6TWF0cml4M3gyRiBjdXJyZW50 VHJhbnNmb3JtOworICAgIHBsYXRmb3JtQ29udGV4dCgpLT5HZXRUcmFuc2Zvcm0oJmN1cnJlbnRU cmFuc2Zvcm0pOworICAgIGRvdWJsZSBvZmZzZXRYID0gY3VycmVudFRyYW5zZm9ybS5fMzE7Cisg ICAgZG91YmxlIG9mZnNldFkgPSBjdXJyZW50VHJhbnNmb3JtLl8zMjsKKworICAgIEJPT0wgb2sg PSA6OkJpdEJsdChkc3RIZGMsIGRzdFJlY3QueCgpICsgb2Zmc2V0WCwgZHN0UmVjdC55KCkgKyBv ZmZzZXRZLCBkc3RSZWN0LndpZHRoKCksIGRzdFJlY3QuaGVpZ2h0KCksIGhkYywgZHN0UmVjdC54 KCksIGRzdFJlY3QueSgpLCBTUkNDT1BZKTsKKyAgICBBU1NFUlQob2spOworCisgICAgaHIgPSBn ZGlSZW5kZXJUYXJnZXQtPlJlbGVhc2VEQyhudWxscHRyKTsKKworICAgIGlmICghZGlkQmVnaW5E cmF3KCkpCisgICAgICAgIGhyID0gcGxhdGZvcm1Db250ZXh0KCktPkVuZERyYXcoKTsKKworICAg IDo6RGVsZXRlREMoaGRjKTsKIH0KIAogdm9pZCBHcmFwaGljc0NvbnRleHQ6OmRyYXdXaW5kb3dz Qml0bWFwKFdpbmRvd3NCaXRtYXAqIGltYWdlLCBjb25zdCBJbnRQb2ludCYgcG9pbnQpCg== 293049 2016-10-27 13:31:51 -0700 2016-10-27 15:07:12 -0700 Patch bug-163988-20161027133227.patch text/plain 2287 pvollan SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDIwNzg0NikKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE1IEBACisyMDE2LTEwLTI1ICBQZXIgQXJu ZSBWb2xsYW4gIDxwdm9sbGFuQGFwcGxlLmNvbT4KKworICAgICAgICBbV2luXVtEaXJlY3QyRF0g SW1wbGVtZW50IEdyYXBoaWNzQ29udGV4dDo6cmVsZWFzZVdpbmRvd3NDb250ZXh0LgorICAgICAg ICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTYzOTg4CisKKyAgICAg ICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgVGhpcyBtZXRob2QgaXMg bmVlZGVkIHRvIGRyYXcgbmF0aXZlIGNvbnRyb2xzLgorCisgICAgICAgICogcGxhdGZvcm0vZ3Jh cGhpY3Mvd2luL0dyYXBoaWNzQ29udGV4dERpcmVjdDJELmNwcDoKKyAgICAgICAgKFdlYkNvcmU6 OkdyYXBoaWNzQ29udGV4dDo6cmVsZWFzZVdpbmRvd3NDb250ZXh0KToKKwogMjAxNi0xMC0yNSAg QnJhZHkgRWlkc29uICA8YmVpZHNvbkBhcHBsZS5jb20+CiAKICAgICAgICAgSW5kZXhlZERCIDIu MDogU3VwcG9ydCBJREJPYmplY3RTdG9yZSBvcGVuS2V5Q3Vyc29yLgpJbmRleDogU291cmNlL1dl YkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3Mvd2luL0dyYXBoaWNzQ29udGV4dERpcmVjdDJELmNwcAo9 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy93aW4vR3JhcGhp Y3NDb250ZXh0RGlyZWN0MkQuY3BwCShyZXZpc2lvbiAyMDc3ODMpCisrKyBTb3VyY2UvV2ViQ29y ZS9wbGF0Zm9ybS9ncmFwaGljcy93aW4vR3JhcGhpY3NDb250ZXh0RGlyZWN0MkQuY3BwCSh3b3Jr aW5nIGNvcHkpCkBAIC0yMzUsNiArMjM1LDM0IEBAIHZvaWQgR3JhcGhpY3NDb250ZXh0OjpkcmF3 TmF0aXZlSW1hZ2UoY28KIAogdm9pZCBHcmFwaGljc0NvbnRleHQ6OnJlbGVhc2VXaW5kb3dzQ29u dGV4dChIREMgaGRjLCBjb25zdCBJbnRSZWN0JiBkc3RSZWN0LCBib29sIHN1cHBvcnRBbHBoYUJs ZW5kLCBib29sIG1heUNyZWF0ZUJpdG1hcCkKIHsKKyAgICBib29sIGNyZWF0ZWRCaXRtYXAgPSBt YXlDcmVhdGVCaXRtYXAgJiYgKCFtX2RhdGEtPm1faGRjIHx8IGlzSW5UcmFuc3BhcmVuY3lMYXll cigpKTsKKyAgICBpZiAoIWNyZWF0ZWRCaXRtYXApIHsKKyAgICAgICAgbV9kYXRhLT5yZXN0b3Jl KCk7CisgICAgICAgIHJldHVybjsKKyAgICB9CisKKyAgICBpZiAoIWhkYyB8fCBkc3RSZWN0Lmlz RW1wdHkoKSkKKyAgICAgICAgcmV0dXJuOworCisgICAgYXV0byBzb3VyY2VCaXRtYXAgPSBhZG9w dEdESU9iamVjdChzdGF0aWNfY2FzdDxIQklUTUFQPig6OkdldEN1cnJlbnRPYmplY3QoaGRjLCBP QkpfQklUTUFQKSkpOworCisgICAgRElCUGl4ZWxEYXRhIHBpeGVsRGF0YShzb3VyY2VCaXRtYXAu Z2V0KCkpOworICAgIEFTU0VSVChwaXhlbERhdGEuYml0c1BlclBpeGVsKCkgPT0gMzIpOworCisg ICAgRDJEMV9CSVRNQVBfUFJPUEVSVElFUyBiaXRtYXBQcm9wZXJ0aWVzID0gRDJEMTo6Qml0bWFw UHJvcGVydGllcyhEMkQxOjpQaXhlbEZvcm1hdChEWEdJX0ZPUk1BVF9COEc4UjhBOF9VTk9STSwg RDJEMV9BTFBIQV9NT0RFX0lHTk9SRSkpOworCisgICAgQ09NUHRyPElEMkQxQml0bWFwPiBiaXRt YXA7CisgICAgSFJFU1VMVCBociA9IHBsYXRmb3JtQ29udGV4dCgpLT5DcmVhdGVCaXRtYXAocGl4 ZWxEYXRhLnNpemUoKSwgcGl4ZWxEYXRhLmJ1ZmZlcigpLCBwaXhlbERhdGEuYnl0ZXNQZXJSb3co KSwgJmJpdG1hcFByb3BlcnRpZXMsICZiaXRtYXApOworCisgICAgaWYgKCFkaWRCZWdpbkRyYXco KSkKKyAgICAgICAgcGxhdGZvcm1Db250ZXh0KCktPkJlZ2luRHJhdygpOworCisgICAgcGxhdGZv cm1Db250ZXh0KCktPkRyYXdCaXRtYXAoYml0bWFwLmdldCgpLCBkc3RSZWN0KTsKKworICAgIGlm ICghZGlkQmVnaW5EcmF3KCkpCisgICAgICAgIGhyID0gcGxhdGZvcm1Db250ZXh0KCktPkVuZERy YXcoKTsKKworICAgIDo6RGVsZXRlREMoaGRjKTsKIH0KIAogdm9pZCBHcmFwaGljc0NvbnRleHQ6 OmRyYXdXaW5kb3dzQml0bWFwKFdpbmRvd3NCaXRtYXAqIGltYWdlLCBjb25zdCBJbnRQb2ludCYg cG9pbnQpCg==