Closed Bug 656797 Opened 9 years ago Closed 8 months ago

Use bilinear scaling for pinch zoom in Fennec if ARM NEON is available

Categories

(Firefox for Android Graveyard :: Panning/Zooming, enhancement)

ARM
Android
enhancement
Not set

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: siarhei.siamashka, Unassigned)

References

Details

(Keywords: mobile, Whiteboard: [testday-2011-06-24])

Attachments

(2 files)

User-Agent:       Mozilla/5.0 (X11; Linux x86_64; rv:2.0.1) Gecko/20110506 Firefox/4.0.1
Build Identifier: 

Zooming looks a bit nicer with bilinear scaling instead of nearest. ARM NEON optimizations make bilinear scaling much faster and maybe even usable for scaling something in realtime such as pinch zoom.

Reproducible: Always
A patch for testing. When using it on Galaxy Tab (having a relatively high 1024x600 screen resolution), pinch zoom is more or less ok in my opinion, even though it is not totally smooth.

It is worth mentioning that bilinear scaling optimizations are relatively new in pixman and more performance improvements are still possible. Just introducing NEON optimizations made bilinear scaling ~10x faster for a number of compositing operations. But r5g6b5->r5g6b5 bilinear scaling used for pinch zoom has a potential for an additional up to ~2x speedup. So even if bilinear scaling is not perfectly suited for pinch zoom right now, it still has a good potential for improvement, assuming that some efforts are actually put into making it happen.

PS. I'm not particularly fond of having to use #ifdef MOZILLA_MAY_SUPPORT_NEON
Attachment #532068 - Flags: review?
Depends on: 656782
Keywords: mobile
Hardware: Other → ARM
Attachment #532068 - Flags: review? → review?(roc)
Attachment #532068 - Flags: review?(roc)
Attachment #532068 - Flags: review?(blassey.bugs)
Attachment #532068 - Flags: review+
Attachment #532068 - Flags: review?(blassey.bugs) → review+
Hmm, I take that back. Pinch zoom looks ok, however trying to do quick kinetic panning (!) becomes much slower for some reason. So apparently some profiling needs to be done to check what is wrong.
mfinkle, can we have your input on this ?
(In reply to comment #1)
> PS. I'm not particularly fond of having to use #ifdef
> MOZILLA_MAY_SUPPORT_NEON

It shouldn't be necessary here. supports_neon() should be an inline function that returns false when MOZILLA_MAY_SUPPORT_NEON is not defined (and thus the contents of the if() should be removed by the optimizer). You should only need it around code that won't compile at all without NEON support in the toolchain (i.e., inline asm or asm-only functions, or code which calls those functions).
OK, makes sense. Somehow I missed that 'supports_neon' function is not placed under #if defined(__GNUC__) && defined(__arm__) check and is also available for x86.
The sources of bad performance on kinetic panning after applying this bilinear enabler patch are:
1. The use of fractional translate transform - bilinear filter is used for blurring the pixels when panning, but it also increases the load on CPU
2. This particular variant of transformed over compositing operation is still not optimized with NEON when the filter is set to bilinear.

Using only integer translate should provide the best performance. Adding the missing NEON optimization should also generally help.
Depends on: 637852
setting feature request to new.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Linux → Android
Whiteboard: [testday-2011-06-24]
(In reply to Siarhei Siamashka from comment #6)
> The sources of bad performance on kinetic panning after applying this
> bilinear enabler patch are:
> 1. The use of fractional translate transform - bilinear filter is used for
> blurring the pixels when panning, but it also increases the load on CPU
> 2. This particular variant of transformed over compositing operation is
> still not optimized with NEON when the filter is set to bilinear.
> 
> Using only integer translate should provide the best performance.

Have recent changes like bug 637852 eliminated the non-integer translations during panning?
Closing all opened bug in a graveyard component
Status: NEW → RESOLVED
Closed: 8 months ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.