On 01.09.2017 1:25, Henri Verbeet wrote:

On 31 August 2017 at 14:00, Nikolay Sivov [email protected] wrote:

...

• if (transform)
• {

•    if (!d2d_matrix_invert(&g_i, transform))
  

•        return D2DERR_UNSUPPORTED_OPERATION;
  

•    d2d_point_transform(&point, &g_i, point.x, point.y);
  

• }

• if (FAILED(hr = geometry_inv_transform_point(transform, &point)))

•    return hr;
  

That's fine in principle, but please do it in a separate commit, and please follow the existing conventions. E.g.:

if (transform && FAILED(hr = d2d_point_transform_inverse(&point,

transform, point.x, point.y))) return hr;

Sure.

...

• stroke_width /= 2.0f;
• rect = geometry->u.rectangle.rect;
• rect.left -= stroke_width;
• rect.right += stroke_width;
• rect.top -= stroke_width;
• rect.bottom += stroke_width;
• dx = max(fabsf((rect.right + rect.left) / 2.0f - point.x) - (rect.right - rect.left) / 2.0f, 0.0f);
• dy = max(fabsf((rect.bottom + rect.top) / 2.0f - point.y) - (rect.bottom - rect.top) / 2.0f, 0.0f);
• if ((*contains = tolerance * tolerance > (dx * dx + dy * dy)))
• {

•    rect = geometry->u.rectangle.rect;
  

•    stroke_width += tolerance;
  

•    rect.left += stroke_width;
  

•    rect.right -= stroke_width;
  

•    rect.top += stroke_width;
  

•    rect.bottom -= stroke_width;
  

•    *contains = rect.left >= point.x || rect.right <= point.x || rect.top >= point.y || rect.bottom <= point.y;
  

• }

I think that looks a little more complicated than it needs to be. How about:

D2D1_POINT_2F d, s;

... s.x = rect.right - rect.left; s.y = rect.bottom - rect.top; d.x = fabsf((rect.right + rect.left) * 0.5f - point.x); d.y = fabsf((rect.bottom + rect.top) * 0.5f - point.y);

if (d.x < (s.x - stroke_width) * 0.5f - tolerance && d.y < (s.y -

stroke_width) * 0.5f - tolerance) { *contains = FALSE; return S_OK; }

d.x = max(d.x - (s.x + stroke_width) * 0.5f, 0.0f);
d.y = max(d.y - (s.y + stroke_width) * 0.5f, 0.0f);
*contains = (d.x * d.x + d.y * d.y) < tolerance * tolerance;

Ok, I'll take a look.

I notice you don't have any tests with transformations, is that intentional? I think you'd need to transform the stroke width with the transformation matrix.

No particular reason, but it's a good point. Also we don't have any tests for non-invertible case apparently, I'll add some.