Commit ca9a32171573a89679d0d96e14f85524387d187a
2021-09-19T15:47:24
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...
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 185 186 187 188 189 190 191 192 193 194 195 196 197 198 199 200 201 202 203 204 205 206 207 208 209 210 211 212
diff --git a/src/render/opengl/SDL_render_gl.c b/src/render/opengl/SDL_render_gl.c
index 758166a..d58ef1d 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 c0a2ce9..4fa256f 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 7ea703d..11e89c6 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;