Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
GWRon opened this issue Oct 29, 2024 · 0 comments

Comments

@GWRon
Copy link
Contributor

GWRon commented Oct 29, 2024

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant