bmx-ng / sdl.mod

SDL backend for BlitzMax
7 stars 6 forks source link

[SDLRender.mod] SetClipRect / SetViewport incorrectly reset parameters on x or y < 0 #62

Open GWRon opened 2 weeks ago

GWRon commented 2 weeks ago

Heya, while fiddling with resizeable windows and SDL.mod I stumbled over an issue when using a viewport with negative X or Y values.

sdlrender.mod's glue.c contains this:

int bmx_SDL_RenderReadPixels(SDL_Renderer * renderer, Uint32 format, void * pixels, int pitch, int x, int y, int w, int h) {
    if (x < 0 || y < 0 || w < 0 || h < 0) {
        return SDL_RenderReadPixels(renderer, 0, format, pixels, pitch);
    } else {
        SDL_Rect r = { x, y, w, h };
        return SDL_RenderReadPixels(renderer, &r, format, pixels, pitch);
    }   
}

int bmx_SDL_RenderSetClipRect(SDL_Renderer * renderer, int x, int y, int w, int h) {
    if (x < 0 || y < 0 || w < 0 || h < 0) {
        return SDL_RenderSetClipRect(renderer, 0);
    } else {
        SDL_Rect r = { x, y, w, h };
        return SDL_RenderSetClipRect(renderer, &r);
    }
}

When comparing this with SDL's sources: https://github.com/libsdl-org/SDL/blob/5b0e838a7433daf0d44aff10574791cced63113e/src/render/SDL_render.c#L2527

int SDL_RenderSetClipRect(SDL_Renderer *renderer, const SDL_Rect *rect)
{
    int retval;
    CHECK_RENDERER_MAGIC(renderer, -1)

    if (rect && rect->w >= 0 && rect->h >= 0) {
        renderer->clipping_enabled = SDL_TRUE;
        renderer->clip_rect.x = (double)rect->x * renderer->scale.x;
        renderer->clip_rect.y = (double)rect->y * renderer->scale.y;
        renderer->clip_rect.w = (double)rect->w * renderer->scale.x;
        renderer->clip_rect.h = (double)rect->h * renderer->scale.y;
    } else {
        renderer->clipping_enabled = SDL_FALSE;
        SDL_zero(renderer->clip_rect);
    }

    retval = QueueCmdSetClipRect(renderer);
    return retval < 0 ? retval : FlushRenderCommandsIfNotBatching(renderer);
}

You see that this "reset rect" behaviour is already "in there" and only happening as soon as width or height of the cliprect (similar for viewport) are below zero.

Other code areas (like the bmx_SDL_RenderReadPixels correctly check for "x<0 or y<0 ..." as counterpart of the "if(rect)" checks I guess. But for viewport (bmx_SDL_RenderSetViewport) and cliprect (bmx_SDL_RenderSetClipRect) I think it should be allowed to have negative x,y coordinates - it is just the "dimensions" which need to be positive.

So the current implementation will reset a viewport/cliprect as soon as x or y become negative:

        renderer.SetClipRect(0, -10, 300, 300)
        Local gx:int, gy:int, gw:int, gh:int
        renderer.GetClipRect(gx, gy, gw, gh)
        print g.viewport_x+", "+g.viewport_y+", "+g.viewport_w+", "+g.viewport_h+" -> " + gx+", "+gy+", "+gw+", "+gh

output:

0, -10, 300, 300 -> 0, 0, 0, 0