sketch_solver.line_point has incorrect implementation?

[voneiden]

Background

Tested out sketching with constraints and found myself in a situation where it seemed like my Distance constraint between a midpoint of a line and a midpoint of an arc was not respected.

rotary_enc_od = 6
result = (
    cq.Sketch()
        .segment((-1, 0), (1, 0), "seg")
        .arc((0, 0), rotary_enc_od/2, 0, 200, "arc")
        .constrain("arc", "FixedPoint", None)
        .constrain("arc", "Radius", rotary_enc_od/2)
        .constrain("arc", "seg", "Coincident", None)
        .constrain("seg", "arc", "Coincident", None)
        .constrain("seg", "Orientation", (1, 0))
        .constrain("arc", "seg", "Distance", (0.5, 0.5, 4.5))
        .solve()
        .assemble()
)
show_object(result)

Output

Expected

Difference is subtle, but the current output produces a distance that is about 3.88 (measured from stl output) instead of the constrained 4.5.

Probable cause

After fiddling a bit with a debugger I noticed that line_point returned a rather odd value for the midpoint (-1.64, -0.85) instead of an expected (0, -0.56).

https:/CadQuery/cadquery/blob/master/cadquery/sketch.py#L924 sends lines/segments to the solver as a tuple (p1.x, p1.y, p2.x, p2.y) where p1 is start point and p2 is end point.

https:/CadQuery/cadquery/blob/master/cadquery/occ_impl/sketch_solver.py#L85 appears to behave as if the above tuple is actually (p1.x, p1.y, d.x, d.y) where p1 is start point and d is a vector from the start point to the end point.

Possible fix

I can get expected results if I change the implementation of line_point to

def line_point(x, val):
    return x[:2] + val * (x[2:] - x[:2])

Thoughts?

[fpq473]

I tried the suggested fix -- WFM and fwiw, LGTM.

[voneiden]

@adam-urbanczyk I've added a test case that uses distance constraint on a rectangle that fails against master and succeeds against the fix.

[adam-urbanczyk]

Will you open a PR?

[voneiden]

Oh dear, have I've opened it one year ago against my own repo? 😂 Yeah I'll open a proper PR later today ..!

[voneiden]

@adam-urbanczyk Alrighty: #1336