CreateSamplingRestrictedDrawable called because of subpixel inaccuracies

NEW
Unassigned

Status

()

5 years ago
4 years ago

People

(Reporter: ali, Unassigned)

Tracking

(Depends on: 1 bug, {perf})

28 Branch
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(firefox28- affected)

Details

Attachments

(1 attachment)

(Reporter)

Description

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):

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?
(Reporter)

Updated

5 years ago
Keywords: perf
(Reporter)

Comment 1

5 years ago
* _are_ floats, not and floats
(Reporter)

Comment 2

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

Updated

5 years ago
Attachment #8373375 - Flags: feedback?(bas)
Status: UNCONFIRMED → NEW
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: ? → -
(Reporter)

Updated

5 years ago
Flags: needinfo?(nical.bugzilla)
Flags: needinfo?(matt.woodrow)

Updated

5 years ago
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.

Updated

5 years ago
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?
(Reporter)

Comment 6

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