Closed
Bug 970311
Opened 11 years ago
Closed 2 years ago
CreateSamplingRestrictedDrawable called because of subpixel inaccuracies
Categories
(Core :: Graphics: Layers, defect)
Tracking
()
RESOLVED
INVALID
People
(Reporter: ali, Unassigned)
References
Details
(Keywords: perf)
Attachments
(1 file)
(deleted),
patch
|
Details | Diff | Splinter Review |
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?
Attachment #8373375 -
Flags: feedback?(bas)
Status: UNCONFIRMED → NEW
status-firefox28:
--- → affected
tracking-firefox28:
--- → ?
Ever confirmed: true
Comment 3•11 years ago
|
||
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.
Flags: needinfo?(nical.bugzilla)
Flags: needinfo?(matt.woodrow)
Updated•11 years ago
|
Flags: needinfo?(nical.bugzilla) → needinfo?(bas)
Comment 4•11 years ago
|
||
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•11 years ago
|
Attachment #8373375 -
Flags: feedback?(bas) → feedback?(jmuizelaar)
Comment 5•11 years ago
|
||
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?
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)
Updated•10 years ago
|
Flags: needinfo?(matt.woodrow)
Updated•2 years ago
|
Severity: normal → S3
Comment 7•2 years ago
|
||
CreateSamplingRestrictedDrawable was disabled in bug 1746662
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → INVALID
You need to log in
before you can comment on or make changes to this bug.
Description
•