Skip to content

Fix aalines overlap #2912

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Oct 3, 2024
Merged

Fix aalines overlap #2912

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
63d3f3f
Fixed draw.aalines overlaping
mzivic7 Jun 5, 2024
552725d
Added missing pixel
mzivic7 Jun 6, 2024
c5c48ba
Added tests
mzivic7 Jun 19, 2024
29f43fa
Added this pr url to tests
mzivic7 Jun 20, 2024
5daf458
Merge branch 'pygame-community:main' into fix_aalines_overlap
mzivic7 Sep 5, 2024
a94e88d
Fixed gaps in line and simplified code
mzivic7 Sep 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
197 changes: 141 additions & 56 deletions src_c/draw.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,9 @@ draw_line(SDL_Surface *surf, int x1, int y1, int x2, int y2, Uint32 color,
int *drawn_area);
static void
draw_aaline(SDL_Surface *surf, Uint32 color, float startx, float starty,
float endx, float endy, int *drawn_area);
float endx, float endy, int *drawn_area,
int disable_first_endpoint, int disable_second_endpoint,
int extra_pixel_for_aalines);
static void
draw_arc(SDL_Surface *surf, int x_center, int y_center, int radius1,
int radius2, int width, double angle_start, double angle_stop,
Expand Down Expand Up @@ -159,7 +161,7 @@ aaline(PyObject *self, PyObject *arg, PyObject *kwargs)
return RAISE(PyExc_RuntimeError, "error locking surface");
}

draw_aaline(surf, color, startx, starty, endx, endy, drawn_area);
draw_aaline(surf, color, startx, starty, endx, endy, drawn_area, 0, 0, 0);

if (!pgSurface_Unlock(surfobj)) {
return RAISE(PyExc_RuntimeError, "error unlocking surface");
Expand Down Expand Up @@ -255,9 +257,13 @@ aalines(PyObject *self, PyObject *arg, PyObject *kwargs)
SDL_Surface *surf = NULL;
Uint32 color;
float pts[4];
float pts_prev[4];
float *xlist, *ylist;
float x, y;
int l, t;
int extra_px;
int steep_prev;
int steep_curr;
PyObject *blend = NULL;
int drawn_area[4] = {INT_MAX, INT_MAX, INT_MIN,
INT_MIN}; /* Used to store bounding box values */
Expand Down Expand Up @@ -344,19 +350,84 @@ aalines(PyObject *self, PyObject *arg, PyObject *kwargs)
return RAISE(PyExc_RuntimeError, "error locking surface");
}

for (loop = 1; loop < length; ++loop) {
/* first line - if open, add endpoint pixels.*/
pts[0] = xlist[0];
pts[1] = ylist[0];
pts[2] = xlist[1];
pts[3] = ylist[1];

/* Previous points.
* Used to compare previous and current line.*/
pts_prev[0] = pts[0];
pts_prev[1] = pts[1];
pts_prev[2] = pts[2];
pts_prev[3] = pts[3];
steep_prev =
fabs(pts_prev[2] - pts_prev[0]) < fabs(pts_prev[3] - pts_prev[1]);
steep_curr = fabs(xlist[2] - pts[2]) < fabs(ylist[2] - pts[1]);
extra_px = steep_prev > steep_curr;
if (closed) {
draw_aaline(surf, color, pts[0], pts[1], pts[2], pts[3], drawn_area, 1,
1, extra_px);
}
else {
draw_aaline(surf, color, pts[0], pts[1], pts[2], pts[3], drawn_area, 0,
1, extra_px);
}

for (loop = 2; loop < length - 1; ++loop) {
pts[0] = xlist[loop - 1];
pts[1] = ylist[loop - 1];
pts[2] = xlist[loop];
pts[3] = ylist[loop];
draw_aaline(surf, color, pts[0], pts[1], pts[2], pts[3], drawn_area);

/* Comparing previous and current line.
* If one is steep and other is not, extra pixel must be drawn.*/
steep_prev =
fabs(pts_prev[2] - pts_prev[0]) < fabs(pts_prev[3] - pts_prev[1]);
steep_curr = fabs(pts[2] - pts[0]) < fabs(pts[3] - pts[1]);
extra_px = steep_prev != steep_curr;
pts_prev[0] = pts[0];
pts_prev[1] = pts[1];
pts_prev[2] = pts[2];
pts_prev[3] = pts[3];
draw_aaline(surf, color, pts[0], pts[1], pts[2], pts[3], drawn_area, 1,
1, extra_px);
}

/* Last line - if open, add endpoint pixels. */
pts[0] = xlist[length - 2];
pts[1] = ylist[length - 2];
pts[2] = xlist[length - 1];
pts[3] = ylist[length - 1];
steep_prev =
fabs(pts_prev[2] - pts_prev[0]) < fabs(pts_prev[3] - pts_prev[1]);
steep_curr = fabs(pts[2] - pts[0]) < fabs(pts[3] - pts[1]);
extra_px = steep_prev != steep_curr;
pts_prev[0] = pts[0];
pts_prev[1] = pts[1];
pts_prev[2] = pts[2];
pts_prev[3] = pts[3];
if (closed) {
draw_aaline(surf, color, pts[0], pts[1], pts[2], pts[3], drawn_area, 1,
1, extra_px);
}
else {
draw_aaline(surf, color, pts[0], pts[1], pts[2], pts[3], drawn_area, 1,
0, extra_px);
}

if (closed && length > 2) {
pts[0] = xlist[length - 1];
pts[1] = ylist[length - 1];
pts[2] = xlist[0];
pts[3] = ylist[0];
draw_aaline(surf, color, pts[0], pts[1], pts[2], pts[3], drawn_area);
steep_prev =
fabs(pts_prev[2] - pts_prev[0]) < fabs(pts_prev[3] - pts_prev[1]);
steep_curr = fabs(pts[2] - pts[0]) < fabs(pts[3] - pts[1]);
extra_px = steep_prev != steep_curr;
draw_aaline(surf, color, pts[0], pts[1], pts[2], pts[3], drawn_area, 1,
1, extra_px);
}

PyMem_Free(xlist);
Expand Down Expand Up @@ -1249,7 +1320,9 @@ set_and_check_rect(SDL_Surface *surf, int x, int y, Uint32 color,

static void
draw_aaline(SDL_Surface *surf, Uint32 color, float from_x, float from_y,
float to_x, float to_y, int *drawn_area)
float to_x, float to_y, int *drawn_area,
int disable_first_endpoint, int disable_second_endpoint,
int extra_pixel_for_aalines)
{
float gradient, dx, dy, intersect_y, brightness;
int x, x_pixel_start, x_pixel_end;
Expand Down Expand Up @@ -1352,68 +1425,80 @@ draw_aaline(SDL_Surface *surf, Uint32 color, float from_x, float from_y,

/* Handle endpoints separately.
* The line is not a mathematical line of thickness zero. The same
* goes for the endpoints. The have a height and width of one pixel. */
* goes for the endpoints. The have a height and width of one pixel.
* Extra pixel drawing is requested externally from aalines.
* It is drawn only when one line is steep and other is not.*/
/* First endpoint */
x_pixel_start = (int)from_x;
y_endpoint = intersect_y = from_y + gradient * (x_pixel_start - from_x);
if (to_x > clip_left + 1.0f) {
x_gap = 1 + x_pixel_start - from_x;
brightness = y_endpoint - (int)y_endpoint;
if (steep) {
x = (int)y_endpoint;
y = x_pixel_start;
}
else {
x = x_pixel_start;
y = (int)y_endpoint;
}
if ((int)y_endpoint < y_endpoint) {
if (!disable_first_endpoint || extra_pixel_for_aalines) {
x_pixel_start = (int)from_x;
y_endpoint = intersect_y =
from_y + gradient * (x_pixel_start - from_x);
if (to_x > clip_left + 1.0f) {
x_gap = 1 + x_pixel_start - from_x;
brightness = y_endpoint - (int)y_endpoint;
if (steep) {
x = (int)y_endpoint;
y = x_pixel_start;
}
else {
x = x_pixel_start;
y = (int)y_endpoint;
}
if ((int)y_endpoint < y_endpoint) {
pixel_color = get_antialiased_color(surf, x, y, color,
brightness * x_gap);
set_and_check_rect(surf, x, y, pixel_color, drawn_area);
}
if (steep) {
x--;
}
else {
y--;
}
brightness = 1 - brightness;
pixel_color =
get_antialiased_color(surf, x, y, color, brightness * x_gap);
set_and_check_rect(surf, x, y, pixel_color, drawn_area);
intersect_y += gradient;
x_pixel_start++;
}
if (steep) {
x--;
}
else {
y--;
}
brightness = 1 - brightness;
pixel_color =
get_antialiased_color(surf, x, y, color, brightness * x_gap);
set_and_check_rect(surf, x, y, pixel_color, drawn_area);
intersect_y += gradient;
x_pixel_start++;
}
/* To be sure main loop skips first endpoint.*/
if (disable_first_endpoint) {
x_pixel_start = (int)ceil(from_x);
intersect_y = from_y + gradient * (x_pixel_start - from_x);
}
/* Second endpoint */
x_pixel_end = (int)ceil(to_x);
if (from_x < clip_right - 1.0f) {
y_endpoint = to_y + gradient * (x_pixel_end - to_x);
x_gap = 1 - x_pixel_end + to_x;
brightness = y_endpoint - (int)y_endpoint;
if (steep) {
x = (int)y_endpoint;
y = x_pixel_end;
}
else {
x = x_pixel_end;
y = (int)y_endpoint;
}
if ((int)y_endpoint < y_endpoint) {
if (!disable_second_endpoint || extra_pixel_for_aalines) {
if (from_x < clip_right - 1.0f) {
y_endpoint = to_y + gradient * (x_pixel_end - to_x);
x_gap = 1 - x_pixel_end + to_x;
brightness = y_endpoint - (int)y_endpoint;
if (steep) {
x = (int)y_endpoint;
y = x_pixel_end;
}
else {
x = x_pixel_end;
y = (int)y_endpoint;
}
if ((int)y_endpoint < y_endpoint) {
pixel_color = get_antialiased_color(surf, x, y, color,
brightness * x_gap);
set_and_check_rect(surf, x, y, pixel_color, drawn_area);
}
if (steep) {
x--;
}
else {
y--;
}
brightness = 1 - brightness;
pixel_color =
get_antialiased_color(surf, x, y, color, brightness * x_gap);
set_and_check_rect(surf, x, y, pixel_color, drawn_area);
}
if (steep) {
x--;
}
else {
y--;
}
brightness = 1 - brightness;
pixel_color =
get_antialiased_color(surf, x, y, color, brightness * x_gap);
set_and_check_rect(surf, x, y, pixel_color, drawn_area);
}

/* main line drawing loop */
Expand Down
54 changes: 54 additions & 0 deletions test/draw_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3646,6 +3646,60 @@ class DrawAALinesTest(AALinesMixin, DrawTestCase):
class to add any draw.aalines specific tests to.
"""

def test_aalines__overlap(self):
"""Ensures that two adjacent antialiased lines are not overlapping.

Draws two lines, and checks if 2 pixels, at shared point between those
two lines, are too bright.

See: https://github.com/pygame-community/pygame-ce/pull/2912
"""
line_color = (150, 150, 150)
max_expected_colors = ((70, 70, 70), (100, 100, 100))
test_points = [[20.1, 25.4], [25.1, 25.6], [30.1, 25.8]]

surface = pygame.display.set_mode((50, 50))
self.draw_aalines(surface, line_color, False, test_points)

for i, y in enumerate((25, 26)):
check_color = tuple(surface.get_at((25, y)))
self.assertLess(
check_color,
max_expected_colors[i],
f"aalines are overlapping, pos={(25, y)}",
)

def test_aalines__steep_missing_pixel(self):
"""Ensures there are no missing pixels between steep and non-steep lines.

Draws two adjacent lines: first is not steep and second is, then
checks if there is missing pixel at shared point between those two lines.

See: https://github.com/pygame-community/pygame-ce/pull/2912
"""
line_color = (150, 150, 150)
min_expected_color = (40, 40, 40)
test_points = [[11.2, 8.5], [17.1, 25.7], [35.8, 25.5], [47.6, 41.8]]

surface = pygame.display.set_mode((50, 50))
self.draw_aalines(surface, line_color, False, test_points)

# First line is steep, and other line is not steep
check_color = tuple(surface.get_at((17, 26)))
self.assertGreater(
check_color,
min_expected_color,
"Pixel is missing between steep and non-steep line, pos=(17, 26)",
)

# First line is not steep, and other line is steep
check_color = tuple(surface.get_at((36, 25)))
self.assertGreater(
check_color,
min_expected_color,
"Pixel is missing between non-steep and steep line, pos=(26, 25)",
)


### Polygon Testing ###########################################################

Expand Down
Loading