ARM Skia NEON patches - 20 - New improved BitmapProcState code

ARM Skia NEON patches - 20 - New improved BitmapProcState code


BitmapProcState: new factorised code

This one basically factorises the clamp and repeat transformations with
some performance improvements. It has the benefit of being faster, much
easier to maintain (nearly three times less code for more work
done :-)), and more complete (all persp transformations weren't optimised
in the previous version).

It also introduces the use of can_truncate_to_fixed_for_decal where
useful.

The effect on benchmarks ranges from a 5% penalty to a 25% gain on a
Cortex-A9 and from a 5% penalty to a 100% gain on a Cortex-A15.

Signed-off-by: Kévin PETIT <kevin.petit@arm.com>;

BUG=

Committed: http://code.google.com/p/skia/source/detail?r=13218

[djsollen, 2013-09-06 19:02:34]

mike, can you run this through your updated bench on a nexus 4/10 and post the
highlights here?

[kevin.petit.not.used.account, 2013-09-09 15:35:22]

I've had a little look at the trybot results. As I didn't mention it explicitly
in the commit message, I'll add it here : mismatches are expected.

[kevin.petit.not.used.account, 2013-09-18 13:44:35]

If you're interested I can split this into two patches:
   - a first one that introduces 99% of the changes but with only two
mismatching gms
   - a small second one with the accuracy improvements that trigger most of the
mismatches

[djsollen, 2013-09-18 13:53:49]

On 2013/09/18 13:44:35, kevin.petit.arm wrote:
> If you're interested I can split this into two patches:
>    - a first one that introduces 99% of the changes but with only two
> mismatching gms
>    - a small second one with the accuracy improvements that trigger most of
the
> mismatches

That would be preferable since we have been dealing with a lot of recent CLs
triggering image regressions.

[kevin.petit.not.used.account, 2013-09-18 14:44:18]

Patch Set 2 reduces a lot the number of mismatches.

The only mismatches that remain are in SCALE-FILTER which had no NEON code
before and was thus using SkFractionalInt for all the calculations. The NEON
version is an approximation on 32bit with SkFixed, basically the same that has
been used in all other transforms.

I have a subsequent patch that improves accuracy by using SkFractionalInt in
more places but it results in more than 40 mismatches.

[kevin.petit.not.used.account, 2013-12-19 17:54:53]

ping.

I've just retested. There are still two mismatching GMs (scaled_tilemodes_npot
and giantbitmap_repeat_bilerp_scale). I still get what I consider to be
interesting speed improvements.

Any comments?

[kevin.petit.not.used.account, 2014-01-13 16:09:58]

Sorry, I just noticed the trybots results and uploaded a rebased patch that will
apply :-).

[kevin.petit.not.used.account, 2014-01-13 18:36:02]

I've had a look at the trybot results: 2 failing GMs, as expected. I can add
them to expectations/gm/ignored-tests.txt provided you are happy with the patch
and the performance.

[djsollen, 2014-01-14 16:29:52]

go ahead and update the ignored-tests file in this CL.

[kevin.petit.not.used.account, 2014-01-14 17:55:48]

I've just updated ignored-tests.txt. The trybots should all go green for Patch
Set 4.

[kevin.petit.not.used.account, 2014-01-14 18:08:17]

Rebased because of a conflict on ignored-tests.txt, Patch Set 5 will apply.

[kevin.petit.not.used.account, 2014-01-21 16:23:53]

I've just rebased and retested.

May I ping again on this one? From what I understand it just needs to go through
the Test-Android-Nexus7-Tegra3-Arm7-Debug trybot to get in.

Thank you!

[kevin.petit.not.used.account, 2014-01-28 13:18:50]

This one has passed the perf bots and the test bot on Patch Set 6. I've just
rebased (to fix a conflict on ignored-tests.txt) and retested.

Any more comments or things you'd like me to do?

[djsollen, 2014-01-28 13:25:50]

The change lgtm as is. Have you worked through our rebaselining process before?
If not, I can help step you through it.

[kevin.petit.not.used.account, 2014-01-28 13:28:38]

On 2014/01/28 13:25:50, djsollen wrote:
> The change lgtm as is. Have you worked through our rebaselining process
before?
> If not, I can help step you through it.

No. This will be my first one. Zheng (who recently contributed the blur NEON
patch) gave me a few tips:

1. commit the patch
2. wait the test bot to generate image results
https://code.google.com/p/skia-autogen/source/list
3. create another CL follow
https://sites.google.com/site/skiadocs/developer-documentation/using-gm-golde...
4. upload the patch

Does that seem sensible to you?

[kevin.petit.not.used.account, 2014-01-28 14:08:50]

On 2014/01/28 13:28:38, kevin.petit.arm wrote:
> On 2014/01/28 13:25:50, djsollen wrote:
> > The change lgtm as is. Have you worked through our rebaselining process
> before?
> > If not, I can help step you through it.

BTW, I was a bit quick to answer: thanks for offering help with the rebaseling
:-).

[djsollen, 2014-01-28 14:12:24]

yes, those are exactly the steps.  Don't forgot to revert the change to
ignored-tests in your rebaseline CL

[kevin.petit.not.used.account, 2014-01-28 15:01:33]

On 2014/01/28 14:12:24, djsollen wrote:
> yes, those are exactly the steps.  Don't forgot to revert the change to
> ignored-tests in your rebaseline CL

Ok, I'll commit the change and proceed with rebaselining.

[commit-bot: I haz the power, 2014-01-28 15:03:29]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/kevin.petit.arm@gmail.com/23835006/27...

[commit-bot: I haz the power, 2014-01-28 15:18:58]

Change committed as 13218

[kevin.petit.not.used.account, 2014-01-28 17:56:01]

Rebaselines are in https://codereview.chromium.org/137833005/.