Add Cubic B-Spline with Prefilter Algorithm
Needs RevisionPublic

Authored by Aaron Carlisle (Blendify) on Jan 1 2018, 10:39 PM.

Details

Summary

This probably is not ready for review as I am not even sure if it works or compiles.

This patch is an updated version of the svn patch found in T30411

For details on the patch have a look at https://wiki.blender.org/index.php/User:Sobotka/Cubic_B-Spline_Scaling_Algorithm

Diff Detail

Repository
rB Blender
Branch
arcpatch-D2984
Build Status
Buildable 1067
Build 1067: arc lint + arc unit
Aaron Carlisle (Blendify) planned changes to this revision.Jan 2 2018, 5:24 PM
  • Update so this now compiles

Not really happy with how this patch requires a copy of the image (goes against compositor design).

Also, in own tests I can't see the difference between BICUBIC and CUBICBSPLINEPREFILTER.
The patch seems to be working as expected and I checked the code runs on the filtered buffer.

If you can't tell the difference, it may not be working correctly. Should be as clearly apparent as in https://youtu.be/nfhTET86kdE

It should also be possible to demonstrate a stair interpolation such as running a sampling using cubic 10x at 10% increase; the end will be a muddy, blurry mess. Do the same with a prefilter and the result should be identical as having done it once at the same resultant value.

Sergey Sharybin (sergey) requested changes to this revision.Jan 9 2018, 10:37 AM

While this might be handy to have, the implementation goes against compositor design:

  • You must not duplicate whole buffer, limit it to the tile only.
  • This is only supported by Image operation, but about render result? What about filtered bicubic filtering of blur result? Filtering is to be fully supported by buffer readers.
source/blender/compositor/intern/COM_SocketReader.h
36

Don't glue words together, use underscore. Keeps better readability.

source/blender/compositor/operations/COM_ImageOperation.cpp
135–143

You must not duplicate the whole buffer. It is OK to have some additional buffer for compositing operations, but those are to be limited to active tile only. See initializeTileData and friends.

source/blender/compositor/operations/COM_ImageOperation.h
43

This is really bad name. What is secondary? m_prefiltered_buffer will be more meaningful name. But also read later.

source/blender/imbuf/intern/imageprocess.c
344–354

Over-documentation, not sure it really helps. Also don't use C++ comments in C code.

369–372

Overdocumentation again. Also, floating buffers are not always RGBA. You need to take ibuf->channels into account.

This revision now requires changes to proceed.Jan 9 2018, 10:37 AM

You must not duplicate whole buffer, limit it to the tile only.

Can we fix the design to work with proper sampling algorithms then?

The sampling algorithm and implementation is to be formulated in a terms that does not need require the whole buffer.

The sampling algorithm and implementation is to be formulated in a terms that does not need require the whole buffer.

Happy to bend around a poorly informed sampling structure, except that to properly derive the coefficients buffer, the entire image is required.

Not keeping the buffer around is surely a performance drain, and seems rather ridiculous. Is there a means to append a buffer as a sub-buffer or something that would keep Blender from recalculating the coefficients?

Open to suggestions here.

You don't need whole buffer, you only need tile. As i've said before, have a look at initializeTileData.

The coefficients are derived by looping through the entire horizontal and vertical rows and columns of data.

Are you suggesting to loop through tiles as opposed to grabbing the whole buffer?

Those coefficients are localizable to a pixel neighborhood. You can calculate them for the tile, and toss them away once the tile is handled.