From ed10d9473f62131890166c3b6bcf71db5e53b444 Mon Sep 17 00:00:00 2001 From: "Ryan C. Gordon" Date: Mon, 10 Feb 2020 12:53:54 -0500 Subject: [PATCH] opengl: Build out full GL_LINES and respect the diamond-exit rule. Likewise for the GLES1 and GLES2 renderers. This solves the missing pixel at the end of a line and removes all the heuristics for various platforms/drivers. It's possible we could still use GL_LINE_STRIP with this and save some vertex buffer space, assuming this doesn't upset some driver somewhere, but this seems to be a clean fix that makes the GL renderers match the software renderer output. Diamond-exit rule explanation: http://graphics-software-engineer.blogspot.com/2012/04/rasterization-rules.html Fixes Bugzilla #3182. --- src/render/opengl/SDL_render_gl.c | 113 +++++++++++++----------- src/render/opengles/SDL_render_gles.c | 66 ++++++++++++-- src/render/opengles2/SDL_render_gles2.c | 67 +++++++++++--- 3 files changed, 175 insertions(+), 71 deletions(-) diff --git a/src/render/opengl/SDL_render_gl.c b/src/render/opengl/SDL_render_gl.c index 133a20b0caf51..41b26dfd57397 100644 --- a/src/render/opengl/SDL_render_gl.c +++ b/src/render/opengl/SDL_render_gl.c @@ -865,6 +865,60 @@ GL_QueueDrawPoints(SDL_Renderer * renderer, SDL_RenderCommand *cmd, const SDL_FP return 0; } +static int +GL_QueueDrawLines(SDL_Renderer * renderer, SDL_RenderCommand *cmd, const SDL_FPoint * points, int count) +{ + GLfloat *verts; + int i; + + SDL_assert(count >= 2); /* should have been checked at the higher level. */ + + verts = (GLfloat *) SDL_AllocateRenderVertices(renderer, (count-1) * 4 * sizeof (GLfloat), 0, &cmd->data.draw.first); + if (!verts) { + return -1; + } + + cmd->data.draw.count = count; + + /* GL_LINE_STRIP seems to be unreliable on various drivers, so try + to build out our own GL_LINES. :( + If the line segment is completely horizontal or vertical, + make it one pixel longer, to satisfy the diamond-exit rule. + We should probably do this for diagonal lines too, but we'd have to + do some trigonometry to figure out the correct pixel and generally + when we have problems with pixel perfection, it's for straight lines + that are missing a pixel that frames something and not arbitrary + angles. Maybe !!! FIXME for later, though. */ + + for (i = 0; i < count-1; i++, points++) { + GLfloat xstart = 0.5f + points[0].x; /* 0.5f to get to the center of the pixel. */ + GLfloat ystart = 0.5f + points[0].y; + GLfloat xend = 0.5f + points[1].x; + GLfloat yend = 0.5f + points[1].y; + + if (xstart == xend) { /* vertical line */ + if (yend > ystart) { + yend += 1.0f; + } else { + ystart += 1.0f; + } + } else if (ystart == yend) { /* horizontal line */ + if (xend > xstart) { + xend += 1.0f; + } else { + xstart += 1.0f; + } + } + + *(verts++) = xstart; + *(verts++) = ystart; + *(verts++) = xend; + *(verts++) = yend; + } + + return 0; +} + static int GL_QueueFillRects(SDL_Renderer * renderer, SDL_RenderCommand *cmd, const SDL_FRect * rects, int count) { @@ -1228,59 +1282,14 @@ GL_RunCommandQueue(SDL_Renderer * renderer, SDL_RenderCommand *cmd, void *vertic case SDL_RENDERCMD_DRAW_LINES: { const GLfloat *verts = (GLfloat *) (((Uint8 *) vertices) + cmd->data.draw.first); const size_t count = cmd->data.draw.count; + SDL_assert(count >= 2); SetDrawState(data, cmd, SHADER_SOLID); - if (count > 2 && (verts[0] == verts[(count-1)*2]) && (verts[1] == verts[(count*2)-1])) { - data->glBegin(GL_LINE_LOOP); - /* GL_LINE_LOOP takes care of the final segment */ - for (i = 1; i < count; ++i, verts += 2) { - data->glVertex2f(verts[0], verts[1]); - } - data->glEnd(); - } else { - #if defined(__MACOSX__) || defined(__WIN32__) - #else - int x1, y1, x2, y2; - #endif - - data->glBegin(GL_LINE_STRIP); - for (i = 0; i < count; ++i, verts += 2) { - data->glVertex2f(verts[0], verts[1]); - } - data->glEnd(); - verts -= 2 * count; - - /* The line is half open, so we need one more point to complete it. - * http://www.opengl.org/documentation/specs/version1.1/glspec1.1/node47.html - * If we have to, we can use vertical line and horizontal line textures - * for vertical and horizontal lines, and then create custom textures - * for diagonal lines and software render those. It's terrible, but at - * least it would be pixel perfect. - */ - - data->glBegin(GL_POINTS); - #if defined(__MACOSX__) || defined(__WIN32__) - /* Mac OS X and Windows seem to always leave the last point open */ - data->glVertex2f(verts[(count-1)*2], verts[(count*2)-1]); - #else - /* Linux seems to leave the right-most or bottom-most point open */ - x1 = verts[0]; - y1 = verts[1]; - x2 = verts[(count-1)*2]; - y2 = verts[(count*2)-1]; - - if (x1 > x2) { - data->glVertex2f(x1, y1); - } else if (x2 > x1) { - data->glVertex2f(x2, y2); - } - if (y1 > y2) { - data->glVertex2f(x1, y1); - } else if (y2 > y1) { - data->glVertex2f(x2, y2); - } - #endif - data->glEnd(); + data->glBegin(GL_LINES); + for (i = 0; i < count-1; ++i, verts += 4) { + data->glVertex2f(verts[0], verts[1]); + data->glVertex2f(verts[2], verts[3]); } + data->glEnd(); break; } @@ -1619,7 +1628,7 @@ GL_CreateRenderer(SDL_Window * window, Uint32 flags) renderer->QueueSetViewport = GL_QueueSetViewport; renderer->QueueSetDrawColor = GL_QueueSetViewport; /* SetViewport and SetDrawColor are (currently) no-ops. */ renderer->QueueDrawPoints = GL_QueueDrawPoints; - renderer->QueueDrawLines = GL_QueueDrawPoints; /* lines and points queue vertices the same way. */ + renderer->QueueDrawLines = GL_QueueDrawLines; renderer->QueueFillRects = GL_QueueFillRects; renderer->QueueCopy = GL_QueueCopy; renderer->QueueCopyEx = GL_QueueCopyEx; diff --git a/src/render/opengles/SDL_render_gles.c b/src/render/opengles/SDL_render_gles.c index 7f980926813fc..2f3261418c61f 100644 --- a/src/render/opengles/SDL_render_gles.c +++ b/src/render/opengles/SDL_render_gles.c @@ -560,6 +560,60 @@ GLES_QueueDrawPoints(SDL_Renderer * renderer, SDL_RenderCommand *cmd, const SDL_ return 0; } +static int +GLES_QueueDrawLines(SDL_Renderer * renderer, SDL_RenderCommand *cmd, const SDL_FPoint * points, int count) +{ + GLfloat *verts; + int i; + + SDL_assert(count >= 2); /* should have been checked at the higher level. */ + + verts = (GLfloat *) SDL_AllocateRenderVertices(renderer, (count-1) * 4 * sizeof (GLfloat), 0, &cmd->data.draw.first); + if (!verts) { + return -1; + } + + cmd->data.draw.count = count; + + /* GL_LINE_STRIP seems to be unreliable on various drivers, so try + to build out our own GL_LINES. :( + If the line segment is completely horizontal or vertical, + make it one pixel longer, to satisfy the diamond-exit rule. + We should probably do this for diagonal lines too, but we'd have to + do some trigonometry to figure out the correct pixel and generally + when we have problems with pixel perfection, it's for straight lines + that are missing a pixel that frames something and not arbitrary + angles. Maybe !!! FIXME for later, though. */ + + for (i = 0; i < count-1; i++, points++) { + GLfloat xstart = 0.5f + points[0].x; /* 0.5f to get to the center of the pixel. */ + GLfloat ystart = 0.5f + points[0].y; + GLfloat xend = 0.5f + points[1].x; + GLfloat yend = 0.5f + points[1].y; + + if (xstart == xend) { /* vertical line */ + if (yend > ystart) { + yend += 1.0f; + } else { + ystart += 1.0f; + } + } else if (ystart == yend) { /* horizontal line */ + if (xend > xstart) { + xend += 1.0f; + } else { + xstart += 1.0f; + } + } + + *(verts++) = xstart; + *(verts++) = ystart; + *(verts++) = xend; + *(verts++) = yend; + } + + return 0; +} + static int GLES_QueueFillRects(SDL_Renderer * renderer, SDL_RenderCommand *cmd, const SDL_FRect * rects, int count) { @@ -900,16 +954,10 @@ GLES_RunCommandQueue(SDL_Renderer * renderer, SDL_RenderCommand *cmd, void *vert case SDL_RENDERCMD_DRAW_LINES: { const GLfloat *verts = (GLfloat *) (((Uint8 *) vertices) + cmd->data.draw.first); const size_t count = cmd->data.draw.count; + SDL_assert(count >= 2); SetDrawState(data, cmd); data->glVertexPointer(2, GL_FLOAT, 0, verts); - if (count > 2 && (verts[0] == verts[(count-1)*2]) && (verts[1] == verts[(count*2)-1])) { - /* GL_LINE_LOOP takes care of the final segment */ - data->glDrawArrays(GL_LINE_LOOP, 0, (GLsizei) (count - 1)); - } else { - data->glDrawArrays(GL_LINE_STRIP, 0, (GLsizei) count); - /* We need to close the endpoint of the line */ - data->glDrawArrays(GL_POINTS, (GLsizei) (count - 1), 1); - } + data->glDrawArrays(GL_LINES, 0, (GLsizei) ((count-1) * 2)); break; } @@ -1158,7 +1206,7 @@ GLES_CreateRenderer(SDL_Window * window, Uint32 flags) renderer->QueueSetViewport = GLES_QueueSetViewport; renderer->QueueSetDrawColor = GLES_QueueSetViewport; /* SetViewport and SetDrawColor are (currently) no-ops. */ renderer->QueueDrawPoints = GLES_QueueDrawPoints; - renderer->QueueDrawLines = GLES_QueueDrawPoints; /* lines and points queue vertices the same way. */ + renderer->QueueDrawLines = GLES_QueueDrawLines; renderer->QueueFillRects = GLES_QueueFillRects; renderer->QueueCopy = GLES_QueueCopy; renderer->QueueCopyEx = GLES_QueueCopyEx; diff --git a/src/render/opengles2/SDL_render_gles2.c b/src/render/opengles2/SDL_render_gles2.c index fb83c875d50ee..0a432e04af607 100644 --- a/src/render/opengles2/SDL_render_gles2.c +++ b/src/render/opengles2/SDL_render_gles2.c @@ -783,6 +783,60 @@ GLES2_QueueDrawPoints(SDL_Renderer * renderer, SDL_RenderCommand *cmd, const SDL return 0; } +static int +GLES2_QueueDrawLines(SDL_Renderer * renderer, SDL_RenderCommand *cmd, const SDL_FPoint * points, int count) +{ + GLfloat *verts; + int i; + + SDL_assert(count >= 2); /* should have been checked at the higher level. */ + + verts = (GLfloat *) SDL_AllocateRenderVertices(renderer, (count-1) * 4 * sizeof (GLfloat), 0, &cmd->data.draw.first); + if (!verts) { + return -1; + } + + cmd->data.draw.count = count; + + /* GL_LINE_STRIP seems to be unreliable on various drivers, so try + to build out our own GL_LINES. :( + If the line segment is completely horizontal or vertical, + make it one pixel longer, to satisfy the diamond-exit rule. + We should probably do this for diagonal lines too, but we'd have to + do some trigonometry to figure out the correct pixel and generally + when we have problems with pixel perfection, it's for straight lines + that are missing a pixel that frames something and not arbitrary + angles. Maybe !!! FIXME for later, though. */ + + for (i = 0; i < count-1; i++, points++) { + GLfloat xstart = 0.5f + points[0].x; /* 0.5f to get to the center of the pixel. */ + GLfloat ystart = 0.5f + points[0].y; + GLfloat xend = 0.5f + points[1].x; + GLfloat yend = 0.5f + points[1].y; + + if (xstart == xend) { /* vertical line */ + if (yend > ystart) { + yend += 1.0f; + } else { + ystart += 1.0f; + } + } else if (ystart == yend) { /* horizontal line */ + if (xend > xstart) { + xend += 1.0f; + } else { + xstart += 1.0f; + } + } + + *(verts++) = xstart; + *(verts++) = ystart; + *(verts++) = xend; + *(verts++) = yend; + } + + return 0; +} + static int GLES2_QueueFillRects(SDL_Renderer * renderer, SDL_RenderCommand *cmd, const SDL_FRect * rects, int count) { @@ -1294,17 +1348,10 @@ GLES2_RunCommandQueue(SDL_Renderer * renderer, SDL_RenderCommand *cmd, void *ver } case SDL_RENDERCMD_DRAW_LINES: { - const GLfloat *verts = (GLfloat *) (((Uint8 *) vertices) + cmd->data.draw.first); const size_t count = cmd->data.draw.count; + SDL_assert(count >= 2); if (SetDrawState(data, cmd, GLES2_IMAGESOURCE_SOLID) == 0) { - if (count > 2 && (verts[0] == verts[(count-1)*2]) && (verts[1] == verts[(count*2)-1])) { - /* GL_LINE_LOOP takes care of the final segment */ - data->glDrawArrays(GL_LINE_LOOP, 0, (GLsizei) (count - 1)); - } else { - data->glDrawArrays(GL_LINE_STRIP, 0, (GLsizei) count); - /* We need to close the endpoint of the line */ - data->glDrawArrays(GL_POINTS, (GLsizei) (count - 1), 1); - } + data->glDrawArrays(GL_LINES, 0, (GLsizei) ((count-1) * 2)); } break; } @@ -2099,7 +2146,7 @@ GLES2_CreateRenderer(SDL_Window *window, Uint32 flags) renderer->QueueSetViewport = GLES2_QueueSetViewport; renderer->QueueSetDrawColor = GLES2_QueueSetViewport; /* SetViewport and SetDrawColor are (currently) no-ops. */ renderer->QueueDrawPoints = GLES2_QueueDrawPoints; - renderer->QueueDrawLines = GLES2_QueueDrawPoints; /* lines and points queue vertices the same way. */ + renderer->QueueDrawLines = GLES2_QueueDrawLines; renderer->QueueFillRects = GLES2_QueueFillRects; renderer->QueueCopy = GLES2_QueueCopy; renderer->QueueCopyEx = GLES2_QueueCopyEx;