CreateSamplingRestrictedDrawable called because of subpixel inaccuracies




5 years ago
4 years ago


(Reporter: ali, Unassigned)


(Depends on: 1 bug, {perf})

28 Branch
Mac OS X

Firefox Tracking Flags

(firefox28- affected)



(1 attachment)



5 years ago
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):


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?


5 years ago
Keywords: perf

Comment 1

5 years ago
* _are_ floats, not and floats

Comment 2

5 years ago
Created attachment 8373375 [details] [diff] [review]
Avoid CreateSamplingRestrictedDrawable because of subpixel inaccuracies


5 years ago
Attachment #8373375 - Flags: feedback?(bas)
status-firefox28: --- → affected
tracking-firefox28: --- → ?
Ever confirmed: true
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.
tracking-firefox28: ? → -


5 years ago
Flags: needinfo?(nical.bugzilla)
Flags: needinfo?(matt.woodrow)
Flags: needinfo?(nical.bugzilla) → needinfo?(bas)
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.
Attachment #8373375 - Flags: feedback?(bas) → feedback?(jmuizelaar)
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

5 years ago

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?
Flags: needinfo?(bas)
Depends on: 1073209
Flags: needinfo?(matt.woodrow)
No longer depends on: 1155249
You need to log in before you can comment on or make changes to this bug.