The formula was not consistent across Blender and behaved strangely, now it is
a simple linear blend between color1 and min(color1, color2).
Reviewed By: brecht
Differential Revision: https://developer.blender.org/D489
This issue is because of a somewhat "special" behavior in old code, which got lost during rB09874df:
There was a variant of the `relinkConnections` function which would leave the socket completely unconnected. This is not a valid state really (given that each unconnected input must otherwise connected to a constant `Set` type node), but was used as a way to distinguish connected alpha/depth sockets in composite and viewer output nodes.
https://developer.blender.org/diffusion/B/browse/master/source/blender/compositor/intern/COM_InputSocket.cpp;28a829893c702918afc5ac1945a06eaefa611594$69
After the large cleanup patch ({D309}) every socket is now automatically connected to a constant, such that `getInputSocketReader` will never return a NULL pointer. This breaks the previous test method, which needs to be replaced by more explicit flags. Luckily this was done only for very few output nodes (Composite, Viewer, Output-File). These now use the regular SetValueOperation default in case "use alpha" is disabled, but set this to an explicit 1.0 value instead of mapping to the node socket.
Many parts of the compositor are unnecessarily complicated. This patch
aims at reducing the complexity of writing nodes and making the code
more transparent.
== Separating Nodes and Operations ==
Currently these are both mixed in the same graph, even though they have
very different purposes and are used at distinct stages in the
compositing process. The patch introduces dedicated graph classes for
nodes and for operations.
This removes the need for a lot of special case checks (isOperation etc.)
and explicit type casts. It simplifies the code since it becomes clear
at every stage what type of node we are dealing with. The compiler can
use static typing to avoid common bugs from mixing up these types and
fewer runtime sanity checks are needed.
== Simplified Node Conversion ==
Converting nodes to operations was previously based on "relinking", i.e.
nodes would start with by mirroring links in the Blender DNA node trees,
then add operations and redirect these links to them. This was very hard
to follow in many cases and required a lot of attention to avoid invalid
states.
Now there is a helper class called the NodeConverter, which is passed to
nodes and implements a much simpler API for this process. Nodes can add
operations and explicit connections as before, but defining "external"
links to the inputs/outputs of the original node now uses mapping
instead of directly modifying link data. Input data (node graph) and
result (operations graph) are cleanly separated.
== Removed Redundant Data Structures ==
A few redundant data structures have been removed, notably the
SocketConnection. These are only needed temporarily during graph
construction. For executing the compositor operations it is perfectly
sufficient to store only the direct input link pointers. A common
pointer indirection is avoided this way (which might also give a little
performance improvement).
== Avoid virtual recursive functions ==
Recursive virtual functions are evil. They are very hard to follow
during debugging. At least in the parts this patch is concerned with
these functions have been replaced by a non-virtual recursive core
function (which might then call virtual non-recursive functions if
needed). See for example NodeOperationBuilder::group_operations.
which tiles are selected for input there was always a constant for correcting
the accuracy.
It seems that the constant was not enough and has been adjusted. (2 => 3).
This is a regression caused by rB67134a7bf689279785e2e40b29cd24243813998b
The UV coordinates read from the UV input must be scaled by the Image
input size instead of the UV input size.
Also now this node uses the UV input resolution instead of the Image
resolution, since this is what determines the available resolution. The
image is EWA-sampled anyway, it's resolution does not have a direct
impact.
This was suggested by Christopher Barrett (terrachild). Corner pin is a common feature in compositing.
The corners for the plane warping can be defined by using vector node inputs to allow using perspective plane transformations without having to go via the MovieClip editor tracking data.
Uses the same math as the PlaneTrack node, but without the link to MovieClip and Object.
{F78199}
The code for PlaneTrack operations has been restructured a bit to share it with the CornerPin node.
* PlaneDistortCommonOperation.h/.cpp: Shared generic code for warping images based on 4 plane corners and a perspective matrix generated from these. Contains operation base classes for both the WarpImage and Mask operations.
* PlaneTrackOperation.h/.cpp: Current plane track node operations, based on the common code above. These add pointers to MovieClip and Object which define the track data from wich to read the corners.
* PlaneCornerPinOperation.h/.cpp: New corner pin variant, using explicit input sockets for the plane corners.
One downside of the current compositor design is that there is no concept of invariables (constants) that don't vary over the image space. This has already been an issue for Blur nodes (size input is usually constant except when "variable size" is enabled) and a few others. For the corner pin node it is necessary that the corner input sockets are also invariant. They have to be evaluated for each tile now, otherwise the data is not available. This in turn makes it necessary to make the operation "complex" and request full input buffers, which adds unnecessary overhead.
This can happen if no image buffers are used to define a sensible
resolution. Then the viewer will stiff create a float buffer in the
output imbuf, which defies the usual ibuf->rect_float check and leads
to invalid memory access. Float buffer should not be created in this
case.
nodes.
The Rotate node was calculating the center with a 1 pixel offset, which
effectively shifts the image by 1 pixel on one or both axis for
right-angle (90 degree) rotations.
Note that the wrapping feature for translate nodes can still produce
undesirable results for non-quadratic images. This is because of how
the resolution calculation works atm: the Rotate node will keep the
resolution of the input image, even if the resulting image is then
cropped or leaves empty margins. There is no easy way to fix that
without redesign.
The underlying cause for these issues is the insufficient sampling of
the bokeh image. For smaller blur radius there will be very few samples
taken, and with 1-pixel radius it boils down to just 4 samples:
2 on the left border (black), 1 in the center (black) and 1 at the top
border (blue) ...
For now have added the workarounds implemented in the CPU version of
that node, which hide these artifacts. Ultimately would be better to
have mipmap levels for the bokeh image input instead.
The blur operations were clamping the filter size to 1, which prevents
no-op blur nodes. Further any value < 1 would also be ignored and in
many combinations the filter scale setting ("Size") would only work in
integer steps.
Now most blur settings will work with smooth Size value scaling as well,
meaning you can choose a reasonably large filter size (e.g. 10) and then
use the Size factor to scale the actual blur radius smoothly.
Note that non-integer filter sizes also depend on the filter type
selected in the Blur node, e.g. "Flat" filtering will still ignore
smooth filter sizes. Gaussian filters work best for this purpose.
The area-of-interest calculation for that node didn't work reliably.
It tries to estimate the distorted rectangular area based on min/max
distortion and dispersion values, but this fails in some cases and
leaves uninitialized buffer chunks. So now simply use the full input
rect as the area, even though it may not be as efficient - at least it
works ...
Also cleaned up the code somewhat to make it understandable, using
separate functions for common stuff instead of cryptic walls of math.
Summary:
Issue was caused by the same tile being written twice to
the EXR file. This was happening because of partial update
of work-in-progress tiles was merging result to the final
render result in order to make color management pipeline
happy.
We need to avoid such a merges and keep memory usage as
low as possible when Save Buffers is enabled.
Now render pipeline will allocate special display buffer
in render layer which will contain combined pass in the
display space. This keeps memory usage as low as we can
do at this moment.
There's one weak thing which is changing color management
settings during rendering would lead to lossy conversion.
This is because render result's display buffer uses color
space from the time when rendering was invoked.
This is actually what was happening in previous release
already actually so not a big issue.
Reviewers: brecht
Reviewed By: brecht
Differential Revision: https://developer.blender.org/D162
It was only used by opengl render and in fact it needed just to
set DISPLAY_BUFFER_INVALID flag for the image buffer.
In theory it wouldn't make any change to opengl render speed
(because this change just moved rect_from_float from color
management code to image save code). And could not see any speed
changes on my laptop.
EWA sampling is designed for downsampling images, i.e. scaling down the size of
input image pixels, which happens regularly in compositing. While the standard
sampling methods (linear, cubic) work reasonably well for linear
transformations, they don't yield good results in non-linear cases like
perspective projection or arbitrary displacement. EWA sampling is comparable to
mipmapping, but avoids problems with discontinuities.
To work correctly the EWA algorithm needs partial derivatives of the mapping
functions which convert output pixel coordinates back into the input image
space (2x2 Jacobian matrix). With these derivatives the EWA algorithm
projects ellipses into the input space and accumulates colors over their
area. This calculation was not done correctly in the compositor, only the
derivatives du/dx and dv/dy were calculation, basically this means it only
worked for non-rotated input images.
The patch introduces full derivative calculations du/dx, du/dy, dv/dx, dv/dy for
the 3 nodes which use EWA sampling currently: PlaneTrackWarp, MapUV and
Displace. In addition the calculation of ellipsis area and axis-aligned
bounding boxes has been fixed.
For the MapUV and Displace nodes the derivatives have to be estimated by
evaluating the UV/displacement inputs with 1-pixel offsets, which can still have
problems on discontinuities and sub-pixel variations. These potential problems
can only be alleviated by more radical design changes in the compositor
functions, which are out of scope for now. Basically the values passed to the
UV/Displacement inputs would need to be associated with their 1st order
derivatives, which requires a general approach to derivatives in all nodes.
inputs.
http://wiki.blender.org/uploads/4/4c/Compo_image_interpolation_borders.png
Problem is that all image buffer reader nodes (RenderLayer, Image,
MovieClip) were clipping pixel coordinates to 0..N range (N being width
or height respectively). Bilinear interpolation works ok then on the
upper-right borders (x, N) and (N, y), since the last (N-1) pixel fades
out to N (background). But the lower-left (x, 0) and (0, y) borders are
not correctly interpolated because the nodes cut off the negative pixels
before the interpolation function can calculate their value.
To fix this, the interpolation functions are now entirely responsible
for handling "out of range" cases, i.e. setting (0,0,0,0) results for
invalid pixels, while also handling interpolation for borders.
Callers should not do pixel range checks themselves, which also makes
the code simpler. Should not have any real performance penalty,
the interpolation functions do this check anyway, so is probably even
slightly faster.
This patch changes most of the reamining degrees usage in internal code into radians.
I let a few which I know off asside, for reasons explained below - and I'm not sure to have found out all of them.
WARNING: this introduces forward incompatibility, which means files saved from this version won't open 100% correctly
in previous versions (a few angle properties would use radians values as degrees...).
Details:
- Data:
-- Lamp.spotsize: Game engine exposed this setting in degrees, to not break the API here I kept it as such
(using getter/setter functions), still using radians internally.
-- Mesh.smoothresh: Didn't touch to this one, as we will hopefully replace it completely by loop normals currently in dev.
- Modifiers:
-- EdgeSplitModifierData.split_angle, BevelModifierData.bevel_angle: Done.
- Postprocessing:
-- WipeVars.angle (sequencer's effect), NodeBokehImage.angle, NodeBoxMask.rotation, NodeEllipseMask.rotation: Done.
- BGE:
-- bConstraintActuator: Orientation type done (the minloc[0] & maxloc[0] cases). Did not touch to 'limit location' type,
it can also limit rotation, but it exposes through RNA the same limit_min/limit_max, which hence
can be either distance or angle values, depending on the mode. Will leave this to BGE team.
-- bSoundActuator.cone_outer_angle_3d, bSoundActuator.cone_inner_angle_3d: Done (note I kept degrees in BGE itself,
as it seems this is the expected value here...).
-- bRadarSensor.angle: Done.
Reviewers: brecht, campbellbarton, sergey, gaiaclary, dfelinto, moguri, jbakker, lukastoenne, howardt
Reviewed By: brecht, campbellbarton, sergey, gaiaclary, moguri, jbakker, lukastoenne, howardt
Thanks to all!
Differential Revision: http://developer.blender.org/D59
Previous wrap implementation was based on inverse
bilinear mapping, which doesn't give perspective.
Now plane track wrap estimates perspective transform
matrix as a homography estimation between frame
coordinates and plane corners.
Uses Libmv's implementation for this, which means
plane track wouldn't work properly with Libmv
disabled. Not a deal for official builds at all,
just folks who keeps things disabled need to be
aware of this.
Distinguish the 3 different methods for acquiring pixel color values (executePixel, executePixelSampled, executePixelFiltered).
This makes it easier to keep track of the different sampling methods (and works nicer with IDEs that do code parsing).
Differential Revision: http://developer.blender.org/D7
The RenderLayers node would use the "combined" image result for all passes which don't have a valid render result yet. This causes problems when the buffer element size is not actually 4 floats (RGBA) as
with the 3 float normal passes. Also the result is rather meaningless then, so just keep the image buffer at NULL for unavailable passes, which will return plain (0,0,0) color.
After discussion with Brecht decided that automatically updating the sockets of the node based on externally modified data (removed EXR file passes) is not desirable behavior. But at least making sure
the correct passes are assigned to the output sockets of the Image node is possible. Now the passes are matched by name instead of using the faulty index stored in the socket data, which is more
reliable. Still may break if changing pass names externally, but an image reload is highly recommended anyway and will fix that.
Debug code for graphviz output moved to a dedicated file COM_Debug.h/cpp.
The DebugInfo class has only static functions, which are called from a number of places to keep track of what is happening in the compositor. If debugging is disabled these are just inline stubs, so we
don't need #ifdefs everywhere and don't get any overhead.
The graphviz output is much more useful now. DebugInfo keeps track of node names in a static string map for meaningful names. It uses a number of colors for various special operation classes.
ExecutionGroups are indicated in graphviz with clusters.
Currently the graphviz .dot files are stored in the BLI_temporary_dir() folder. A separate dot file is generated for each stage of the ExecutionGroup scheduling, this is intended to give some idea of the
compositor progress, but could still be improved.
