Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(7135)

Issue 6603053: CUBIC B SPLINE WITH PREFILTER: Scaling algorithm implementation

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 6 months ago by troy_s
Modified:
11 years, 6 months ago
Reviewers:
lukas.toenne1
Base URL:
https://svn.blender.org/svnroot/bf-blender/trunk/blender/
Visibility:
Public.

Description

This patch implements a prefilter for standard cubic scaling.

Patch Set 1 #

Total comments: 5
Unified diffs Side-by-side diffs Delta from patch set Stats (+176 lines, -2 lines) Patch
source/blender/compositor/intern/COM_SocketReader.h View 1 chunk +2 lines, -1 line 2 comments Download
source/blender/compositor/operations/COM_ImageOperation.h View 2 chunks +5 lines, -1 line 0 comments Download
source/blender/compositor/operations/COM_ImageOperation.cpp View 6 chunks +35 lines, -0 lines 3 comments Download
source/blender/compositor/operations/COM_MultilayerImageOperation.cpp View 1 chunk +7 lines, -0 lines 0 comments Download
source/blender/imbuf/IMB_imbuf.h View 1 chunk +14 lines, -0 lines 0 comments Download
source/blender/imbuf/intern/imageprocess.c View 2 chunks +112 lines, -0 lines 0 comments Download
source/blender/makesrna/intern/rna_nodetree.c View 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 3
troy_s
11 years, 6 months ago (2012-10-04 17:22:43 UTC) #1
lukas.toenne1
https://codereview.appspot.com/6603053/diff/1/source/blender/compositor/intern/COM_SocketReader.h File source/blender/compositor/intern/COM_SocketReader.h (right): https://codereview.appspot.com/6603053/diff/1/source/blender/compositor/intern/COM_SocketReader.h#newcode36 source/blender/compositor/intern/COM_SocketReader.h:36: COM_PS_CUBICBSPLINEPREFILTER = 3 Would this actually ever be used ...
11 years, 6 months ago (2012-10-04 18:45:55 UTC) #2
troy_s
11 years, 6 months ago (2012-10-04 19:07:36 UTC) #3
Thanks so much for taking time to review this Lukas.

https://codereview.appspot.com/6603053/diff/1/source/blender/compositor/inter...
File source/blender/compositor/intern/COM_SocketReader.h (right):

https://codereview.appspot.com/6603053/diff/1/source/blender/compositor/inter...
source/blender/compositor/intern/COM_SocketReader.h:36:
COM_PS_CUBICBSPLINEPREFILTER = 3
On 2012/10/04 18:45:55, lukas.toenne1 wrote:
> Would this actually ever be used anywhere? From looking at the other sampling
> modes it seems that nodes would have to explicitly override the sampler
> parameter to make use of it.

Indeed. This was a welcome addition to be able to control the scaling algorithm,
so I merely adapted to the existing format.

> 
> IIRC there was an experimental node to force a particular sampling mode on
> inputs, but it was not intended for production work and only used for testing.

Bad idea in my mind. See other comment in other file.

> 
> Furthermore you also need to ensure that all nodes which refer to the sampler
> input can handle this additional case or fall back to one of the other modes
> safely. In particular the MovieClipOperation (which should probably ignore
it).

I believe this is plug and play useful immediately and would provide a valuable
option to interested artists.

I don't believe MovieClip is an exception here.

> Also ReadBufferOperation would use cubic in this case.

Explain?

> 
> Alternatively this could be made an option just for the image node to override
> the requested sampling mode locally for just this node, or replace BICUBIC
with
> the prefilter method if the option is enabled. See also the next comment!

This is certainly increases a cubic accuracy, and in many instances where cubic
is a default, it should be upgraded to this. However, within an artistic choice
role, the option to select approaches is extremely valuable to an artist.

https://codereview.appspot.com/6603053/diff/1/source/blender/compositor/opera...
File source/blender/compositor/operations/COM_ImageOperation.cpp (right):

https://codereview.appspot.com/6603053/diff/1/source/blender/compositor/opera...
source/blender/compositor/operations/COM_ImageOperation.cpp:149: lockMutex();
On 2012/10/04 18:45:55, lukas.toenne1 wrote:
> Don't think such mutex locks + buffer check should be done in the inner pixel
> loop. Of course, to avoid this one would have to know the sampling mode in
> advance. Referring to my previous comment, the easiest way to do this would be
a
> node option "use bspline prefilter" or so, which, if enabled, would always
> generate the secondary buffer in initExecution and possibly ignore the
requested
> sampling mode altogether.

I believe offering sampling method is mandatory. For example, a pixelated effect
zoom would require nearest. So I don't believe removing the selection is a wise
path.

I agree that the per pixel location is awkward, but it would seem it is hard
coded into an architecture that didn't foresee frequency domain filters on
imaging?

Perhaps we can address this with a similar BaseImageOperation mode akin to
setting Comolex mode?
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b