173152 2017-06-09 04:41:07 -0700 [Win][HighDPI] Repaint glitches when scrolling. 2023-10-27 18:12:16 -0700 1 1 1 Unclassified WebKit WebKit Misc. WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=196339 InRadar P2 Normal --- 1 pvollan pvollan ahmad.saleem792 bfulgham commit-queue fujii webkit-bug-importer oldest_to_newest 1317354 0 pvollan 2017-06-09 04:41:07 -0700 This happens when the device scale factor is non-integral, e.g. 1.25x. 1317355 1 312426 pvollan 2017-06-09 04:43:07 -0700 Created attachment 312426 Patch 1317397 2 312426 bfulgham 2017-06-09 08:42:39 -0700 Comment on attachment 312426 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=312426&action=review > Source/WebKit/win/WebView.cpp:954 > + // FIXME: This is inefficient, we should be able to blit the scroll rectangle in this case as well. Is this due to a bug in the CA accelerated compositing code path? Or is there a bug in BitBlt in the Windows code? 1317412 3 pvollan 2017-06-09 09:01:24 -0700 (In reply to Brent Fulgham from comment #2) > Comment on attachment 312426 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=312426&action=review > > > Source/WebKit/win/WebView.cpp:954 > > + // FIXME: This is inefficient, we should be able to blit the scroll rectangle in this case as well. > > Is this due to a bug in the CA accelerated compositing code path? Or is > there a bug in BitBlt in the Windows code? I believe the problem is the conversion from logical scroll delta to scroll delta in pixel coordinates. When the scale factor is non-integral, this conversion is not correct, since we clamp the result. I would prefer a better fix than the current patch :) 1321190 4 312426 bfulgham 2017-06-20 12:38:02 -0700 Comment on attachment 312426 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=312426&action=review >>> Source/WebKit/win/WebView.cpp:954 >>> + // FIXME: This is inefficient, we should be able to blit the scroll rectangle in this case as well. >> >> Is this due to a bug in the CA accelerated compositing code path? Or is there a bug in BitBlt in the Windows code? > > I believe the problem is the conversion from logical scroll delta to scroll delta in pixel coordinates. When the scale factor is non-integral, this conversion is not correct, since we clamp the result. I would prefer a better fix than the current patch :) Would we be better off using some of the subpixel layout rounding functions Zalan created? Maybe if we took the ceiling of the calculated pixels we would always be drawing enough, without drawing too much? For example, what if instead of "clampTo<int>" we used std::ceil? Also, maybe we could adjust the clipRect like we do the scrollViewRect: FloatRect rawClipRect(logicalClipRect); rawClipRect.scale(scaleFactor); FloatRect clipRect = enclosingIntRect(rawClipRect); Or something like that? 1321200 5 pvollan 2017-06-20 12:48:16 -0700 (In reply to Brent Fulgham from comment #4) > Comment on attachment 312426 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=312426&action=review > > >>> Source/WebKit/win/WebView.cpp:954 > >>> + // FIXME: This is inefficient, we should be able to blit the scroll rectangle in this case as well. > >> > >> Is this due to a bug in the CA accelerated compositing code path? Or is there a bug in BitBlt in the Windows code? > > > > I believe the problem is the conversion from logical scroll delta to scroll delta in pixel coordinates. When the scale factor is non-integral, this conversion is not correct, since we clamp the result. I would prefer a better fix than the current patch :) > > Would we be better off using some of the subpixel layout rounding functions > Zalan created? Maybe if we took the ceiling of the calculated pixels we > would always be drawing enough, without drawing too much? > > For example, what if instead of "clampTo<int>" we used std::ceil? > > Also, maybe we could adjust the clipRect like we do the scrollViewRect: > > FloatRect rawClipRect(logicalClipRect); > rawClipRect.scale(scaleFactor); > FloatRect clipRect = enclosingIntRect(rawClipRect); > > Or something like that? Thank you, Brent! I will try out these ideas. 1340120 6 312426 bfulgham 2017-08-18 09:52:46 -0700 Comment on attachment 312426 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=312426&action=review >>>>> Source/WebKit/win/WebView.cpp:954 >>>>> + // FIXME: This is inefficient, we should be able to blit the scroll rectangle in this case as well. >>>> >>>> Is this due to a bug in the CA accelerated compositing code path? Or is there a bug in BitBlt in the Windows code? >>> >>> I believe the problem is the conversion from logical scroll delta to scroll delta in pixel coordinates. When the scale factor is non-integral, this conversion is not correct, since we clamp the result. I would prefer a better fix than the current patch :) >> >> Would we be better off using some of the subpixel layout rounding functions Zalan created? Maybe if we took the ceiling of the calculated pixels we would always be drawing enough, without drawing too much? >> >> For example, what if instead of "clampTo<int>" we used std::ceil? >> >> Also, maybe we could adjust the clipRect like we do the scrollViewRect: >> >> FloatRect rawClipRect(logicalClipRect); >> rawClipRect.scale(scaleFactor); >> FloatRect clipRect = enclosingIntRect(rawClipRect); >> >> Or something like that? > > Thank you, Brent! I will try out these ideas. Did any of those ideas work? 1495498 7 312426 bfulgham 2019-01-17 09:17:28 -0800 Comment on attachment 312426 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=312426&action=review >>>>>> Source/WebKit/win/WebView.cpp:954 >>>>>> + // FIXME: This is inefficient, we should be able to blit the scroll rectangle in this case as well. >>>>> >>>>> Is this due to a bug in the CA accelerated compositing code path? Or is there a bug in BitBlt in the Windows code? >>>> >>>> I believe the problem is the conversion from logical scroll delta to scroll delta in pixel coordinates. When the scale factor is non-integral, this conversion is not correct, since we clamp the result. I would prefer a better fix than the current patch :) >>> >>> Would we be better off using some of the subpixel layout rounding functions Zalan created? Maybe if we took the ceiling of the calculated pixels we would always be drawing enough, without drawing too much? >>> >>> For example, what if instead of "clampTo<int>" we used std::ceil? >>> >>> Also, maybe we could adjust the clipRect like we do the scrollViewRect: >>> >>> FloatRect rawClipRect(logicalClipRect); >>> rawClipRect.scale(scaleFactor); >>> FloatRect clipRect = enclosingIntRect(rawClipRect); >>> >>> Or something like that? >> >> Thank you, Brent! I will try out these ideas. > > Did any of those ideas work? I spoke with Per Arne in person. It doesn't look like the other ideas work, so we should proceed with this for now. Can you please change the FIME to reference a bugzilla that documents the need to improve the logic for this case? 1495565 8 359395 pvollan 2019-01-17 11:26:15 -0800 Created attachment 359395 Patch 1495625 9 pvollan 2019-01-17 13:20:24 -0800 (In reply to Brent Fulgham from comment #7) > Comment on attachment 312426 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=312426&action=review > > >>>>>> Source/WebKit/win/WebView.cpp:954 > >>>>>> + // FIXME: This is inefficient, we should be able to blit the scroll rectangle in this case as well. > >>>>> > >>>>> Is this due to a bug in the CA accelerated compositing code path? Or is there a bug in BitBlt in the Windows code? > >>>> > >>>> I believe the problem is the conversion from logical scroll delta to scroll delta in pixel coordinates. When the scale factor is non-integral, this conversion is not correct, since we clamp the result. I would prefer a better fix than the current patch :) > >>> > >>> Would we be better off using some of the subpixel layout rounding functions Zalan created? Maybe if we took the ceiling of the calculated pixels we would always be drawing enough, without drawing too much? > >>> > >>> For example, what if instead of "clampTo<int>" we used std::ceil? > >>> > >>> Also, maybe we could adjust the clipRect like we do the scrollViewRect: > >>> > >>> FloatRect rawClipRect(logicalClipRect); > >>> rawClipRect.scale(scaleFactor); > >>> FloatRect clipRect = enclosingIntRect(rawClipRect); > >>> > >>> Or something like that? > >> > >> Thank you, Brent! I will try out these ideas. > > > > Did any of those ideas work? > > I spoke with Per Arne in person. It doesn't look like the other ideas work, > so we should proceed with this for now. > > Can you please change the FIME to reference a bugzilla that documents the > need to improve the logic for this case? I have filed https://bugs.webkit.org/show_bug.cgi?id=193542. Thanks for reviewing! 1495627 10 pvollan 2019-01-17 13:22:39 -0800 rdar://problem/45269953 1495641 11 359395 commit-queue 2019-01-17 13:45:13 -0800 Comment on attachment 359395 Patch Clearing flags on attachment: 359395 Committed r240131: <https://trac.webkit.org/changeset/240131> 1988159 12 ahmad.saleem792 2023-10-27 18:12:16 -0700 Landed: https://github.com/WebKit/WebKit/commit/148c3c85ecc0fbdbfbc3016c8850dd53585fd215 312426 2017-06-09 04:43:07 -0700 2019-01-17 09:17:28 -0800 Patch bug-173152-20170609044307.patch text/plain 2205 pvollan SW5kZXg6IFNvdXJjZS9XZWJLaXQvd2luL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2Uv V2ViS2l0L3dpbi9DaGFuZ2VMb2cJKHJldmlzaW9uIDIxNzk3NykKKysrIFNvdXJjZS9XZWJLaXQv d2luL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE3IEBACisyMDE3LTA2LTA5 ICBQZXIgQXJuZSBWb2xsYW4gIDxwdm9sbGFuQGFwcGxlLmNvbT4KKworICAgICAgICBbV2luXVtI aWdoRFBJXSBSZXBhaW50IGdsaXRjaGVzIHdoZW4gc2Nyb2xsaW5nLgorICAgICAgICBodHRwczov L2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTczMTUyCisKKyAgICAgICAgUmV2aWV3 ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgTm9uLWludGVncmFsIGRldmljZSBzY2Fs ZSBmYWN0b3JzIGFyZSBjYXVzaW5nIHJlcGFpbnQgZ2xpdGNoZXMsIGJlY2F1c2UgdGhlIGNvbXB1 dGF0aW9uIG9mIHRoZSBzY3JvbGwKKyAgICAgICAgZGVsdGEgaW4gcGl4ZWwgY29vcmRpbmF0ZXMg ZnJvbSB0aGUgc2Nyb2xsIGRlbHRhIGluIGxvZ2ljYWwgY29vcmRpbmF0ZXMgd2lsbCBub3QgYWx3 YXlzIGJlIGNvcnJlY3QuCisgICAgICAgIEluc3RlYWQgb2YgYmxpdHRpbmcgdGhlIHNjcm9sbCBy ZWN0YW5nbGUsIHJlcGFpbnQgdGhlIGVudGlyZSByZWdpb24gYWZmZWN0ZWQgYnkgc2Nyb2xsaW5n LgorCisgICAgICAgICogV2ViVmlldy5jcHA6CisgICAgICAgIChXZWJWaWV3OjpzY3JvbGxCYWNr aW5nU3RvcmUpOgorCiAyMDE3LTA2LTA0ICBLb25zdGFudGluIFRva2FyZXYgIDxhbm51bGVuQHlh bmRleC5ydT4KIAogICAgICAgICBGaXggYnVpbGQgb2YgV2luZG93cy1zcGVjaWZpYyBjb2RlIHdp dGggSUNVIDU5LjEKSW5kZXg6IFNvdXJjZS9XZWJLaXQvd2luL1dlYlZpZXcuY3BwCj09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT0KLS0tIFNvdXJjZS9XZWJLaXQvd2luL1dlYlZpZXcuY3BwCShyZXZpc2lvbiAyMTc4NDYpCisr KyBTb3VyY2UvV2ViS2l0L3dpbi9XZWJWaWV3LmNwcAkod29ya2luZyBjb3B5KQpAQCAtOTQ3LDYg Kzk0NywxOCBAQCB2b2lkIFdlYlZpZXc6OmFkZFRvRGlydHlSZWdpb24oR0RJT2JqZWN0CiAKIHZv aWQgV2ViVmlldzo6c2Nyb2xsQmFja2luZ1N0b3JlKEZyYW1lVmlldyogZnJhbWVWaWV3LCBpbnQg bG9naWNhbER4LCBpbnQgbG9naWNhbER5LCBjb25zdCBJbnRSZWN0JiBsb2dpY2FsU2Nyb2xsVmll d1JlY3QsIGNvbnN0IEludFJlY3QmIGxvZ2ljYWxDbGlwUmVjdCkKIHsKKyAgICBpZiAoZGV2aWNl U2NhbGVGYWN0b3IoKSAhPSBzdGF0aWNfY2FzdDxpbnQ+KGRldmljZVNjYWxlRmFjdG9yKCkpKSB7 CisgICAgICAgIC8vIE5vbi1pbnRlZ3JhbCBkZXZpY2Ugc2NhbGUgZmFjdG9ycyBhcmUgY2F1c2lu ZyByZXBhaW50IGdsaXRjaGVzLCBiZWNhdXNlIHRoZSBjb21wdXRhdGlvbiBvZiB0aGUgc2Nyb2xs CisgICAgICAgIC8vIGRlbHRhIGluIHBpeGVsIGNvb3JkaW5hdGVzIGZyb20gdGhlIHNjcm9sbCBk ZWx0YSBpbiBsb2dpY2FsIGNvb3JkaW5hdGVzIHdpbGwgbm90IGFsd2F5cyBiZSBjb3JyZWN0Lgor ICAgICAgICAvLyBJbnN0ZWFkIG9mIGJsaXR0aW5nIHRoZSBzY3JvbGwgcmVjdGFuZ2xlLCByZXBh aW50IHRoZSBlbnRpcmUgcmVnaW9uIGFmZmVjdGVkIGJ5IHNjcm9sbGluZy4KKyAgICAgICAgLy8g RklYTUU6IFRoaXMgaXMgaW5lZmZpY2llbnQsIHdlIHNob3VsZCBiZSBhYmxlIHRvIGJsaXQgdGhl IHNjcm9sbCByZWN0YW5nbGUgaW4gdGhpcyBjYXNlIGFzIHdlbGwuCisgICAgICAgIEludFJlY3Qg cmVwYWludFJlY3QgPSBsb2dpY2FsU2Nyb2xsVmlld1JlY3Q7CisgICAgICAgIHJlcGFpbnRSZWN0 Lm1vdmUobG9naWNhbER4LCBsb2dpY2FsRHkpOworICAgICAgICByZXBhaW50UmVjdC51bml0ZShs b2dpY2FsU2Nyb2xsVmlld1JlY3QpOworICAgICAgICByZXBhaW50KHJlcGFpbnRSZWN0LCB0cnVl KTsKKyAgICAgICAgcmV0dXJuOworICAgIH0KKwogICAgIG1fbmVlZHNEaXNwbGF5ID0gdHJ1ZTsK IAogICAgIC8vIERpbWVuc2lvbnMgcGFzc2VkIHRvIHVzIGZyb20gV2ViQ29yZSBhcmUgaW4gbG9n aWNhbCB1bml0cy4gV2UgbXVzdCBjb252ZXJ0IHRvIHBpeGVsczoK 359395 2019-01-17 11:26:15 -0800 2019-01-17 13:45:13 -0800 Patch bug-173152-20190117112614.patch text/plain 2354 pvollan SW5kZXg6IFNvdXJjZS9XZWJLaXRMZWdhY3kvd2luL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBT b3VyY2UvV2ViS2l0TGVnYWN5L3dpbi9DaGFuZ2VMb2cJKHJldmlzaW9uIDI0MDEyMCkKKysrIFNv dXJjZS9XZWJLaXRMZWdhY3kvd2luL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsx LDE4IEBACisyMDE5LTAxLTE3ICBQZXIgQXJuZSBWb2xsYW4gIDxwdm9sbGFuQGFwcGxlLmNvbT4K KworICAgICAgICBbV2luXVtIaWdoRFBJXSBSZXBhaW50IGdsaXRjaGVzIHdoZW4gc2Nyb2xsaW5n LgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTczMTUy CisgICAgICAgIDxyZGFyOi8vcHJvYmxlbS80NTI2OTk1Mz4KKworICAgICAgICBSZXZpZXdlZCBi eSBCcmVudCBGdWxnaGFtLgorCisgICAgICAgIE5vbi1pbnRlZ3JhbCBkZXZpY2Ugc2NhbGUgZmFj dG9ycyBhcmUgY2F1c2luZyByZXBhaW50IGdsaXRjaGVzLCBiZWNhdXNlIHRoZSBjb21wdXRhdGlv biBvZiB0aGUgc2Nyb2xsCisgICAgICAgIGRlbHRhIGluIHBpeGVsIGNvb3JkaW5hdGVzIGZyb20g dGhlIHNjcm9sbCBkZWx0YSBpbiBsb2dpY2FsIGNvb3JkaW5hdGVzIHdpbGwgbm90IGFsd2F5cyBi ZSBjb3JyZWN0LgorICAgICAgICBJbnN0ZWFkIG9mIGJsaXR0aW5nIHRoZSBzY3JvbGwgcmVjdGFu Z2xlLCByZXBhaW50IHRoZSBlbnRpcmUgcmVnaW9uIGFmZmVjdGVkIGJ5IHNjcm9sbGluZy4KKwor ICAgICAgICAqIFdlYlZpZXcuY3BwOgorICAgICAgICAoV2ViVmlldzo6c2Nyb2xsQmFja2luZ1N0 b3JlKToKKwogMjAxOS0wMS0xNyAgQWxleCBDaHJpc3RlbnNlbiAgPGFjaHJpc3RlbnNlbkB3ZWJr aXQub3JnPgogCiAgICAgICAgIFN0b3AgdXNpbmcgTmV0d29ya1N0b3JhZ2VTZXNzaW9uOjpzdG9y YWdlU2Vzc2lvbiBpbiBXZWJDb3JlCkluZGV4OiBTb3VyY2UvV2ViS2l0TGVnYWN5L3dpbi9XZWJW aWV3LmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViS2l0TGVnYWN5L3dpbi9XZWJWaWV3LmNw cAkocmV2aXNpb24gMjQwMTE3KQorKysgU291cmNlL1dlYktpdExlZ2FjeS93aW4vV2ViVmlldy5j cHAJKHdvcmtpbmcgY29weSkKQEAgLTk1Myw2ICs5NTMsMTkgQEAgdm9pZCBXZWJWaWV3OjphZGRU b0RpcnR5UmVnaW9uKEdESU9iamVjdAogCiB2b2lkIFdlYlZpZXc6OnNjcm9sbEJhY2tpbmdTdG9y ZShGcmFtZVZpZXcqIGZyYW1lVmlldywgaW50IGxvZ2ljYWxEeCwgaW50IGxvZ2ljYWxEeSwgY29u c3QgSW50UmVjdCYgbG9naWNhbFNjcm9sbFZpZXdSZWN0LCBjb25zdCBJbnRSZWN0JiBsb2dpY2Fs Q2xpcFJlY3QpCiB7CisgICAgaWYgKGRldmljZVNjYWxlRmFjdG9yKCkgIT0gc3RhdGljX2Nhc3Q8 aW50PihkZXZpY2VTY2FsZUZhY3RvcigpKSkgeworICAgICAgICAvLyBOb24taW50ZWdyYWwgZGV2 aWNlIHNjYWxlIGZhY3RvcnMgYXJlIGNhdXNpbmcgcmVwYWludCBnbGl0Y2hlcywgYmVjYXVzZSB0 aGUgY29tcHV0YXRpb24gb2YgdGhlIHNjcm9sbAorICAgICAgICAvLyBkZWx0YSBpbiBwaXhlbCBj b29yZGluYXRlcyBmcm9tIHRoZSBzY3JvbGwgZGVsdGEgaW4gbG9naWNhbCBjb29yZGluYXRlcyB3 aWxsIG5vdCBhbHdheXMgYmUgY29ycmVjdC4KKyAgICAgICAgLy8gSW5zdGVhZCBvZiBibGl0dGlu ZyB0aGUgc2Nyb2xsIHJlY3RhbmdsZSwgcmVwYWludCB0aGUgZW50aXJlIHJlZ2lvbiBhZmZlY3Rl ZCBieSBzY3JvbGxpbmcuCisgICAgICAgIC8vIEZJWE1FOiBUaGlzIGlzIGluZWZmaWNpZW50LCB3 ZSBzaG91bGQgYmUgYWJsZSB0byBibGl0IHRoZSBzY3JvbGwgcmVjdGFuZ2xlIGluIHRoaXMgY2Fz ZSBhcyB3ZWxsLAorICAgICAgICAvLyBzZWUgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19i dWcuY2dpP2lkPTE5MzU0Mi4KKyAgICAgICAgSW50UmVjdCByZXBhaW50UmVjdCA9IGxvZ2ljYWxT Y3JvbGxWaWV3UmVjdDsKKyAgICAgICAgcmVwYWludFJlY3QubW92ZShsb2dpY2FsRHgsIGxvZ2lj YWxEeSk7CisgICAgICAgIHJlcGFpbnRSZWN0LnVuaXRlKGxvZ2ljYWxTY3JvbGxWaWV3UmVjdCk7 CisgICAgICAgIHJlcGFpbnQocmVwYWludFJlY3QsIHRydWUpOworICAgICAgICByZXR1cm47Cisg ICAgfQorCiAgICAgbV9uZWVkc0Rpc3BsYXkgPSB0cnVlOwogCiAgICAgLy8gRGltZW5zaW9ucyBw YXNzZWQgdG8gdXMgZnJvbSBXZWJDb3JlIGFyZSBpbiBsb2dpY2FsIHVuaXRzLiBXZSBtdXN0IGNv bnZlcnQgdG8gcGl4ZWxzOgo=