ImBuf: optimize IMB_transform #115653

Aras Pranckevicius · 2023-12-01T11:20:19+01:00

2023-12-01 11:20:19 +01:00

IMB_transform is used by Sequencer (and other places) to do image translation/rotation/scale on the CPU. This PR speeds up parts of it, particularly when bilinear filtering is used. No behavior changes are expected.

Don't use virtual function calls inside inner loop. The code was using class hierarchies with virtual calls just to do equivalent of "outside of image? ignore" and "wrap UV coordinates or not?" decisions. Make those use non-virtual function based code.
Simplify pixel sampling functions to only do the work as needed by anything within Blender codebase. For example, bilinear sampling of uchar images always uses 4 RGBA channels and never does "UV wrap" logic.
Bilinear interpolation uchar: completely branchless SIMD code now.
Bilinear interpolation float: 2x floor() calls instead of 4x floor() + 2x ceil(), and final sample blending is done with SIMD.

Sequencer at 4K UHD resolution, with two image strips that need a transform, playback framerate:

Windows Ryzen 5950X: 18.7fps -> 26.2fps (IMB_transform time per frame goes 26.3ms -> 11.2ms)
Mac M1 Max: 27.3fps -> 31.4fps

At that point the IMB_transform is not the slowest part of where playback takes time (but rather sequencer effect application etc.).

Note: the amount of actual code got a bit smaller. But I've added 100 lines of unit tests in BLI_math_interp_test.cc, the bilinear interpolation functions were only tested very indirectly by CPU compositor template image tests.

`IMB_transform` is used by Sequencer (and other places) to do image translation/rotation/scale on the CPU. This PR speeds up parts of it, particularly when bilinear filtering is used. **No** behavior changes are expected. - Don't use virtual function calls inside inner loop. The code was using class hierarchies with virtual calls just to do equivalent of "outside of image? ignore" and "wrap UV coordinates or not?" decisions. Make those use non-virtual function based code. - Simplify pixel sampling functions to only do the work as needed by anything within Blender codebase. For example, bilinear sampling of uchar images always uses 4 RGBA channels and never does "UV wrap" logic. - Bilinear interpolation uchar: completely branchless SIMD code now. - Bilinear interpolation float: 2x `floor()` calls instead of 4x `floor()` + 2x `ceil()`, and final sample blending is done with SIMD. Sequencer at 4K UHD resolution, with two image strips that need a transform, playback framerate: - Windows Ryzen 5950X: 18.7fps -> 26.2fps (`IMB_transform` time per frame goes 26.3ms -> 11.2ms) - Mac M1 Max: 27.3fps -> 31.4fps At that point the IMB_transform is not the slowest part of where playback takes time (but rather sequencer effect application etc.). Note: the amount of _actual code_ got a bit smaller. But I've added 100 lines of unit tests in `BLI_math_interp_test.cc`, the bilinear interpolation functions were only tested very indirectly by CPU compositor template image tests.

❤️ 2 🎉 1 🚀 1

Aras Pranckevicius added 5 commits 2023-12-01 11:20:30 +01:00

1253411a91 ImBuf: don't use virtual calls in inner IMB_transform loop

The CropSource & NoDiscard functors were virtual classes for no good
reason really.

VSE, 4K resolution, two transformed image strips with bilinear filter,
Windows Ryzen 5950X: IMB_transform 26.3ms -> 24.7ms

c3f696e726 ImBuf: simplify bilinear/bicubic interpolation functions to what is actually used

- BLI_bilinear_interpolation_wrap_char is not used at all,
- BLI_bicubic_interpolation_char / BLI_bilinear_interpolation_char
  always uses 4 components

b0ea53dcff ImBuf: speedup bilinear_interpolation functions

Can do with 2 floor calls instead of 4 floor + 2 ceil calls.

VSE, 4K resolution, two transformed image strips with bilinear filter,
Windows Ryzen 5950X: IMB_transform 24.7ms -> 17.3ms

8e46f1b6b2 ImBuf: don't use virtual calls to do UV wrapping

Inner loop of IMB_transform was using virtual functions to do UV
wrapping. Simplify all of that from 3 classes to one bool.

VSE, 4K resolution, two transformed image strips with bilinear filter,
Windows Ryzen 5950X: IMB_transform 17.3ms -> 16.9ms

695da0c0f6 ImBuf: use SSE in bilinear_interpolation functions

VSE, 4K resolution, two transformed image strips with bilinear filter,
Windows Ryzen 5950X: IMB_transform 16.9ms -> 13.4ms

Aras Pranckevicius added 1 commit 2023-12-01 12:18:27 +01:00

buildbot/vexp-code-patch-coordinator Build done. Details

fe9db4d1e4 Format code

Aras Pranckevicius added 2 commits 2023-12-01 16:50:49 +01:00

183f585f08 ImBuf: add unit tests for BLI_bilinear_interpolation_char, fix rounding, do not require SSE4

buildbot/vexp-code-patch-coordinator Build done. Details

ce9860df3a Merge branch 'main' into imb_transform_opt

Aras Pranckevicius added 1 commit 2023-12-01 17:48:49 +01:00

buildbot/vexp-code-patch-coordinator Build done. Details

608fdcf337 Trying to fix Linux build

Aras Pranckevicius added 2 commits 2023-12-02 11:40:18 +01:00

6fc295d97f ImBuf: make BLI_bilinear_interpolation_char fully SSE and branchless

VSE, 4K resolution, two transformed image strips with bilinear filter,
Windows Ryzen 5950X: IMB_transform 13.4ms -> 11.2ms

8c8b4b30b9 Merge remote-tracking branch 'origin/main' into imb_transform_opt

Aras Pranckevicius added 1 commit 2023-12-02 11:53:39 +01:00

buildbot/vexp-code-patch-coordinator Build done. Details

8ffbfa061c Cleanup

Aras Pranckevicius added 1 commit 2023-12-02 12:05:25 +01:00

fbd9716206 No need to do floor() hard ways on NEON (or SSE4, which is not on yet but someday might be)

Aras Pranckevicius added 1 commit 2023-12-02 12:07:12 +01:00

buildbot/vexp-code-patch-coordinator Build done. Details

0bc8f85648 Fixing Linux build

Aras Pranckevicius added 1 commit 2023-12-02 13:41:29 +01:00

buildbot/vexp-code-patch-coordinator Build done. Details

faa6e268f1 Fixing Mac x64 build

Aras Pranckevicius commented

2023-12-02 13:41:43 +01:00

@blender-bot build

Aras Pranckevicius changed title from ~~WIP: ImBuf: optimize IMB_transform~~ to ImBuf: optimize IMB_transform

2023-12-02 14:09:49 +01:00

Aras Pranckevicius added this to the Video Sequencer project 2023-12-02 14:10:03 +01:00

Aras Pranckevicius requested review from Sergey Sharybin 2023-12-02 14:10:18 +01:00

Aras Pranckevicius requested review from Jeroen Bakker 2023-12-02 14:10:32 +01:00

Jeroen Bakker reviewed 2023-12-05 16:04:39 +01:00

Jeroen Bakker left a comment

Thanks for working on this. I still needs to go over the details of the patch.

For now intrisics are fine. The usecase is quite isolated, and doesn't change that much over time.

In the future ( not this PR) we would like to move some parts to our BaseVec classes in blenlib.

Thanks for working on this. I still needs to go over the details of the patch. For now intrisics are fine. The usecase is quite isolated, and doesn't change that much over time. In the future ( not this PR) we would like to move some parts to our BaseVec classes in blenlib.

Aras Pranckevicius commented

2023-12-05 17:45:45 +01:00

In the future ( not this PR) we would like to move some parts to our BaseVec classes in blenlib

Yeah I saw discussions "we should really pick/use some simd library" a few times. Personally TBH not sure if it belongs inside BaseVec itself. I can see both pros & cons of that. Largest downsides of that approach: 1) "SIMD for vectors/colors" does not scale to wider vectors than about 4 (so would not take advantage of AVX2, AVX512 etc.), 2) making BaseVec classes use SIMD underneath does affect their alignment, which may or might not be a problem. My vote would be on having "some" SIMD wrapper/helper library, that is along side of the vector/matrix library, i.e. separate types like vfloat perhaps (without size specified, map to whatever is CPU SIMD width) or vfloat4 etc.

But all of that indeed is outside the topic of this PR.

> In the future ( not this PR) we would like to move some parts to our BaseVec classes in blenlib Yeah I saw discussions "we should really pick/use some simd library" a few times. Personally TBH not sure if it belongs inside BaseVec itself. I can see both pros & cons of that. Largest downsides of that approach: 1) "SIMD for vectors/colors" does not scale to wider vectors than about 4 (so would not take advantage of AVX2, AVX512 etc.), 2) making BaseVec classes use SIMD underneath does affect their alignment, which may or might not be a problem. My vote would be on having "some" SIMD wrapper/helper library, that is _along side_ of the vector/matrix library, i.e. separate types like `vfloat` perhaps (without size specified, map to whatever is CPU SIMD width) or `vfloat4` etc. But all of that indeed is outside the topic of this PR.

Jeroen Bakker approved these changes 2023-12-14 10:42:03 +01:00

Aras Pranckevicius merged commit 1e0bf33b00 into main

2023-12-14 15:10:41 +01:00

Aras Pranckevicius referenced this issue from a commit

2023-12-14 15:10:42 +01:00

ImBuf: optimize IMB_transform

Aras Pranckevicius deleted branch imb_transform_opt

2023-12-14 15:10:44 +01:00

Brian Savery (AMD) referenced this issue from a commit

2023-12-28 20:07:45 +01:00

ImBuf: optimize IMB_transform

Sign in to join this conversation.

No reviewers

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

ImBuf: optimize IMB_transform #115653