On mobile, use nearest-neighbor interpolation when drawing BasicThebesLayers with a scale transform

RESOLVED FIXED

Status

()

defect
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: dougt, Assigned: dougt)

Tracking

Trunk
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

No description provided.
Posted patch patch v.1Splinter Review
Assignee: nobody → doug.turner
Attachment #472691 - Flags: review?(jones.chris.g)
I don't think we want to see |ifdef MOZ_GFX_OPTIMIZE_MOBILE| code in gfx/layers/*, but I'm CC'ing the sr's in this neighborhood for feedback.  I think I'd only want to take this patch as an eleventh-hour fix before a release or blocking a followup bug to clean up the logic.  We're going to need to be able to twiddle *Canvas*Layer and *Image*Layer in the same way.

It's not 100% clear to me where the code to decide the filter algorithm should live.  We might be able to get fast bilinear interpolation on mobile GPUs, so the default setting might be both platform- and layer-manager-specific.  On the other hand, on mobile it seems wise to use nearest-neighbor for *all* gfxPatterns that don't explicitly set their filter.  roc tells that me that the code that actually cares about its scaling algorithm should be setting the filter explicitly already.  The former seems to indicate a LayerManager interface, and the latter seems to indicate a gfxPlatform interface.

I'm now thinking maybe should file a followup bug to sort out the "right" interface for default interpolation algorithm and go ahead and land this on cedar.
Summary: Use nearest neighbor filter when MOZ_GFX_OPTIMIZE_MOBILE is set → On mobile, use nearest-neighbor interpolation when drawing BasicThebesLayers with a scale transform
FYI nsLayoutUtils::GetGraphicsFilterForFrame currently has a MOZ_GFX_OPTIMIZE_MOBILE #ifdef that makes image-rendering:auto use FILTER_NEAREST. That gets handed down to CanvasLayer and ImageLayer already.

I'd be fine with Doug's patch moved to BasicThebesLayer. I don't think we want it for GL layers ... on GL we should be fine with blinear. If not, we can duplicate doug's patch there, possibly making it conditional on the GPU type.
Comment on attachment 472691 [details] [diff] [review]
patch v.1

Easiest way to localize this to BasicLayers.cpp is probably to
 - make |virtual gfxFilter ThebesLayer::Filter()| that returns LINEAR by default.
 - in BasicThebesLayer, override |Filter()| to return NEAREST |ifdef MOZ_OPTIMIZE_MOBILE|

This patch works great on device, BTW.  Nice work!
er, why not just pass gfxPattern::FILTER_NEAREST to SetFilter() directly?  (Also I thought that a layer already has a filter property, can't mobile just override/honor that?)
Layer doesn't, no.

But that's a point ... DrawBufferWithRotation is only used for ThebesBuffer-->ThebesBuffer copies, for which no scaling is used, and for ThebesBuffer-->screen drawing in BasicLayers.  ThebesLayerOGL has an entirely different rendering path -->screen.

roc, this patch is already BasicLayers-only, sort of, so I'm ready to land it if you're OK with it.
Comment on attachment 472691 [details] [diff] [review]
patch v.1

OK, this is fine then.
Attachment #472691 - Flags: review?(jones.chris.g) → review+
http://hg.mozilla.org/projects/cedar/rev/44273fae4f96
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.