This happens when the device scale factor is non-integral, e.g. 1.25x.
Created attachment 312426 [details] Patch
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?
(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 :)
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?
(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.
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?
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?
Created attachment 359395 [details] Patch
(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!
rdar://problem/45269953
Comment on attachment 359395 [details] Patch Clearing flags on attachment: 359395 Committed r240131: <https://trac.webkit.org/changeset/240131>
Landed: https://github.com/WebKit/WebKit/commit/148c3c85ecc0fbdbfbc3016c8850dd53585fd215