This time including some regression tests.
-- v3: win32u: Emit a rectangle for 1-pixel ellipses. win32u: Normalize the ellipse width/height in dibdrv_RoundRect().
From: Elizabeth Figura zfigura@codeweavers.com
Ellipses must be exactly tangent with their bounding rects. Generating arcs 1 pixel off from tangent may result in visual corruption (if the application makes assumptions about what visual rect was drawn to), and may also result in noticeable visual errors (as is the case in bug 57296). --- dlls/win32u/dibdrv/graphics.c | 160 +++++++++++++++++++--------------- 1 file changed, 88 insertions(+), 72 deletions(-)
diff --git a/dlls/win32u/dibdrv/graphics.c b/dlls/win32u/dibdrv/graphics.c index 9d956e51950..e3c1f7ec3d9 100644 --- a/dlls/win32u/dibdrv/graphics.c +++ b/dlls/win32u/dibdrv/graphics.c @@ -171,16 +171,21 @@ static void add_pen_lines_bounds( dibdrv_physdev *dev, int count, const POINT *p add_clipped_bounds( dev, &bounds, dev->clip ); }
-/* Draw the "top half" of the ellipse, viz. the half that doesn't have any - * points in the lower-right quadrant. - * - * We draw as many as eight arcs; in each we always increment one of X or Y +static void lp_to_dp_double( const DC *dc, double *x, double *y ) +{ + double in_x = *x, in_y = *y; + + *x = in_x * dc->xformWorld2Vport.eM11 + in_y * dc->xformWorld2Vport.eM21 + dc->xformWorld2Vport.eDx; + *y = in_x * dc->xformWorld2Vport.eM12 + in_y * dc->xformWorld2Vport.eM22 + dc->xformWorld2Vport.eDy; +} + +/* We draw as many as eight arcs; in each we always increment one of X or Y * (depending on whether the absolute value of the slope is less than * or greater than 1) and possibly increment the other. * * Loosely based on an algorithm by David Eberly. */ -static unsigned int generate_ellipse_top_half( const DC *dc, double width, double height, POINT *points ) +static unsigned int generate_ellipse_top_half( const DC *dc, const RECT *rect, POINT *points ) { /* A general ellipse centered at (0, 0) can be defined by * ax² + 2bxy + cy² - d = 0. @@ -189,15 +194,19 @@ static unsigned int generate_ellipse_top_half( const DC *dc, double width, doubl * yields an ellipse in the same form. * Precompute the transformed coefficients. */ const XFORM *xform = &dc->xformVport2World; - double wsq = (width / 2) * (width / 2); - double hsq = (height / 2) * (height / 2); + double center_x = (rect->left + rect->right) / 2.0; + double center_y = (rect->top + rect->bottom) / 2.0; + const double width = rect->right - rect->left, height = rect->bottom - rect->top; + const double wsq = (width / 2) * (width / 2); + const double hsq = (height / 2) * (height / 2); const double a = (hsq * xform->eM11 * xform->eM11) + (wsq * xform->eM12 * xform->eM12); const double b = (hsq * xform->eM11 * xform->eM21) + (wsq * xform->eM12 * xform->eM22); const double c = (hsq * xform->eM21 * xform->eM21) + (wsq * xform->eM22 * xform->eM22); const double d = (hsq * wsq); unsigned int pos = 0; - int start_x; - POINT pt; + double x, y, start_x; /* relative coordinates */ + + lp_to_dp_double( dc, ¢er_x, ¢er_y );
/* We start from the point that satisfies y = 0, i.e. ax² = d. * Note that a must be nonzero if the transformation matrix was singular. @@ -207,70 +216,79 @@ static unsigned int generate_ellipse_top_half( const DC *dc, double width, doubl * This point is not on the ellipse axis, but that doesn't matter for our * algorithm. We will take this set of points and rotate it 180 degrees * to get the opposite side. */ - pt.x = -sqrt(d / a); - pt.y = 0; - start_x = pt.x; + x = GDI_ROUND( center_x - sqrt(d / a) ) - center_x; + y = GDI_ROUND( center_y ) - center_y; + start_x = x;
- while (pt.y > 0 || (pt.y == 0 && pt.x != -start_x)) + while (y > 0 || (y == 0 && x != -start_x)) { - double dx = (b * pt.x + c * pt.y); - double dy = -(a * pt.x + b * pt.y); + double dx = (b * x + c * y); + double dy = -(a * x + b * y); int x_inc = (dx > 0 ? 1 : -1); int y_inc = (dy > 0 ? 1 : -1); double sigma1, sigma2;
- points[pos++] = pt; + points[pos].x = GDI_ROUND( center_x + x ); + points[pos].y = GDI_ROUND( center_y + y ); + ++pos;
if (fabs(dy) > fabs(dx)) { /* Increment y, maybe increment x. */ - pt.y += y_inc; + y += y_inc;
/* If this point has 45° slope, and it's directly adjacent to one * we just wrote, using the algorithm as-is will select this point * and then the one horizontally adjacent to it (our reflection * across the 45° line.) This creates an L-shaped corner, which we * don't want. Force incrementing X to skip that corner. */ - if (fabs(b * pt.x + c * pt.y) == fabs(a * pt.x + b * pt.y)) + if (fabs(b * x + c * y) == fabs(a * x + b * y)) { - pt.x += x_inc; + x += x_inc; continue; }
- sigma1 = (a * pt.x * pt.x) + (2 * b * pt.x * pt.y) + (c * pt.y * pt.y) - d; - pt.x += x_inc; - sigma2 = (a * pt.x * pt.x) + (2 * b * pt.x * pt.y) + (c * pt.y * pt.y) - d; + sigma1 = (a * x * x) + (2 * b * x * y) + (c * y * y) - d; + x += x_inc; + sigma2 = (a * x * x) + (2 * b * x * y) + (c * y * y) - d;
/* Pick whichever point is closer to the ellipse. */ if (fabs(sigma2) > fabs(sigma1)) - pt.x -= x_inc; + x -= x_inc; } else { /* Increment x, maybe increment y. */ - pt.x += x_inc; + x += x_inc;
- if (fabs(b * pt.x + c * pt.y) == fabs(a * pt.x + b * pt.y)) + if (fabs(b * x + c * y) == fabs(a * x + b * y)) { - pt.y += y_inc; + y += y_inc; continue; }
- sigma1 = (a * pt.x * pt.x) + (2 * b * pt.x * pt.y) + (c * pt.y * pt.y) - d; - pt.y += y_inc; - sigma2 = (a * pt.x * pt.x) + (2 * b * pt.x * pt.y) + (c * pt.y * pt.y) - d; + sigma1 = (a * x * x) + (2 * b * x * y) + (c * y * y) - d; + y += y_inc; + sigma2 = (a * x * x) + (2 * b * x * y) + (c * y * y) - d;
if (fabs(sigma2) > fabs(sigma1)) - pt.y -= y_inc; + y -= y_inc; } } return pos; }
-static int find_intersection( const POINT *points, double x, double y, int count ) +static int find_intersection( const DC *dc, const POINT *points, const RECT *rect, double x, double y, int count ) { + double center_x = (rect->left + rect->right) / 2.0; + double center_y = (rect->top + rect->bottom) / 2.0; int i;
+ lp_to_dp_double( dc, ¢er_x, ¢er_y ); + lp_to_dp_double( dc, &x, &y ); + x -= center_x; + y -= center_y; + /* First point is always x <= 0, y == 0, so if y >= 0 we are in the * top half. */
@@ -282,7 +300,7 @@ static int find_intersection( const POINT *points, double x, double y, int count /* top half */ for (i = 0; i < count; i++) { - if (points[i].x * y >= points[i].y * x) + if ((points[i].x - center_x) * y >= (points[i].y - center_y) * x) break; } return i; @@ -292,30 +310,21 @@ static int find_intersection( const POINT *points, double x, double y, int count /* bottom half */ for (i = 0; i < count; i++) { - if (points[i].x * -y >= points[i].y * -x) + if ((points[i].x - center_x) * -y >= (points[i].y - center_y) * -x) break; } return count + i; } }
-static void lp_to_dp_no_translate( DC *dc, double *x, double *y ) -{ - double in_x = *x, in_y = *y; - - *x = in_x * dc->xformWorld2Vport.eM11 + in_y * dc->xformWorld2Vport.eM21; - *y = in_x * dc->xformWorld2Vport.eM12 + in_y * dc->xformWorld2Vport.eM22; -} - -static int get_arc_points( DC *dc, int arc_dir, const RECT *rect, double start_x, double start_y, - double end_x, double end_y, POINT *points ) +static int get_arc_points( DC *dc, int arc_dir, const RECT *rect, int start_x, int start_y, + int end_x, int end_y, POINT *points ) { int i, pos, count, start_pos, end_pos; - int width = rect->right - rect->left; - int height = rect->bottom - rect->top; - POINT center = {rect->left + width / 2, rect->top + height / 2}; + POINT reflect; + RECT dp_rect;
- count = generate_ellipse_top_half( dc, width, height, points ); + count = generate_ellipse_top_half( dc, rect, points );
/* The ellipse is always generated counterclockwise from the origin. * This means our points will essentially be backwards if the world @@ -323,13 +332,8 @@ static int get_arc_points( DC *dc, int arc_dir, const RECT *rect, double start_x if (dc->xformWorld2Vport.eM11 * dc->xformWorld2Vport.eM22 < dc->xformWorld2Vport.eM12 * dc->xformWorld2Vport.eM21) arc_dir = (arc_dir == AD_CLOCKWISE ? AD_COUNTERCLOCKWISE : AD_CLOCKWISE);
- /* Transform the start and end, but do not translate them, so that they - * remain relative to the ellipse center. */ - lp_to_dp_no_translate( dc, &start_x, &start_y ); - lp_to_dp_no_translate( dc, &end_x, &end_y ); - - start_pos = find_intersection( points, start_x, start_y, count ); - end_pos = find_intersection( points, end_x, end_y, count ); + start_pos = find_intersection( dc, points, rect, start_x, start_y, count ); + end_pos = find_intersection( dc, points, rect, end_x, end_y, count ); if (arc_dir == AD_CLOCKWISE) { int tmp = start_pos; @@ -338,7 +342,24 @@ static int get_arc_points( DC *dc, int arc_dir, const RECT *rect, double start_x } if (end_pos <= start_pos) end_pos += 2 * count;
- lp_to_dp( dc, ¢er, 1 ); + /* The bottom half of the ellipse can be generated by reflecting each + * point across the ellipse center. In general, this is P' = C + (C - P). + * If C = (L + R) / 2 [for opposite rect corners L and R] then this is + * (L + R - P). + * + * However, we are dealing with already transformed points, and the + * LP to DP transformation f(x) = Mx + B is not linear due to the offset (B) + * component. However, the reflected point f(P') can be expressed as: + * + * f(P') = f(L + R - P) + * = M(L + R - P) + B + * = ML + B + MR + B - MP - B + * = f(L) + f(R) - f(P) + */ + dp_rect = *rect; + lp_to_dp( dc, (POINT *)&dp_rect, 2 ); + reflect.x = dp_rect.left + dp_rect.right; + reflect.y = dp_rect.bottom + dp_rect.top;
pos = count; if (arc_dir == AD_COUNTERCLOCKWISE) @@ -347,13 +368,13 @@ static int get_arc_points( DC *dc, int arc_dir, const RECT *rect, double start_x { if ((i / count) % 2 == 0) { - points[pos].x = center.x + points[i % count].x; - points[pos].y = center.y + points[i % count].y; + points[pos].x = points[i % count].x; + points[pos].y = points[i % count].y; } else { - points[pos].x = center.x - points[i % count].x; - points[pos].y = center.y - points[i % count].y; + points[pos].x = reflect.x - points[i % count].x; + points[pos].y = reflect.y - points[i % count].y; } } } @@ -363,13 +384,13 @@ static int get_arc_points( DC *dc, int arc_dir, const RECT *rect, double start_x { if ((i / count) % 2 == 0) { - points[pos].x = center.x + points[i % count].x; - points[pos].y = center.y + points[i % count].y; + points[pos].x = points[i % count].x; + points[pos].y = points[i % count].y; } else { - points[pos].x = center.x - points[i % count].x; - points[pos].y = center.y - points[i % count].y; + points[pos].x = reflect.x - points[i % count].x; + points[pos].y = reflect.y - points[i % count].y; } } } @@ -416,11 +437,6 @@ static BOOL draw_arc( PHYSDEV dev, INT left, INT top, INT right, INT bottom, pt[0].y = start_y; pt[1].x = end_x; pt[1].y = end_y; - /* make them relative to the ellipse center */ - pt[0].x -= rect.left + width / 2; - pt[0].y -= rect.top + height / 2; - pt[1].x -= rect.left + width / 2; - pt[1].y -= rect.top + height / 2;
max_points = get_arc_max_points( dc, &rect ); points = malloc( max_points * sizeof(*points) ); @@ -1570,12 +1586,12 @@ BOOL dibdrv_RoundRect( PHYSDEV dev, INT left, INT top, INT right, INT bottom, order_rect( &rect );
SetRect( &arc_rect, rect.left, rect.top, rect.left + ellipse_width, rect.top + ellipse_height ); - /* Points are relative to the arc center. - * We just need to specify any point on the vector. */ - count = get_arc_points( dc, AD_CLOCKWISE, &arc_rect, -1.0, 0.0, 0.0, -1.0, top_points ); + count = get_arc_points( dc, AD_CLOCKWISE, &arc_rect, arc_rect.left, (arc_rect.top + arc_rect.bottom) / 2, + (arc_rect.left + arc_rect.right) / 2, arc_rect.top, top_points );
SetRect( &arc_rect, rect.right - ellipse_width, rect.top, rect.right, rect.top + ellipse_height ); - count += get_arc_points( dc, AD_CLOCKWISE, &arc_rect, 0.0, -1.0, 1.0, 0.0, top_points + count ); + count += get_arc_points( dc, AD_CLOCKWISE, &arc_rect, (arc_rect.left + arc_rect.right + 1) / 2, arc_rect.top, + arc_rect.right, (arc_rect.top + arc_rect.bottom) / 2, top_points + count );
if (count * 2 > max_points) ERR( "point count %u * 2 exceeds max points %u\n", count, max_points );
From: Elizabeth Figura zfigura@codeweavers.com
--- dlls/win32u/dibdrv/graphics.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/dlls/win32u/dibdrv/graphics.c b/dlls/win32u/dibdrv/graphics.c index e3c1f7ec3d9..a22022ac613 100644 --- a/dlls/win32u/dibdrv/graphics.c +++ b/dlls/win32u/dibdrv/graphics.c @@ -428,6 +428,19 @@ static BOOL draw_arc( PHYSDEV dev, INT left, INT top, INT right, INT bottom, BOOL ret = TRUE; HRGN outline = 0, interior = 0;
+ if (dc->attr->graphics_mode == GM_COMPATIBLE) + { + /* Exclude the lower right bounds. */ + if (right < left) + --left; + else + --right; + if (bottom < top) + --top; + else + --bottom; + } + SetRect( &rect, left, top, right, bottom );
width = rect.right - rect.left;
From: Elizabeth Figura zfigura@codeweavers.com
--- dlls/win32u/dibdrv/graphics.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/dlls/win32u/dibdrv/graphics.c b/dlls/win32u/dibdrv/graphics.c index a22022ac613..b3e8813d2a7 100644 --- a/dlls/win32u/dibdrv/graphics.c +++ b/dlls/win32u/dibdrv/graphics.c @@ -1576,6 +1576,19 @@ BOOL dibdrv_RoundRect( PHYSDEV dev, INT left, INT top, INT right, INT bottom, if (!ellipse_width || !ellipse_height) return dibdrv_Rectangle( dev, left, top, right, bottom );
+ if (dc->attr->graphics_mode == GM_COMPATIBLE) + { + /* Exclude the lower right bounds. */ + if (right < left) + --left; + else + --right; + if (bottom < top) + --top; + else + --bottom; + } + SetRect( &arc_rect, 0, 0, ellipse_width, ellipse_height ); /* We are drawing four arcs, but the number of points we will actually use * is exactly as many as in one ellipse arc. */
From: Elizabeth Figura zfigura@codeweavers.com
--- dlls/win32u/dibdrv/graphics.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/dlls/win32u/dibdrv/graphics.c b/dlls/win32u/dibdrv/graphics.c index b3e8813d2a7..5f01d9e68e4 100644 --- a/dlls/win32u/dibdrv/graphics.c +++ b/dlls/win32u/dibdrv/graphics.c @@ -430,6 +430,9 @@ static BOOL draw_arc( PHYSDEV dev, INT left, INT top, INT right, INT bottom,
if (dc->attr->graphics_mode == GM_COMPATIBLE) { + if (left == right || top == bottom) + return TRUE; + /* Exclude the lower right bounds. */ if (right < left) --left;
From: Elizabeth Figura zfigura@codeweavers.com
--- dlls/win32u/dibdrv/graphics.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/dlls/win32u/dibdrv/graphics.c b/dlls/win32u/dibdrv/graphics.c index 5f01d9e68e4..43a43a6f9bd 100644 --- a/dlls/win32u/dibdrv/graphics.c +++ b/dlls/win32u/dibdrv/graphics.c @@ -1571,7 +1571,7 @@ BOOL dibdrv_RoundRect( PHYSDEV dev, INT left, INT top, INT right, INT bottom, dibdrv_physdev *pdev = get_dibdrv_pdev( dev ); DC *dc = get_physdev_dc( dev ); RECT rect, arc_rect; - POINT rect_center, *points, *top_points; + POINT reflect, *points, *top_points; int count, max_points; BOOL ret = TRUE; HRGN outline = 0, interior = 0; @@ -1625,9 +1625,10 @@ BOOL dibdrv_RoundRect( PHYSDEV dev, INT left, INT top, INT right, INT bottom, if (count * 2 > max_points) ERR( "point count %u * 2 exceeds max points %u\n", count, max_points );
- rect_center.x = (rect.left + rect.right) / 2; - rect_center.y = (rect.top + rect.bottom) / 2; - lp_to_dp( dc, &rect_center, 1 ); + /* See get_arc_points() on the construction of "reflect". */ + lp_to_dp( dc, (POINT *)&rect, 2 ); + reflect.x = rect.left + rect.right; + reflect.y = rect.bottom + rect.top;
if (dc->attr->arc_direction == AD_CLOCKWISE) { @@ -1635,8 +1636,8 @@ BOOL dibdrv_RoundRect( PHYSDEV dev, INT left, INT top, INT right, INT bottom, { points[i].x = top_points[i].x; points[i].y = top_points[i].y; - points[count + i].x = rect_center.x * 2 - points[i].x; - points[count + i].y = rect_center.y * 2 - points[i].y; + points[count + i].x = reflect.x - points[i].x; + points[count + i].y = reflect.y - points[i].y; } } else @@ -1645,8 +1646,8 @@ BOOL dibdrv_RoundRect( PHYSDEV dev, INT left, INT top, INT right, INT bottom, { points[i].x = top_points[count - 1 - i].x; points[i].y = top_points[count - 1 - i].y; - points[count + i].x = rect_center.x * 2 - points[i].x; - points[count + i].y = rect_center.y * 2 - points[i].y; + points[count + i].x = reflect.x - points[i].x; + points[count + i].y = reflect.y - points[i].y; } }
From: Elizabeth Figura zfigura@codeweavers.com
This was lost in 5924ab4c5155d97f4b23a33e97258c1ec382ea02. --- dlls/win32u/dibdrv/graphics.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/dlls/win32u/dibdrv/graphics.c b/dlls/win32u/dibdrv/graphics.c index 43a43a6f9bd..0a54eec8a0c 100644 --- a/dlls/win32u/dibdrv/graphics.c +++ b/dlls/win32u/dibdrv/graphics.c @@ -1592,6 +1592,9 @@ BOOL dibdrv_RoundRect( PHYSDEV dev, INT left, INT top, INT right, INT bottom, --bottom; }
+ ellipse_width = min( ellipse_width, abs( right + 1 - left )); + ellipse_height = min( ellipse_height, abs( bottom + 1 - top )); + SetRect( &arc_rect, 0, 0, ellipse_width, ellipse_height ); /* We are drawing four arcs, but the number of points we will actually use * is exactly as many as in one ellipse arc. */
From: Elizabeth Figura zfigura@codeweavers.com
Whether native does so is inconsistent, but we need this to ensure that the round rect quarter-arcs we generate actually do include the center on both sides if the size is odd. --- dlls/win32u/dibdrv/graphics.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/dlls/win32u/dibdrv/graphics.c b/dlls/win32u/dibdrv/graphics.c index 0a54eec8a0c..5f715209362 100644 --- a/dlls/win32u/dibdrv/graphics.c +++ b/dlls/win32u/dibdrv/graphics.c @@ -340,6 +340,8 @@ static int get_arc_points( DC *dc, int arc_dir, const RECT *rect, int start_x, i start_pos = end_pos; end_pos = tmp; } + if (end_pos != start_pos) + ++end_pos; if (end_pos <= start_pos) end_pos += 2 * count;
/* The bottom half of the ellipse can be generated by reflecting each
From: Elizabeth Figura zfigura@codeweavers.com
--- dlls/win32u/dibdrv/graphics.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/dlls/win32u/dibdrv/graphics.c b/dlls/win32u/dibdrv/graphics.c index 5f715209362..fb92f137b9b 100644 --- a/dlls/win32u/dibdrv/graphics.c +++ b/dlls/win32u/dibdrv/graphics.c @@ -1601,6 +1601,10 @@ BOOL dibdrv_RoundRect( PHYSDEV dev, INT left, INT top, INT right, INT bottom, /* We are drawing four arcs, but the number of points we will actually use * is exactly as many as in one ellipse arc. */ max_points = get_arc_max_points( dc, &arc_rect ); + /* We may generate the "same" center point of the ellipse twice, once on + * each side of each straight line. Add one point per side to account + * for this. */ + max_points += 4; points = malloc( max_points * sizeof(*points) ); top_points = malloc( max_points * sizeof(*points) ); if (!points || !top_points)
From: Elizabeth Figura zfigura@codeweavers.com
get_arc_points() uses the GM_ADVANCED convention, where a MxN rect is one with M = (right - left + 1), N = (top - bottom + 1). --- dlls/win32u/dibdrv/graphics.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/dlls/win32u/dibdrv/graphics.c b/dlls/win32u/dibdrv/graphics.c index fb92f137b9b..36fe965c4b6 100644 --- a/dlls/win32u/dibdrv/graphics.c +++ b/dlls/win32u/dibdrv/graphics.c @@ -1623,11 +1623,11 @@ BOOL dibdrv_RoundRect( PHYSDEV dev, INT left, INT top, INT right, INT bottom, SetRect( &rect, left, top, right, bottom ); order_rect( &rect );
- SetRect( &arc_rect, rect.left, rect.top, rect.left + ellipse_width, rect.top + ellipse_height ); + SetRect( &arc_rect, rect.left, rect.top, rect.left + ellipse_width - 1, rect.top + ellipse_height - 1 ); count = get_arc_points( dc, AD_CLOCKWISE, &arc_rect, arc_rect.left, (arc_rect.top + arc_rect.bottom) / 2, (arc_rect.left + arc_rect.right) / 2, arc_rect.top, top_points );
- SetRect( &arc_rect, rect.right - ellipse_width, rect.top, rect.right, rect.top + ellipse_height ); + SetRect( &arc_rect, rect.right - ellipse_width + 1, rect.top, rect.right, rect.top + ellipse_height - 1 ); count += get_arc_points( dc, AD_CLOCKWISE, &arc_rect, (arc_rect.left + arc_rect.right + 1) / 2, arc_rect.top, arc_rect.right, (arc_rect.top + arc_rect.bottom) / 2, top_points + count );
From: Elizabeth Figura zfigura@codeweavers.com
--- dlls/win32u/dibdrv/graphics.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/dlls/win32u/dibdrv/graphics.c b/dlls/win32u/dibdrv/graphics.c index 36fe965c4b6..1554f8415d0 100644 --- a/dlls/win32u/dibdrv/graphics.c +++ b/dlls/win32u/dibdrv/graphics.c @@ -216,11 +216,14 @@ static unsigned int generate_ellipse_top_half( const DC *dc, const RECT *rect, P * This point is not on the ellipse axis, but that doesn't matter for our * algorithm. We will take this set of points and rotate it 180 degrees * to get the opposite side. */ - x = GDI_ROUND( center_x - sqrt(d / a) ) - center_x; + if (!height || !width) + x = GDI_ROUND( center_x ) - center_x; + else + x = GDI_ROUND( center_x - sqrt(d / a) ) - center_x; y = GDI_ROUND( center_y ) - center_y; start_x = x;
- while (y > 0 || (y == 0 && x != -start_x)) + do { double dx = (b * x + c * y); double dy = -(a * x + b * y); @@ -274,7 +277,8 @@ static unsigned int generate_ellipse_top_half( const DC *dc, const RECT *rect, P if (fabs(sigma2) > fabs(sigma1)) y -= y_inc; } - } + } while (y > 0 || (y == 0 && x != -start_x)); + return pos; }
From: Elizabeth Figura zfigura@codeweavers.com
--- dlls/gdi32/tests/bitmap.c | 295 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 295 insertions(+)
diff --git a/dlls/gdi32/tests/bitmap.c b/dlls/gdi32/tests/bitmap.c index eb5ef0994d6..47a2ac886f7 100644 --- a/dlls/gdi32/tests/bitmap.c +++ b/dlls/gdi32/tests/bitmap.c @@ -5981,6 +5981,300 @@ static void test_D3DKMTCreateDCFromMemory( void ) ok(ret, "Failed to free memory, error %lu.\n", GetLastError()); }
+static void test_arcs(void) +{ + static const unsigned int dib_width = 100, dib_height = 100; + unsigned int *bits; + HBITMAP bitmap; + XFORM xform; + BOOL ret; + HPEN pen; + HDC dc; + + static const BITMAPINFO bitmap_info = + { + .bmiHeader.biSize = sizeof(BITMAPINFOHEADER), + .bmiHeader.biWidth = dib_width, + .bmiHeader.biHeight = dib_height, + .bmiHeader.biBitCount = 32, + .bmiHeader.biPlanes = 1, + }; + + dc = CreateCompatibleDC( 0 ); + bitmap = CreateDIBSection( dc, &bitmap_info, DIB_RGB_COLORS, (void **)&bits, NULL, 0 ); + SelectObject( dc, bitmap ); + pen = CreatePen( PS_SOLID, 1, 0x111111 ); + SelectObject( dc, pen ); + + SelectObject( dc, CreateSolidBrush(0)); + + /* Don't test exact pixels, since approximating the native arc drawing + * algorithm is difficult. Do test that we're drawing to the right bounds, + * though. */ + + memset( bits, 0, dib_width * dib_height * 4 ); + Arc( dc, 10, 10, 40, 25, 0, 15, 20, 0 ); + + for (unsigned int y = 9; y <= 26; ++y) + { + for (unsigned int x = 9; x <= 41; ++x) + { + int colour = bits[(dib_height - 1 - y) * dib_width + x]; + + if (x < 10 || y < 10 || x >= 40 || y >= 25 || (x < 22 && y < 16)) + ok(!colour, "Got unexpected colour %08x at (%u, %u).\n", colour, x, y); + + if (((x == 24 || x == 25) && (y == 10 || y == 24)) /* top/bottom center */ + || ((x == 10 || x == 39) && y == 17)) /* left/right center */ + ok(colour == 0x111111, "Got unexpected colour %08x at (%u, %u).\n", colour, x, y); + } + } + + memset( bits, 0, dib_width * dib_height * 4 ); + RoundRect( dc, 10, 10, 40, 25, 6, 9 ); + + for (unsigned int y = 9; y <= 26; ++y) + { + for (unsigned int x = 9; x <= 41; ++x) + { + int colour = bits[(dib_height - 1 - y) * dib_width + x]; + + if (x < 10 || y < 10 || x >= 40 || y >= 25) + ok(!colour, "Got unexpected colour %08x at (%u, %u).\n", colour, x, y); + + if (((x >= 13 && x < 37) && (y == 10 || y == 24)) /* top/bottom edge */ + || ((y >= 14 && y < 21) && (x == 10 || x == 39))) /* left/right edge */ + ok(colour == 0x111111, "Got unexpected colour %08x at (%u, %u).\n", colour, x, y); + } + } + + /* Round rect with ellipse sizes larger than the rectangle dimensions. + * This draws the whole ellipse, effectively clamping to the relevant dimensions. */ + + memset( bits, 0, dib_width * dib_height * 4 ); + RoundRect( dc, 10, 10, 40, 25, 6, 20 ); + + for (unsigned int y = 9; y <= 26; ++y) + { + for (unsigned int x = 9; x <= 41; ++x) + { + int colour = bits[(dib_height - 1 - y) * dib_width + x]; + + if (x < 10 || y < 10 || x >= 40 || y >= 25) + ok(!colour, "Got unexpected colour %08x at (%u, %u).\n", colour, x, y); + + if (((x >= 13 && x < 37) && (y == 10 || y == 24)) /* top/bottom edge */ + || ((x == 10 || x == 39) && y == 17)) /* left/right center */ + ok(colour == 0x111111, "Got unexpected colour %08x at (%u, %u).\n", colour, x, y); + } + } + + /* Round rect with a 1-pixel ellipse. */ + + memset( bits, 0, dib_width * dib_height * 4 ); + RoundRect( dc, 10, 10, 40, 25, 1, 1 ); + + for (unsigned int y = 9; y <= 26; ++y) + { + for (unsigned int x = 9; x <= 41; ++x) + { + int colour = bits[(dib_height - 1 - y) * dib_width + x]; + + if (x < 10 || y < 10 || x >= 40 || y >= 25) + ok(!colour, "Got unexpected colour %08x at (%u, %u).\n", colour, x, y); + + if (((x >= 11 && x < 39) && (y == 10 || y == 24)) /* top/bottom edge */ + || ((x == 10 || x == 39) && (y >= 11 && y < 24))) /* left/right edge */ + ok(colour == 0x111111, "Got unexpected colour %08x at (%u, %u).\n", colour, x, y); + } + } + + /* Round rect with a 0-pixel ellipse, which is identical to a simple rect. */ + + memset( bits, 0, dib_width * dib_height * 4 ); + RoundRect( dc, 10, 10, 40, 25, 0, 0 ); + + for (unsigned int y = 9; y <= 26; ++y) + { + for (unsigned int x = 9; x <= 41; ++x) + { + int colour = bits[(dib_height - 1 - y) * dib_width + x]; + + if (((x >= 10 && x < 40) && (y == 10 || y == 24)) /* top/bottom edge */ + || ((x == 10 || x == 39) && (y >= 10 && y < 25))) /* left/right edge */ + ok(colour == 0x111111, "Got unexpected colour %08x at (%u, %u).\n", colour, x, y); + else + ok(!colour, "Got unexpected colour %08x at (%u, %u).\n", colour, x, y); + } + } + + /* 0-pixel arc. */ + + memset( bits, 0, dib_width * dib_height * 4 ); + ret = Arc( dc, 10, 10, 40, 10, 0, 1, 1, 0 ); + ok(ret == TRUE, "Got %d.\n", ret); + + for (unsigned int y = 9; y <= 11; ++y) + { + for (unsigned int x = 9; x <= 41; ++x) + { + int colour = bits[(dib_height - 1 - y) * dib_width + x]; + + ok(!colour, "Got unexpected colour %08x at (%u, %u).\n", colour, x, y); + } + } + + /* Arc with identical start/end points draws the whole ellipse. */ + + memset( bits, 0, dib_width * dib_height * 4 ); + Arc( dc, 10, 10, 40, 25, 0, 1, 0, 1 ); + + for (unsigned int y = 9; y <= 26; ++y) + { + for (unsigned int x = 9; x <= 41; ++x) + { + int colour = bits[(dib_height - 1 - y) * dib_width + x]; + + if (x < 10 || y < 10 || x >= 40 || y >= 25) + ok(!colour, "Got unexpected colour %08x at (%u, %u).\n", colour, x, y); + + if (((x == 24 || x == 25) && (y == 10 || y == 24)) /* top/bottom center */ + || ((x == 10 || x == 39) && y == 17)) /* left/right center */ + ok(colour == 0x111111, "Got unexpected colour %08x at (%u, %u).\n", colour, x, y); + } + } + + /* Flipped arc direction. */ + + SetArcDirection( dc, AD_CLOCKWISE ); + + memset( bits, 0, dib_width * dib_height * 4 ); + Arc( dc, 10, 10, 40, 25, 20, 0, 0, 15 ); + + for (unsigned int y = 9; y <= 26; ++y) + { + for (unsigned int x = 9; x <= 41; ++x) + { + int colour = bits[(dib_height - 1 - y) * dib_width + x]; + + if (x < 10 || y < 10 || x >= 40 || y >= 25 || (x < 22 && y < 16)) + ok(!colour, "Got unexpected colour %08x at (%u, %u).\n", colour, x, y); + + if (((x == 24 || x == 25) && (y == 10 || y == 24)) /* top/bottom center */ + || ((x == 10 || x == 39) && y == 17)) /* left/right center */ + ok(colour == 0x111111, "Got unexpected colour %08x at (%u, %u).\n", colour, x, y); + } + } + + /* Flip the arc rect. */ + + memset( bits, 0, dib_width * dib_height * 4 ); + Arc( dc, 40, 10, 10, 25, 20, 0, 0, 15 ); + + for (unsigned int y = 9; y <= 26; ++y) + { + for (unsigned int x = 9; x <= 41; ++x) + { + int colour = bits[(dib_height - 1 - y) * dib_width + x]; + + if (x < 10 || y < 10 || x >= 40 || y >= 25 || (x < 22 && y < 16)) + ok(!colour, "Got unexpected colour %08x at (%u, %u).\n", colour, x, y); + + if (((x == 24 || x == 25) && (y == 10 || y == 24)) /* top/bottom center */ + || ((x == 10 || x == 39) && y == 17)) /* left/right center */ + ok(colour == 0x111111, "Got unexpected colour %08x at (%u, %u).\n", colour, x, y); + } + } + + /* Test with GM_ADVANCED. */ + + SetGraphicsMode( dc, GM_ADVANCED ); + + memset( bits, 0, dib_width * dib_height * 4 ); + Arc( dc, 10, 10, 40, 25, 20, 0, 0, 14 ); + + for (unsigned int y = 9; y <= 26; ++y) + { + for (unsigned int x = 9; x <= 41; ++x) + { + int colour = bits[(dib_height - 1 - y) * dib_width + x]; + + if (x < 10 || y < 10 || x > 40 || y > 25 || (x < 22 && y < 16)) + ok(!colour, "Got unexpected colour %08x at (%u, %u).\n", colour, x, y); + + if ((x == 25 && (y == 10 || y == 25)) /* top/bottom center */ + || ((x == 10 || x == 40) && (y == 17 || y == 18))) /* left/right center */ + ok(colour == 0x111111, "Got unexpected colour %08x at (%u, %u).\n", colour, x, y); + } + } + + memset( bits, 0, dib_width * dib_height * 4 ); + RoundRect( dc, 10, 10, 40, 25, 6, 9 ); + + for (unsigned int y = 9; y <= 26; ++y) + { + for (unsigned int x = 9; x <= 41; ++x) + { + int colour = bits[(dib_height - 1 - y) * dib_width + x]; + + if (x < 10 || y < 10 || x > 40 || y > 25) + ok(!colour, "Got unexpected colour %08x at (%u, %u).\n", colour, x, y); + + if (((x >= 13 && x <= 37) && (y == 10 || y == 25)) /* top/bottom edge */ + || ((y >= 14 && y <= 21) && (x == 10 || x == 40))) /* left/right edge */ + ok(colour == 0x111111, "Got unexpected colour %08x at (%u, %u).\n", colour, x, y); + } + } + + /* Flip the world transform. */ + + memset( &xform, 0, sizeof(xform) ); + xform.eM11 = 1.0f; + xform.eM22 = -1.0f; + xform.eDy = 35.0f; + SetWorldTransform( dc, &xform ); + + memset( bits, 0, dib_width * dib_height * 4 ); + Arc( dc, 10, 10, 40, 25, 20, 0, 0, 14 ); + + for (unsigned int y = 9; y <= 26; ++y) + { + for (unsigned int x = 9; x <= 41; ++x) + { + int colour = bits[(dib_height - 1 - y) * dib_width + x]; + + if (x < 10 || y < 10 || x > 40 || y > 25 || (x < 22 && y > 19)) + ok(!colour, "Got unexpected colour %08x at (%u, %u).\n", colour, x, y); + + if ((x == 25 && (y == 10 || y == 25)) /* top/bottom center */ + || ((x == 10 || x == 40) && (y == 17 || y == 18))) /* left/right center */ + ok(colour == 0x111111, "Got unexpected colour %08x at (%u, %u).\n", colour, x, y); + } + } + + memset( bits, 0, dib_width * dib_height * 4 ); + RoundRect( dc, 10, 10, 40, 25, 6, 9 ); + + for (unsigned int y = 9; y <= 26; ++y) + { + for (unsigned int x = 9; x <= 41; ++x) + { + int colour = bits[(dib_height - 1 - y) * dib_width + x]; + + if (x < 10 || y < 10 || x > 40 || y > 25) + ok(!colour, "Got unexpected colour %08x at (%u, %u).\n", colour, x, y); + + if (((x >= 13 && x <= 37) && (y == 10 || y == 25)) /* top/bottom edge */ + || ((y >= 14 && y <= 21) && (x == 10 || x == 40))) /* left/right edge */ + ok(colour == 0x111111, "Got unexpected colour %08x at (%u, %u).\n", colour, x, y); + } + } + + DeleteObject( bitmap ); + DeleteObject( pen ); + DeleteDC( dc ); +} + START_TEST(bitmap) { HMODULE hdll; @@ -6027,4 +6321,5 @@ START_TEST(bitmap) test_SetDIBitsToDevice(); test_SetDIBitsToDevice_RLE8(); test_D3DKMTCreateDCFromMemory(); + test_arcs(); }
From: Elizabeth Figura zfigura@codeweavers.com
Windows generates L-shaped corners. --- dlls/win32u/dibdrv/graphics.c | 17 ----------------- 1 file changed, 17 deletions(-)
diff --git a/dlls/win32u/dibdrv/graphics.c b/dlls/win32u/dibdrv/graphics.c index 1554f8415d0..c629a59ae1b 100644 --- a/dlls/win32u/dibdrv/graphics.c +++ b/dlls/win32u/dibdrv/graphics.c @@ -240,17 +240,6 @@ static unsigned int generate_ellipse_top_half( const DC *dc, const RECT *rect, P /* Increment y, maybe increment x. */ y += y_inc;
- /* If this point has 45° slope, and it's directly adjacent to one - * we just wrote, using the algorithm as-is will select this point - * and then the one horizontally adjacent to it (our reflection - * across the 45° line.) This creates an L-shaped corner, which we - * don't want. Force incrementing X to skip that corner. */ - if (fabs(b * x + c * y) == fabs(a * x + b * y)) - { - x += x_inc; - continue; - } - sigma1 = (a * x * x) + (2 * b * x * y) + (c * y * y) - d; x += x_inc; sigma2 = (a * x * x) + (2 * b * x * y) + (c * y * y) - d; @@ -264,12 +253,6 @@ static unsigned int generate_ellipse_top_half( const DC *dc, const RECT *rect, P /* Increment x, maybe increment y. */ x += x_inc;
- if (fabs(b * x + c * y) == fabs(a * x + b * y)) - { - y += y_inc; - continue; - } - sigma1 = (a * x * x) + (2 * b * x * y) + (c * y * y) - d; y += y_inc; sigma2 = (a * x * x) + (2 * b * x * y) + (c * y * y) - d;
From: Elizabeth Figura zfigura@codeweavers.com
--- dlls/gdi32/tests/bitmap.c | 23 ++++++++++++++++++++++- dlls/win32u/dibdrv/graphics.c | 4 ++-- 2 files changed, 24 insertions(+), 3 deletions(-)
diff --git a/dlls/gdi32/tests/bitmap.c b/dlls/gdi32/tests/bitmap.c index 47a2ac886f7..2284f1f5e23 100644 --- a/dlls/gdi32/tests/bitmap.c +++ b/dlls/gdi32/tests/bitmap.c @@ -6039,7 +6039,28 @@ static void test_arcs(void) { int colour = bits[(dib_height - 1 - y) * dib_width + x];
- if (x < 10 || y < 10 || x >= 40 || y >= 25) + if (x < 10 || y < 10 || x >= 40 || y >= 25 + || ((x == 10 || x == 39) && (y == 10 || y == 24))) /* corners */ + ok(!colour, "Got unexpected colour %08x at (%u, %u).\n", colour, x, y); + + if (((x >= 13 && x < 37) && (y == 10 || y == 24)) /* top/bottom edge */ + || ((y >= 14 && y < 21) && (x == 10 || x == 39))) /* left/right edge */ + ok(colour == 0x111111, "Got unexpected colour %08x at (%u, %u).\n", colour, x, y); + } + } + + /* Same round rect, but with the ellipse width and height inverted. */ + memset( bits, 0, dib_width * dib_height * 4 ); + RoundRect( dc, 10, 10, 40, 25, -6, -9 ); + + for (unsigned int y = 9; y <= 26; ++y) + { + for (unsigned int x = 9; x <= 41; ++x) + { + int colour = bits[(dib_height - 1 - y) * dib_width + x]; + + if (x < 10 || y < 10 || x >= 40 || y >= 25 + || ((x == 10 || x == 39) && (y == 10 || y == 24))) /* corners */ ok(!colour, "Got unexpected colour %08x at (%u, %u).\n", colour, x, y);
if (((x >= 13 && x < 37) && (y == 10 || y == 24)) /* top/bottom edge */ diff --git a/dlls/win32u/dibdrv/graphics.c b/dlls/win32u/dibdrv/graphics.c index c629a59ae1b..21085a1d84a 100644 --- a/dlls/win32u/dibdrv/graphics.c +++ b/dlls/win32u/dibdrv/graphics.c @@ -1581,8 +1581,8 @@ BOOL dibdrv_RoundRect( PHYSDEV dev, INT left, INT top, INT right, INT bottom, --bottom; }
- ellipse_width = min( ellipse_width, abs( right + 1 - left )); - ellipse_height = min( ellipse_height, abs( bottom + 1 - top )); + ellipse_width = min( abs( ellipse_width ), abs( right - left ) + 1 ); + ellipse_height = min( abs( ellipse_height ), abs( bottom - top ) + 1 );
SetRect( &arc_rect, 0, 0, ellipse_width, ellipse_height ); /* We are drawing four arcs, but the number of points we will actually use
From: Elizabeth Figura zfigura@codeweavers.com
--- dlls/win32u/dibdrv/graphics.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/dlls/win32u/dibdrv/graphics.c b/dlls/win32u/dibdrv/graphics.c index 21085a1d84a..df62bc1dea5 100644 --- a/dlls/win32u/dibdrv/graphics.c +++ b/dlls/win32u/dibdrv/graphics.c @@ -1565,9 +1565,6 @@ BOOL dibdrv_RoundRect( PHYSDEV dev, INT left, INT top, INT right, INT bottom, BOOL ret = TRUE; HRGN outline = 0, interior = 0;
- if (!ellipse_width || !ellipse_height) - return dibdrv_Rectangle( dev, left, top, right, bottom ); - if (dc->attr->graphics_mode == GM_COMPATIBLE) { /* Exclude the lower right bounds. */ @@ -1584,6 +1581,9 @@ BOOL dibdrv_RoundRect( PHYSDEV dev, INT left, INT top, INT right, INT bottom, ellipse_width = min( abs( ellipse_width ), abs( right - left ) + 1 ); ellipse_height = min( abs( ellipse_height ), abs( bottom - top ) + 1 );
+ if (ellipse_width <= 1 || ellipse_height <= 1) + return dibdrv_Rectangle( dev, left, top, right, bottom ); + SetRect( &arc_rect, 0, 0, ellipse_width, ellipse_height ); /* We are drawing four arcs, but the number of points we will actually use * is exactly as many as in one ellipse arc. */
Hi,
It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated.
The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=150714
Your paranoid android.
=== build (build log) ===
error: patch failed: dlls/win32u/dibdrv/graphics.c:171 error: patch failed: dlls/win32u/dibdrv/graphics.c:340 error: patch failed: dlls/win32u/dibdrv/graphics.c:1623 error: patch failed: dlls/win32u/dibdrv/graphics.c:216 error: patch failed: dlls/win32u/dibdrv/graphics.c:240 Task: Patch failed to apply
=== debian11 (build log) ===
error: patch failed: dlls/win32u/dibdrv/graphics.c:171 error: patch failed: dlls/win32u/dibdrv/graphics.c:340 error: patch failed: dlls/win32u/dibdrv/graphics.c:1623 error: patch failed: dlls/win32u/dibdrv/graphics.c:216 error: patch failed: dlls/win32u/dibdrv/graphics.c:240 Task: Patch failed to apply
=== debian11b (build log) ===
error: patch failed: dlls/win32u/dibdrv/graphics.c:171 error: patch failed: dlls/win32u/dibdrv/graphics.c:340 error: patch failed: dlls/win32u/dibdrv/graphics.c:1623 error: patch failed: dlls/win32u/dibdrv/graphics.c:216 error: patch failed: dlls/win32u/dibdrv/graphics.c:240 Task: Patch failed to apply
Apparently there are still issues: https://bugs.winehq.org/show_bug.cgi?id=57579#c8
I don't have access to Multisim, but I've added two commits which fix bugs I found in LTspice and Microsoft Paint from Windows XP.
We've decided to revert the existing arc changes (!7101) and will defer this MR until after the 10.0 release.
This merge request was closed by Elizabeth Figura.
-
Elizabeth Figura -
Elizabeth Figura (＠zfigura)
-
Huw Davies (＠huw)
-
Marvin