Differential D4841 Diff 19296 source/blender/editors/animation/keyframes_general.c

Changeset View

Standalone View

source/blender/editors/animation/keyframes_general.c

Context not available.
	#include "BLI_blenlib.h"	#include "BLI_blenlib.h"
	#include "BLI_utildefines.h"	#include "BLI_utildefines.h"
	#include "BLI_string_utils.h"	#include "BLI_string_utils.h"
		#include "BLI_math.h"

	#include "DNA_anim_types.h"	#include "DNA_anim_types.h"
	#include "DNA_object_types.h"	#include "DNA_object_types.h"
	#include "DNA_scene_types.h"	#include "DNA_scene_types.h"

	#include "BKE_action.h"	#include "BKE_action.h"
		#include "BKE_curve.h"
	#include "BKE_fcurve.h"	#include "BKE_fcurve.h"
	#include "BKE_report.h"	#include "BKE_report.h"
	#include "BKE_main.h"	#include "BKE_main.h"
Context not available.

	/* ---------------- */	/* ---------------- */

		/**
		* F-Curve 'decimate' function that removes a certain ratio of curve
		* points that will affect the curves overall shape the least.
		*/
		void decimate_fcurve(bAnimListElem *ale, float remove_ratio)
		{
		FCurve fcu = (FCurve )ale->key_data;

		/* Check if the curve actually has any points */
		if (fcu == NULL \|\| fcu->bezt == NULL \|\| fcu->totvert == 0) {
		return;
		}

		const int target_fcurve_verts = max_ii(2, fcu->totvert - fcu->totvert * remove_ratio);

		sybrenUnsubmitted Done Inline Actions You may want to add a comment here that explains that you don't want to decimate by using the error, but only by the number of remaining keyframes. sybren: You may want to add a comment here that explains that you don't want to decimate by using the…
		BezTriple *old_bezts = fcu->bezt;
		sybrenUnsubmitted Done Inline Actions The `error` here is a bit unclear; it could mean "this is the target length of the keyframes in error", e.g. the ones that are removed, or it could be "this is the target length of the keyframes to keep, for whatever function deals with errory things", e.g. the ones to keep. sybren: The `error` here is a bit unclear; it could mean "this is the target length of the keyframes in…

		sybrenUnsubmitted Done Inline Actions I wouldn't mind just passing `BEZT_FLAG_TEMP_TAG` directly to the `BKE_curve_decimate_bezt_array()` function. It's only used in one place anyway. The other variables at least clarify the numbers assigned to them, so they have added value, but passing a flag is IMO clear enough. sybren: I wouldn't mind just passing `BEZT_FLAG_TEMP_TAG` directly to the…
		sybrenUnsubmitted Done Inline Actions The above variables could be declared in the `if`-block they are used in. sybren: The above variables could be declared in the `if`-block they are used in.
		sybrenUnsubmitted Done Inline Actions The `old_bezts` variable is declared at the top of the function, whereas `target_fcurve_verts` and `num` are declared at this spot. This seems a bit strange to me, to have two declaration styles used intermixed in new code. sybren: The `old_bezts` variable is declared at the top of the function, whereas `target_fcurve_verts`…
		if (target_fcurve_verts != fcu->totvert) {
		sybrenUnsubmitted Done Inline Actions `num` is unnecessarily short and undescriptive; `totvert` or even `fcu_totvert` makes things a lot clearer. The placement of this line is confusing; the code probably exists at all to make the `for`-loop below faster, but `num` is already declared here. Furthermore, the use of `fcu->totvert` in the `if`-statement below isn't using `num` even though it's closer to this line. Either declare here and avoid `fcu->totvert` in the `if`, or just declare it directly above the `for`-loop before the `fcu->totvert = 0` assignment. sybren: `num` is unnecessarily short and undescriptive; `totvert` or even `fcu_totvert` makes things a…
		/* We don't want to limit the decimation to a certain error margin */
		const float error_sq_max = FLT_MAX;
		BKE_curve_decimate_bezt_array(fcu->bezt,
		fcu->totvert,
		12, /* 12 is the resolution of graph editor curves */
		false,
		SELECT,
		sybrenUnsubmitted Done Inline Actions Magic number; add a comment to explain why 12 is a good value here. sybren: Magic number; add a comment to explain why 12 is a good value here.
		BEZT_FLAG_TEMP_TAG,
		error_sq_max,
		target_fcurve_verts);
		}

		uint old_totvert = fcu->totvert;
		fcu->bezt = NULL;
		fcu->totvert = 0;

		for (int i = 0; i < old_totvert; i++) {
		BezTriple *bezt = (old_bezts + i);
		if ((bezt->f2 & BEZT_FLAG_TEMP_TAG) == 0) {
		insert_bezt_fcurve(fcu, bezt, 0);
		}
		}
		/* now free the memory used by the old BezTriples */
		if (old_bezts) {
		MEM_freeN(old_bezts);
		}
		}

		/* ---------------- */

	/* temp struct used for smooth_fcurve */	/* temp struct used for smooth_fcurve */
	typedef struct tSmooth_Bezt {	typedef struct tSmooth_Bezt {
	float h1, h2, h3; / bezt->vec[0,1,2][1] */	float h1, h2, h3; / bezt->vec[0,1,2][1] */
Context not available.
		sybrenUnsubmitted Done Inline Actions Unused parameter ‘ac’. sybren: Unused parameter ‘ac’.
		sybrenUnsubmitted Done Inline Actions No need for parentheses around the equality checks. sybren: No need for parentheses around the equality checks.
		sybrenUnsubmitted Done Inline Actions I would always write full sentences, so "Check if there are any points." or something like that. sybren: I would always write full sentences, so "Check if there are any points." or something like that.