Skip to content

feat: add cubic bezier curve - #1410

Open
fran68 wants to merge 3 commits into
kalkih:devfrom
fran68:dev-feat/add_cubic_bezier_curve
Open

feat: add cubic bezier curve#1410
fran68 wants to merge 3 commits into
kalkih:devfrom
fran68:dev-feat/add_cubic_bezier_curve

Conversation

@fran68

@fran68 fran68 commented Jul 31, 2026

Copy link
Copy Markdown

This PR adds the option to use the cubic Bezier C curve for a line graph. In this case using the show_state: last the state value matches the value shown by touching the last point.

image image

It correlates much better to the values by the linear curve since it's not shaped in the way the quadratic bezier curve is.
image

@ildar170975

Copy link
Copy Markdown
Collaborator

Will check it when be near a PC.
Just a quick remark: please move “0.15” to const.js.

Comment thread src/buildConfig.js

conf.fill_baseline = checkNumericOption(conf, 'fill_baseline', undefined);

conf.tension = checkNumericOption(conf, 'tension', DEFAULT_BEZIERC_TENSION, 0);

@ildar170975 ildar170975 Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it needed to specify minBound or maxBound?
(these are optional arguments of checkNumericOption())
IMHO - should be 0...0.25 (or smaller)

@ildar170975

ildar170975 commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

@fran68
In Readme, it is needed to add a description of a tension option (if this is a config option).
Also, is the "tension" a good name? Shouldn't the name reflect what it is related to? (like smoothing_tension, you better know a physical meaning of this parameter)
Googled the subject a bit, seems that "tension" is a some kind of "official" designation, let's stick to it, but perhaps we need to rename it to "smoothing_tension".
Also, add a note in a description that it only matters in case of smoothing = bezierc.

Comment thread README.md
| upper_bound_secondary | number *or* string | | v0.5.0 | Set a fixed upper bound for the graph secondary Y-axis. String value starting with ~ (e.g. `~50`) specifies soft bound.
| min_bound_range_secondary | number | | v0.x.x | Applied after everything, makes sure there's a minimum range that the secondary Y-axis will have. Useful for not making small changes look large because of scale.
| smoothing | boolean | `true` | v0.8.0 | Whether to make graph line smooth.
| smoothing | boolean | `true` | v0.8.0 | Whether to make graph line smooth. Use `bezierc` to describe a line by a cubic Bezier curve.

@ildar170975 ildar170975 Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it mean that the smoothing option can be false/true or bezierc? Let's specify it here explicitly. Please check a proposed description, see below.

Comment thread src/graph.js
this._groupBy = groupBy;
this._endTime = 0;
this.fill_baseline = fill_baseline;
this._tension = tension;

@ildar170975 ildar170975 Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a mix of different namings here, let's follow this rule: if a property has a getter/setter - then it's name is this._something (starts with "_"), otherwise just this.something. THing is that some not-undescored properties are only used in the class, so it is really a mix imho... I will go through all properties of Graph class & set "underscore" where i it is needed (although it is just a cosmetic/stylistic change).

@ildar170975

ildar170975 commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

We need to add some description of the new smoothing method into Readme.
User should understand (in some extent) a difference between smoothing: true & smoothing: bezierc (and for more details they can google, but some minimal info should be provided in Docs, may be a few words). And a possible user question might be "why not making the new method as a default if it is better?"

As a variant:
| smoothing | boolean or string | true | v0.8.0 | Set the graph line shape: false (straight lines), true (quadratic Bezier curve, legacy method), or 'cubic' (cubic Bezier curve, gives a more precise presentation). |

Here:

  1. I intentionally do not use a "smoothness" word in a description to allow to add a new "stepline" method in future (like in a stock history-graph). Imho "shape" can refer both to a "smoothness" & "stepline".
  2. The bezierc word might be "too much"; since the current method is "quadratic" - why not to add a new "cubic" method? (i.e. rename the bezierc to cubic)

@ildar170975

ildar170975 commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

In this case using the show_state: last the state value matches the value shown by touching the last point.

  1. Note that per-entity option show_state can only be true/false.
  2. Currently with a show.state: last we already can see the last point's value, please check:
last

Imho, the main point here might be "the new BezierC smoothing gives a nicer look" or something similar...
(better than the currently implemented quadratic spline etc etc)

For the record - it seems that this is called "Catmull-Rom spline to Bezier curve conversion" (using 4 points to create a curve based on cubic curves).

Here is what I got with this PR:
image

type: custom:mini-graph-card
entities:
  - entity: sensor.xiaomi_cg_1_co2
    smoothing: bezierc
    _show_graph: false
  - entity: sensor.xiaomi_cg_1_co2
    smoothing: true
    _show_graph: false
  - entity: sensor.xiaomi_cg_1_co2
    smoothing: false
    line_width: 2
    color: black
    _show_graph: false
height: 200
show:
  points: true
  labels: true
  state: true
  fill: false

Number of points for smoothing=false & bezierc are same (12 = 24*0.5), there are points in the beginning & in the end (points for both methods are same).
For the current quadratic curve (smoothing=true) there are 11 points (because of midPoints), there are no points in the beginning & in the end.
I would say, the current method is suspicious in comparison to the proposed one.

Totally like the new tension option))))
image
We need to set min/maxBound for this option.
IMHO - should be 0...0.25 (or smaller)

Comment thread src/graph.js
const p3 = (i + 2 < arr.length) ? arr[i + 2] : p2;

// First control point (close to p1)
const cp1x = p1[0] + (p2[0] - p0[0]) * tension;

@ildar170975 ildar170975 Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't they be addressed as p[X] instead of p[0] ? (for consistency)
Same comment for coords[0][0] above.

Comment thread src/graph.js
genBezierCPath(coords) {
// Starting point with x with coords[0][0] and y with coords[0][Y]
let path = `${coords[0][0]},${coords[0][Y]}`;
const tension = this._tension; // Default 0.15

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better to remove this comment since we may change a default value in "const.js"

Comment thread src/const.js
const DEFAULT_HOURS_TO_SHOW = 24;
const DEFAULT_POINTS_PER_HOUR = 0.5;
const DEFAULT_STATIC_VALUE_LABEL_OFFSET = 20; // in %
const DEFAULT_BEZIERC_TENSION = 0.15;

@ildar170975 ildar170975 Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as 1/6 ? (approx). Saw this "1/6" parameter in formulae in internet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants