Ah, okay. That makes sense. In my original patch, I had just stored everything as a cubic in the same way everything was previously stored as a quadratic.
If both are to co-exist, should the functions for detecting intersections between beziers all be cubic, or should they be temporarily promoted to a cubic in that instance? Same goes with bezier-line intersections. One of the nice things about storing them all with both cubic and quadratic coordinates was that it meant that things that used one or the other could do so interchangeably, but I guess it can be done that way too with them as separate types with a little more work.
Also, when they're eventually sent to the pixel shader, would you like there to be separate paths for cubics and quadratics? Or can I promote a quadratic to a cubic in that case? It kind of gets a little messy supporting both, but if that's what's best, I can do that.
On Tue, Feb 25, 2020 at 9:45 AM Henri Verbeet hverbeet@gmail.com wrote:
Hi Connor,
It will probably take me a while to go through the entire series, but here are some comments/questions on the first two patches.
On Tue, 25 Feb 2020 at 06:03, Connor McAdams conmanx360@gmail.com wrote:
+struct d2d_bezier_controls +{
- D2D1_POINT_2F c0, c1;
- D2D1_POINT_2F cq0;
+};
Why do you need this structure? It seems more convenient to rename D2D_VERTEX_TYPE_BEZIER to D2D_VERTEX_TYPE_QUADRATIC_BEZIER, and introduce D2D_VERTEX_TYPE_CUBIC_BEZIER, while keeping the control point arrays the way they are. Not in the least because d2d_path_geometry_Stream() will need to know the original segment types.
@@ -1907,8 +1928,13 @@ static BOOL d2d_geometry_apply_intersections(struct d2d_geometry *geometry, d2d_point_lerp(&q[0], p[0], p[1], t); d2d_point_lerp(&q[1], p[1], p[2], t);
figure->bezier_controls[inter->control_idx + control_offset] = q[0];
if (!(d2d_figure_insert_bezier_control(figure, inter->control_idx + control_offset + 1, &q[1])))
d2d_bezier_quad_to_cubic(p[0], &q[0], &inter->p, &c[0], &c[1]);
figure->bezier_controls[inter->control_idx + control_offset].cq0 = q[0];
figure->bezier_controls[inter->control_idx + control_offset].c0 = c[0];
figure->bezier_controls[inter->control_idx + control_offset].c1 = c[1];
d2d_bezier_quad_to_cubic(&inter->p, &q[1], p[2], &c[0], &c[1]);
if (!(d2d_figure_insert_bezier_control(figure, inter->control_idx + control_offset + 1, &c[0], &c[1], &q[1]))) return FALSE; ++control_offset;
...
@@ -2769,8 +2795,15 @@ static BOOL d2d_geometry_split_bezier(struct d2d_geometry *geometry, const struc d2d_point_lerp(&q[1], p[1], p[2], 0.5f); d2d_point_lerp(&q[2], &q[0], &q[1], 0.5f);
- figure->bezier_controls[idx->control_idx] = q[0];
- if (!(d2d_figure_insert_bezier_control(figure, idx->control_idx + 1, &q[1])))
- d2d_bezier_quad_to_cubic(p[0], &q[0], &q[2], &c[0], &c[1]);
- figure->bezier_controls[idx->control_idx].cq0 = q[0];
- figure->bezier_controls[idx->control_idx].c0 = c[0];
- figure->bezier_controls[idx->control_idx].c1 = c[1];
- d2d_bezier_quad_to_cubic(&q[0], &q[1], p[2], &c[0], &c[1]);
- if (!(d2d_figure_insert_bezier_control(figure, idx->control_idx + 1, &c[0], &c[1], &q[1]))) return FALSE; if (!(d2d_figure_insert_vertex(figure, idx->vertex_idx + 1, q[2]))) return FALSE;
Likewise, you wouldn't need these awkward bits of code.