grimfang4 / sdl-gpu

A library for high-performance, modern 2D graphics with SDL written in C.
MIT License
1.19k stars 123 forks source link

Shape rendering appears to be off-by-one #220

Open vddCore opened 3 years ago

vddCore commented 3 years ago

Hi, I've been using SDL_gpu as a backend library for a while now and I just noticed how specifically shape rendering appears to be off-by-one depending on the calls used. This issue appears on both a Linux-based laptop with an Intel HD Graphics 4000 integrated GPU and a Windows-based desktop computer with an AMD Radeon RX5700 XT - none of my systems use DPI scaling. The coordinate mode used is the default one, inverted, although changing the coordinate mode to mathematical does not seem to make any difference on shape rendering in general. Both viewport and camera have been left default. Changing the renderer versions does not change anything. The issue also affects rendering to a GPU_Image via GPU_Target.

The full, reproducible example source code is located at the end of this entry. Now, onto the issue...

For example the following will result in pixel at (0, 0) not being rendered at the expected location on Y-axis, later on you'll see X-axis is aligned properly.

GPU_Clear(screen);
GPU_Pixel(screen, 0, 0, c);
GPU_Flip(screen);

The window flags were set to SDL_WINDOW_BORDERLESS, although the system window decorations state appears to be irrelevant to the issue.

Now, after nudging the Y coordinate 1 pixel to the bottom, this appears: You can easily see that the call to draw a pixel at (0, 1) is exactly at the previously expected (0, 0). When drawing to the bottom-right corner it appears to be exactly 1 row above the expected one, too.

Here's what happens when drawing a rectangle outline using GPU_Rectangle: This does not need any magnification to see that there's an issue.

An interesting thing to notice is that this doesn't seem to affect shapes that are filled and drawn to top-left corner. Here's what happens when drawing a filled rectangle using GPU_RectangleFilled to (x1, y1) being at (0, 0):

And the following happens when drawing a filled rectangle using GPU_RectangleFilled to (x1, y1) being at (screen_width - 8, screen_height - 8) - rectangle dimensions here were changed to 8x8 to help include the most relevant information:

I've tried to walk through the code on my own, but I have failed to find the place at which the failure occurs. Do any of the maintainers have an idea what might be going on here?

If you have no time to fix it - completely fine - hints on where the issue might occur are more than enough. I can fix it myself and submit a PR, but I simply have no idea where to look at this point.

As promised, the full example:

#include "SDL.h"
#include "SDL_gpu.h"

const uint16_t screen_width = 320;
const uint16_t screen_height = 240;

static SDL_Color c = {
    .r = 255,
    .g = 0,
    .b = 0,
    .a = 255
};

void draw_pixel_topleft(GPU_Target* screen) {
    GPU_Pixel(screen, 0, 0, c);
}

void draw_pixel_bottomright(GPU_Target* screen) {
    GPU_Pixel(screen, screen_width - 1, screen_height - 1, c);
}

void draw_line_topleft(GPU_Target* screen) {
    GPU_Line(screen, 0, 0, 32, 0, c);
}

void draw_rect_topleft(GPU_Target* screen) {
    GPU_Rectangle(screen, 0, 0, 16, 16, c);
}

void draw_rect_filled_topleft(GPU_Target* screen) {
    GPU_RectangleFilled(screen, 0, 0, 16, 16, c);
}

void draw_rect_filled_bottomright(GPU_Target* screen) {
    GPU_RectangleFilled(screen, screen_width - 8, screen_height - 8, screen_width - 1, screen_height - 1, c);
}

int main(int argc, char** argv)
{
    SDL_Event event;
    Uint8 done;

    printRenderers();

    GPU_Target* screen = GPU_InitRenderer(
        GPU_RENDERER_OPENGL_4,
        320,
        240,
        SDL_WINDOW_BORDERLESS
    );

    if (screen == NULL)
        return -1;

    printCurrentRenderer();

    GPU_Clear(screen);
    draw_rect_filled_bottomright(screen);
    GPU_Flip(screen);

    done = 0;
    while (!done)
    {
        while (SDL_PollEvent(&event))
        {
            if (event.type == SDL_QUIT)
                done = 1;
        }
    }

    GPU_Quit();

    return 0;
}
TwentyPast4 commented 2 years ago

Very good and detailed issue report - I'm facing the same problems. It's especially noticeable when drawing very small textures into a much larger destination rectangle - the image is not offset by one pixel, but by one pixel magnified by the same scale, devided in half. I'm drawing a QR code that's a 25pix image into a 250px box, and it is being offset by 5px down and right of where it should be

TwentyPast4 commented 2 years ago

Very good and detailed issue report - I'm facing the same problems. It's especially noticeable when drawing very small textures into a much larger destination rectangle - the image is not offset by one pixel, but by one pixel magnified by the same scale, devided in half. I'm drawing a QR code that's a 25pix image into a 250px box, and it is being offset by 5px down and right of where it should be

I have found that the issue that causes this problem is having position or dimension snapping enabled on the texture. For some reason, position & dimension snapping is resolved on the source rectangle, causing large offsets on the scaled-up image position. This is probably a bug and should be fixed. Maybe something similar is also true for shapes?

avahe-kellenberger commented 2 years ago

I've found the issue, I believe.

static void Line(GPU_Renderer* renderer, GPU_Target* target, float x1, float y1, float x2, float y2, SDL_Color color)
{
  float thickness = GetLineThickness(renderer);

  float t = thickness/2;
  float line_angle = atan2f(y2 - y1, x2 - x1);
  float tc = t*cosf(line_angle); // 0.5
  float ts = t*sinf(line_angle); // 0.0

  BEGIN_UNTEXTURED("GPU_Line", GL_TRIANGLES, 4, 6);

  SET_UNTEXTURED_VERTEX(x1 + ts, y1 - tc, r, g, b, a);
  SET_UNTEXTURED_VERTEX(x1 - ts, y1 + tc, r, g, b, a);
  SET_UNTEXTURED_VERTEX(x2 + ts, y2 - tc, r, g, b, a);

  SET_INDEXED_VERTEX(1);
  SET_INDEXED_VERTEX(2);
  SET_UNTEXTURED_VERTEX(x2 - ts, y2 + tc, r, g, b, a);
}

As you may notice, t is half of the line thickness. This value is broken down into tc and ts, which are 0.5 and 0.0 respectively, when thickness == 1.

Tc is then subtracted from y1 and y2, which results in the line rendering one pixel higher.

Personally, I expected the line to simple be rendered exactly only the pixel (at least when it has a thickness of 1). I changed float t = thickness/2; to float t = thickness; and it worked as expected.


This isn't a complete fix however, but at least pinpoints the underlying issues of these functions, and why we are seeing some off-by-one errors.

@grimfang4 If you have any thoughts on this, I'd like to get this issue resolved, as it's causing some odd rendering in a project of mine. I'd be fine with fixing this myself and submitting a PR if we can come to a consensus on the matter

grimfang4 commented 2 years ago

I'm pretty sure some of the shape rendering being off-by-one is due to issues I was having reconciling OpenGL's pixel centers (0.5,0.5) for both shapes and textures that scales correctly. I'm sure there's a good fix, I just haven't found it yet and haven't had the time to dig in again.

@avahe-kellenberger For rendering lines as implemented, this seems to mean that lines should be offset by 0.5 so that the starting pixel is "covered", but floating point precision may make it miss just slightly. That's why you might see 0.5 or 0.375 or something in code or discussions about this as an offset. I might have had that adjustment in SDL_gpu but maybe took it out?

This is all what the pixel-perfect-test is supposed to highlight (if you can improve it, please do!). I think the thing to do would be to add this offset to one shape at a time and see how it renders at a variety of line angles.

grimfang4 commented 2 years ago

@avahe-kellenberger To clarify the line math, that function would draw a quad centered on the point you give it. For example, given the points (0,0) and (20,0) with a thickness of 1, the line angle is 0 so tc = 0.5 and ts = 0. The quad is then drawn using the points (0,-0.5), (0, 0.5), (20,-0.5), (20, 0.5). This is why I'd suggest that it's the floating point precision/rounding that is making it hit a row of pixels one pixel higher than it should.

avahe-kellenberger commented 2 years ago

I've done some (minimal) testing to verify, and I believe you're correct. With the line example given (rendering at (0, 0) to (32, 0)), adding an offset of 0.000125 to each value seems to just nudge it over to the right location. 0.000124 and lower would not.

That's why you might see 0.5 or 0.375 or something in code or discussions about this as an offset I'll likely go this route for now.

avahe-kellenberger commented 2 years ago

I believe GPU_RectangleFilled's x2 and y2 are exclusive @vddCore which is why you'd see the last pixel not filled in (shouldn't be using screen_width - 1, screen_height - 1 but rather screen_width, screen_height)

The underlying code uses the top-level corner of pixels for rendering, which would naturally make the x2 and y2 coords exclusive, since the end of the rendering stops at the top left of (x2, y2).

avahe-kellenberger commented 2 years ago

@grimfang4 I'd like some eyes on this PR when you have time - it's solved the issues I was having with rendering shapes.

I'd much prefer to get some form of my PR merged instead of maintaining my own fork of this repo. Thanks!