From ca9a32171573a89679d0d96e14f85524387d187a Mon Sep 17 00:00:00 2001
From: "Ryan C. Gordon" <[EMAIL REDACTED]>
Date: Sun, 19 Sep 2021 15:47:24 -0400
Subject: [PATCH] render: GL/GLES now draw lines almost perfectly matching
software renderer.
One place known to differ in a significant way is a single line segment that
starts and ends on the same point; the GL renderers will light up a single
pixel here, whereas the software renderer will not. My current belief is this
is a bug in the software renderer, based on the wording of the docs:
"SDL_RenderDrawLine() draws the line to include both end points."
You can see an example program that triggers that difference in Bug #2006.
As it stands, the GL renderers might _also_ render diagonal lines differently,
as the the Bresenham step might vary between implementations (one does three
pixels and then two, the other does two and then three, etc). But this patch
causes those lines to start and end on the correct pixel, and that's the best
we can do, and all anyone really needs here.
Not closing any bugs with this patch (yet!), but here are several that it
appears to fix. If no other corner cases pop up, we'll call this done.
Reference Bug #2006.
Reference Bug #1626.
Reference Bug #4001.
...and probably others...
---
src/render/opengl/SDL_render_gl.c | 51 ++++++++++++-----------
src/render/opengles/SDL_render_gles.c | 51 ++++++++++++-----------
src/render/opengles2/SDL_render_gles2.c | 54 ++++++++++++-------------
3 files changed, 76 insertions(+), 80 deletions(-)
diff --git a/src/render/opengl/SDL_render_gl.c b/src/render/opengl/SDL_render_gl.c
index 758166a985..d58ef1d6a2 100644
--- a/src/render/opengl/SDL_render_gl.c
+++ b/src/render/opengl/SDL_render_gl.c
@@ -890,39 +890,38 @@ static int
GL_QueueDrawLines(SDL_Renderer * renderer, SDL_RenderCommand *cmd, const SDL_FPoint * points, int count)
{
int i;
+ GLfloat prevx, prevy;
const size_t vertlen = (sizeof (GLfloat) * 2) * count;
GLfloat *verts = (GLfloat *) SDL_AllocateRenderVertices(renderer, vertlen, 0, &cmd->data.draw.first);
+
if (!verts) {
return -1;
}
cmd->data.draw.count = count;
- /* Offset to hit the center of the pixel. */
- for (i = 0; i < count; i++) {
- *(verts++) = 0.5f + points[i].x;
- *(verts++) = 0.5f + points[i].y;
- }
-
- /* Make the last line segment one pixel longer, to satisfy the
- diamond-exit rule. */
- verts -= 4;
- {
- const GLfloat xstart = verts[0];
- const GLfloat ystart = verts[1];
- const GLfloat xend = verts[2];
- const GLfloat yend = verts[3];
-
- if (ystart == yend) { /* horizontal line */
- verts[(xend > xstart) ? 2 : 0] += 1.0f;
- } else if (xstart == xend) { /* vertical line */
- verts[(yend > ystart) ? 3 : 1] += 1.0f;
- } else { /* bump a pixel in the direction we are moving in. */
- const GLfloat deltax = xend - xstart;
- const GLfloat deltay = yend - ystart;
- const GLfloat angle = SDL_atan2f(deltay, deltax);
- verts[2] += SDL_cosf(angle);
- verts[3] += SDL_sinf(angle);
- }
+ /* 0.5f offset to hit the center of the pixel. */
+ prevx = 0.5f + points->x;
+ prevy = 0.5f + points->y;
+ *(verts++) = prevx;
+ *(verts++) = prevy;
+
+ /* bump the end of each line segment out a quarter of a pixel, to provoke
+ the diamond-exit rule. Without this, you won't just drop the last
+ pixel of the last line segment, but you might also drop pixels at the
+ edge of any given line segment along the way too. */
+ for (i = 1; i < count; i++) {
+ const GLfloat xstart = prevx;
+ const GLfloat ystart = prevy;
+ const GLfloat xend = points[i].x + 0.5f; /* 0.5f to hit pixel center. */
+ const GLfloat yend = points[i].y + 0.5f;
+ /* bump a little in the direction we are moving in. */
+ const GLfloat deltax = xend - xstart;
+ const GLfloat deltay = yend - ystart;
+ const GLfloat angle = SDL_atan2f(deltay, deltax);
+ prevx = xend + (SDL_cosf(angle) * 0.25f);
+ prevy = yend + (SDL_sinf(angle) * 0.25f);
+ *(verts++) = prevx;
+ *(verts++) = prevy;
}
return 0;
diff --git a/src/render/opengles/SDL_render_gles.c b/src/render/opengles/SDL_render_gles.c
index c0a2ce9dc8..4fa256ff43 100644
--- a/src/render/opengles/SDL_render_gles.c
+++ b/src/render/opengles/SDL_render_gles.c
@@ -564,39 +564,38 @@ static int
GLES_QueueDrawLines(SDL_Renderer * renderer, SDL_RenderCommand *cmd, const SDL_FPoint * points, int count)
{
int i;
+ GLfloat prevx, prevy;
const size_t vertlen = (sizeof (GLfloat) * 2) * count;
GLfloat *verts = (GLfloat *) SDL_AllocateRenderVertices(renderer, vertlen, 0, &cmd->data.draw.first);
+
if (!verts) {
return -1;
}
cmd->data.draw.count = count;
- /* Offset to hit the center of the pixel. */
- for (i = 0; i < count; i++) {
- *(verts++) = 0.5f + points[i].x;
- *(verts++) = 0.5f + points[i].y;
- }
-
- /* Make the last line segment one pixel longer, to satisfy the
- diamond-exit rule. */
- verts -= 4;
- {
- const GLfloat xstart = verts[0];
- const GLfloat ystart = verts[1];
- const GLfloat xend = verts[2];
- const GLfloat yend = verts[3];
-
- if (ystart == yend) { /* horizontal line */
- verts[(xend > xstart) ? 2 : 0] += 1.0f;
- } else if (xstart == xend) { /* vertical line */
- verts[(yend > ystart) ? 3 : 1] += 1.0f;
- } else { /* bump a pixel in the direction we are moving in. */
- const GLfloat deltax = xend - xstart;
- const GLfloat deltay = yend - ystart;
- const GLfloat angle = SDL_atan2f(deltay, deltax);
- verts[2] += SDL_cosf(angle);
- verts[3] += SDL_sinf(angle);
- }
+ /* 0.5f offset to hit the center of the pixel. */
+ prevx = 0.5f + points->x;
+ prevy = 0.5f + points->y;
+ *(verts++) = prevx;
+ *(verts++) = prevy;
+
+ /* bump the end of each line segment out a quarter of a pixel, to provoke
+ the diamond-exit rule. Without this, you won't just drop the last
+ pixel of the last line segment, but you might also drop pixels at the
+ edge of any given line segment along the way too. */
+ for (i = 1; i < count; i++) {
+ const GLfloat xstart = prevx;
+ const GLfloat ystart = prevy;
+ const GLfloat xend = points[i].x + 0.5f; /* 0.5f to hit pixel center. */
+ const GLfloat yend = points[i].y + 0.5f;
+ /* bump a little in the direction we are moving in. */
+ const GLfloat deltax = xend - xstart;
+ const GLfloat deltay = yend - ystart;
+ const GLfloat angle = SDL_atan2f(deltay, deltax);
+ prevx = xend + (SDL_cosf(angle) * 0.25f);
+ prevy = yend + (SDL_sinf(angle) * 0.25f);
+ *(verts++) = prevx;
+ *(verts++) = prevy;
}
return 0;
diff --git a/src/render/opengles2/SDL_render_gles2.c b/src/render/opengles2/SDL_render_gles2.c
index 7ea703d8fc..11e89c6946 100644
--- a/src/render/opengles2/SDL_render_gles2.c
+++ b/src/render/opengles2/SDL_render_gles2.c
@@ -691,9 +691,9 @@ GLES2_QueueDrawLines(SDL_Renderer * renderer, SDL_RenderCommand *cmd, const SDL_
const SDL_bool colorswap = (renderer->target && (renderer->target->format == SDL_PIXELFORMAT_ARGB8888 || renderer->target->format == SDL_PIXELFORMAT_RGB888));
int color;
int i;
- const size_t vertlen = (2 * sizeof (float) + sizeof (int)) * count;
+ GLfloat prevx, prevy;
+ const size_t vertlen = ((2 * sizeof (float)) + sizeof (int)) * count;
GLfloat *verts = (GLfloat *) SDL_AllocateRenderVertices(renderer, vertlen, 0, &cmd->data.draw.first);
-
if (!verts) {
return -1;
}
@@ -706,33 +706,31 @@ GLES2_QueueDrawLines(SDL_Renderer * renderer, SDL_RenderCommand *cmd, const SDL_
cmd->data.draw.count = count;
- /* Offset to hit the center of the pixel. */
- for (i = 0; i < count; i++) {
- *(verts++) = 0.5f + points[i].x;
- *(verts++) = 0.5f + points[i].y;
- *((int *)verts++) = color;
- }
+ /* 0.5f offset to hit the center of the pixel. */
+ prevx = 0.5f + points->x;
+ prevy = 0.5f + points->y;
+ *(verts++) = prevx;
+ *(verts++) = prevy;
+ *((int *)verts++) = color;
- /* Make the last line segment one pixel longer, to satisfy the
- diamond-exit rule. */
- verts -= 3 + 3;
- {
- const GLfloat xstart = verts[0];
- const GLfloat ystart = verts[1];
- const GLfloat xend = verts[3 + 0];
- const GLfloat yend = verts[3 + 1];
-
- if (ystart == yend) { /* horizontal line */
- verts[(xend > xstart) ? 3 + 0: 0] += 1.0f;
- } else if (xstart == xend) { /* vertical line */
- verts[(yend > ystart) ? 3 + 1: 1] += 1.0f;
- } else { /* bump a pixel in the direction we are moving in. */
- const GLfloat deltax = xend - xstart;
- const GLfloat deltay = yend - ystart;
- const GLfloat angle = SDL_atan2f(deltay, deltax);
- verts[3 + 0] += SDL_cosf(angle);
- verts[3 + 1] += SDL_sinf(angle);
- }
+ /* bump the end of each line segment out a quarter of a pixel, to provoke
+ the diamond-exit rule. Without this, you won't just drop the last
+ pixel of the last line segment, but you might also drop pixels at the
+ edge of any given line segment along the way too. */
+ for (i = 1; i < count; i++) {
+ const GLfloat xstart = prevx;
+ const GLfloat ystart = prevy;
+ const GLfloat xend = points[i].x + 0.5f; /* 0.5f to hit pixel center. */
+ const GLfloat yend = points[i].y + 0.5f;
+ /* bump a little in the direction we are moving in. */
+ const GLfloat deltax = xend - xstart;
+ const GLfloat deltay = yend - ystart;
+ const GLfloat angle = SDL_atan2f(deltay, deltax);
+ prevx = xend + (SDL_cosf(angle) * 0.25f);
+ prevy = yend + (SDL_sinf(angle) * 0.25f);
+ *(verts++) = prevx;
+ *(verts++) = prevy;
+ *((int *)verts++) = color;
}
return 0;