870287 - DisplayItemClip::AddOffsetAndComputeDifference doesn't take pixel snapping into account

Status: NEW

(Core :: Layout, defect)

Description

[Matt Woodrow (:mattwoodrow)]

Attachment: Snap to pixels before comparing

This is causing unnecessary invalidations and buffer self-copying in some cases.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 747327 [details] [diff] [review]
Snap to pixels before comparing

Review of attachment 747327 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/src/nsRect.h
@@ +362,5 @@
>  }
>  
> +inline nsRect
> +nsIntRect::ScaleToAppUnits(float aXScale, float aYScale,
> +                           nscoord aAppUnitsPerPixel) const

Shouldn't this be called ScaleToAppUnitsInverse?

::: layout/base/DisplayItemClip.cpp
@@ +291,5 @@
> +             nscoord aAppUnitsPerDevPixel)
> +{
> +  nsIntRect pixelRect =
> +    aRect.ScaleToNearestPixels(aXScale, aYScale, aAppUnitsPerDevPixel);
> +  return pixelRect.ScaleToAppUnits(aXScale, aYScale, aAppUnitsPerDevPixel);

I don't really like this. The extra rounding caused by the conversion back to appunits could hurt us. Can we avoid that by comparing things in layer pixels and returning the difference in layer pixels?

Comment 2

[Markus Stange [:mstange]]

(In reply to Matt Woodrow (:mattwoodrow) from comment #0)
> This is causing unnecessary invalidations and buffer self-copying in some
> cases.

Do you have a testcase for this?

Comment 3

[Matt Woodrow (:mattwoodrow)]

I don't remember, sorry. The initial bug comment is unfortunately pretty useless here.