[RFC PATCH 1/5] d2d1: Store cubic bezier control points.

[Henri Verbeet]

On Tue, 17 Mar 2020 at 21:46, Connor McAdams <conmanx360 at gmail.com> wrote:
> On Tue, Mar 17, 2020 at 09:04:10PM +0330, Henri Verbeet wrote:
> > On Sun, 15 Mar 2020 at 23:13, Connor McAdams <conmanx360 at gmail.com> wrote:
> > > @@ -57,6 +62,7 @@ struct d2d_segment_idx
> > >      size_t figure_idx;
> > >      size_t vertex_idx;
> > >      size_t control_idx;
> > > +    size_t bezier_idx;
> > >  };
> > >
> > >  struct d2d_figure
> > > @@ -70,6 +76,7 @@ struct d2d_figure
> > >      D2D1_POINT_2F *bezier_controls;
> > >      size_t bezier_controls_size;
> > >      size_t bezier_control_count;
> > > +    size_t bezier_control_points;
> > >
> > Are those really needed?
> Yes, they become more useful in later patches where there are
> triangulation structures per-bezier and not per-point, so knowing which
> bezier control we are on and not which control point we are on becomes
> necessary. I guess I could just introduce these changes in those patches
> where it becomes used specifically, but it seemed to be more related to
> this patch (in my opinion).

It's hard to say much without having seen the code that uses these,
but for what it's worth:
    - What does "bezier_control_points" signify? Based on the name I'd
expect it to be an array of bezier control points; i.e., the same as
"bezier_controls". In practice it seems to mostly be a variant of
"bezier_control_count", but without looking much closer at the patches
it's not clear it stores any information that couldn't be
reconstructed from the vertex type information.
    - That is again without having seen the code using it, but the
d2d_segment_idx structure is used for indexing segments. E.g. suppose
you have the index {1, 2, 3}, that refers to the segment starting at
figures[1]->vertices[2], of type figures[1]->vertex_types[2]. If that
type were to be a cubic bezier, it would have control points
figures[1]->bezier_controls[3] and figures[1]->bezier_controls[4].
Unless this was the last segment in the figure, the next segment would
then have index {1, 3, 5}.

> > > -static BOOL d2d_figure_insert_bezier_control(struct d2d_figure *figure, size_t idx, const D2D1_POINT_2F *p)
> > > +static BOOL d2d_figure_insert_quadratic_bezier_control(struct d2d_figure *figure, size_t idx, const D2D1_POINT_2F *p)
> > >  {
> > >      if (!d2d_array_reserve((void **)&figure->bezier_controls, &figure->bezier_controls_size,
> > > -            figure->bezier_control_count + 1, sizeof(*figure->bezier_controls)))
> > > +            figure->bezier_control_points + 1, sizeof(*figure->bezier_controls)))
> > >      {
> > >          ERR("Failed to grow bezier controls array.\n");
> > >          return FALSE;
> > >      }
> > >
> > >      memmove(&figure->bezier_controls[idx + 1], &figure->bezier_controls[idx],
> > > -            (figure->bezier_control_count - idx) * sizeof(*figure->bezier_controls));
> > > +            (figure->bezier_control_points - idx) * sizeof(*figure->bezier_controls));
> > >      figure->bezier_controls[idx] = *p;
> > > +    ++figure->bezier_control_points;
> > >      ++figure->bezier_control_count;
> > >
> > >      return TRUE;
> > >  }
> > >
> > > -static BOOL d2d_figure_add_bezier_control(struct d2d_figure *figure, const D2D1_POINT_2F *p)
> > > +static BOOL d2d_figure_add_quadratic_bezier_control(struct d2d_figure *figure, const D2D1_POINT_2F *p)
> > >  {
> > >      if (!d2d_array_reserve((void **)&figure->bezier_controls, &figure->bezier_controls_size,
> > > -            figure->bezier_control_count + 1, sizeof(*figure->bezier_controls)))
> > > +            figure->bezier_control_points + 1, sizeof(*figure->bezier_controls)))
> > >      {
> > >          ERR("Failed to grow bezier controls array.\n");
> > >          return FALSE;
> > >      }
> > >
> > > -    figure->bezier_controls[figure->bezier_control_count++] = *p;
> > > +    figure->bezier_controls[figure->bezier_control_points++] = *p;
> > > +    ++figure->bezier_control_count;
> > >
> > >      return TRUE;
> > >  }
> > These don't seem all that specific to quadratics.
> They are specific to quadratics in that they're only introducing one new
> control point. When cubics actually begin to be in use, there are
> d2d_figure_add_cubic_bezier_control functions along with the quadratic
> ones.
>
Well, sure. But you could also handle that by either simply calling
d2d_figure_add_bezier_control()/d2d_figure_insert_bezier_control()
twice for cubics, or by introducing
d2d_figure_add_bezier_controls()/d2d_figure_insert_bezier_controls()
which would add/insert an array of points instead of a single one.