[webkit-reviews] review requested: [Bug 86882] [chromium] Make sure that render surfaces are not pixel doubled with a device scale factor of 2 : [Attachment 148203] Patch

[bugzilla-daemon at webkit.org]

vollick at chromium.org has asked	for review:
Bug 86882: [chromium] Make sure that render surfaces are not pixel doubled with
a device scale factor of 2
https://bugs.webkit.org/show_bug.cgi?id=86882

Attachment 148203: Patch
https://bugs.webkit.org/attachment.cgi?id=148203&action=review

------- Additional Comments from vollick at chromium.org
(In reply to comment #26)
> Sorry to have sent you off on such a wrong path, but I've come around to
agreeing that calcDrawTransforms really needs to be operating in physical and
not logical pixels.
>
> If we somehow allowed floating point logical pixel clips in the compositor,
and then tried to convert them later to integer physical pixel clips, we'd run
into any number of floating point issues.  GL only operates in physical pixels
and its clip rects are ints, so in some ways it makes a lot of sense to operate
in that space directly.  And, it's not just clips and scissoring.  The culling
system also assumes that enclosingIntRect/enclosedIntRect on pixels rounds to
physical pixels.  These are all systems that expect exactness and it would be a
nightmare to make that work ok (or to fix bugs when it failed.)
>
> It would be helpful to have comments on LayerChromium.h describing what space
various members are in, e.g. bounds() is in logical pixels, drawTransform()
converts logical into physical pixels, transform() operates on logical pixels.
Cool, I've added some comments to LayerChromium. Please let me know if I should
add some more.
>
> I have a few worries that would make me feel better if you could investigate:
What space is hit testing and input in? Do they need to be converted to
physical pixel space? Are we correctly passing back scroll position from the
compositor thread in logical pixel space?

Hit testing and input are done in logical pixels, except for the impl thread
hit testing which is done in physical pixels due to the scaling of the matrices
in calcDrawTransforms. Scroll deltas appear to always be in logical pixels
(looking at CCLTHI::scrollBy).