970311 - CreateSamplingRestrictedDrawable called because of subpixel inaccuracies

Status: RESOLVED INVALID

(Core :: Graphics: Layers, defect)

Description

[Ali Ak]

When going through gfxUtils::DrawPixelSnapped we check to see if aImageRect contains aSourceRect and also if aSubimage rect contains aImageRect and if they do, we may hit the CreateSamplingRestrictedDrawable path. It seems like we hit the CreateSamplingRestrictedDrawable path because the Rect::Contains function returns false when for example an aSourceRect is (0, 0, 81, 90) and aImageRect is (-0.01, 0, 81, 90)

I did a small check with the profiler (I think I'm reading it right) but as it is, (I was loading the reftest layout/reftests/border-image/border-image-element.html repeatedly) I got 0.6% of time spent in DrawPixelSnapped attribute to calls to CreateSamplingRestrictedDrawable.

After clamping the x/y values in source rect and image rect before calling contains, the profiler doesn't show calls to CreateSamplingRestrictedDrawable anymore.

try seems all green (few things still running while I'm filing this):

try: https://tbpl.mozilla.org/?tree=Try&rev=c64e017a48e8

Is there a reason why the source and destination structures and floats all the way down to the back end draw calls? Shouldn't they be clamped at some point?

Comment 1

[Ali Ak]

* _are_ floats, not and floats

Comment 2

[Ali Ak]

Attachment: Avoid CreateSamplingRestrictedDrawable because of subpixel inaccuracies

Comment 3

[Lukas Blakk [:lsblakk] use ?needinfo]

Unless this is new to 28 or a spike in performance, there's nothing to drive this for a particular release, minusing for now.  Feel free to renominate if there's more here I'm not seeing.

Comment 4

[Nicolas Silva [:nical]]

Bas, could you have a quick look at this? it's blocking other work and look like a rather low hanging fruit with a patch just waiting for your feedback.

Comment 5

[Bas Schouten (:bas.schouten)]

Comment on attachment 8373375 [details] [diff] [review]
Avoid CreateSamplingRestrictedDrawable because of subpixel inaccuracies

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

Hrm, if aImageRect actually -is- significantly bigger than aSourceRect, won't this patch actually potentially let us see sampling artifacts? Or am I missing something?

Comment 6

[Ali Ak]

Hi,

How's that? It just rounds up fractions to whole numbers for the contains check no? I might not understand correctly (and I'm probably definitely missing something), but my understanding is that it doesn't make sense for pixels to have fractional values, and that aImageRect et al are pixel representations?

Comment 7

[Gregory Pappas [:gregp]]

CreateSamplingRestrictedDrawable was disabled in bug 1746662