libsdl-org / SDL

Simple Directmedia Layer
https://libsdl.org
zlib License
9.07k stars 1.71k forks source link

SDL_RenderTexture fails to render if source rect has zero size #8580

Open wareya opened 9 months ago

wareya commented 9 months ago

This is caused by this rect intersection check:

https://github.com/libsdl-org/SDL/blob/4722269fb62315640cbdbd8916cfc937ee3603d4/src/render/SDL_render.c#L3214-L3218

And this early-out in the intersection checking code:

https://github.com/libsdl-org/SDL/blob/4722269fb62315640cbdbd8916cfc937ee3603d4/src/video/SDL_rect_impl.h#L82-L86

Drawing texture rects with zero source size is a generally well-defined operation in virtually all "real" graphics APIs, even really old ones (it essentially picks a single pixel (or line of pixels) from the texture, possibly filtered, for the entire rect) and should not silently fail.

I can see the rationale behind clipping the source rect, but it feels like bailing out early when it's zero-size is an oversight brought on by the clipping operation.

icculus commented 9 months ago

If you ask it to draw 0x0 pixels, then that's what it does. This doesn't seem like a bug to me.

wareya commented 9 months ago

I'm not asking it to draw 0x0 pixels. I'm asking it to draw many pixels sourced from a 0x0 region on the texture. These are very different things.

wareya commented 9 months ago

For example, imagine I'm drawing a fixed-size 64x64 rectangle somewhere on the screen, sourced from this region of this image:

image

This should work, and it does.

Now imagine that I gradually reduce the size of the region over many frames until it's zero, keeping the position the same. The texture rect will appear to zoom in, until the source rect reaches zero size, at which point SDL_RenderTexture will stop working. If you were to instead do this with raw UV coordinates and SDL_RenderGeometry, it would work fine without any problems, looking exactly the same until the source rect becomes zero-size, at which it would continue to work instead of disappearing.

Zero-size texture sampling source regions are mathematically well-defined and rendering APIs almost always support them.

slouken commented 9 months ago

If you just remove the check for the source rectangle, does it do what you want?

wareya commented 9 months ago

With this modification to SDL_render.c:

    if (srcrect) {
        real_srcrect = *srcrect;
        /*
        if (!SDL_GetRectIntersectionFloat(srcrect, &real_srcrect, &real_srcrect)) {
            return 0;
        }
        */
    }

And this modification to testrendercopyex.c to function as a test:

    s->sprite_rect.x = (float)((viewport.w - s->sprite_rect.w) / 2);
    s->sprite_rect.y = (float)((viewport.h - s->sprite_rect.h) / 2);

/* new code */
    s->sprite_rect.x = 128;
    s->sprite_rect.y = 128;
    s->sprite_rect.w = 256;
    s->sprite_rect.h = 256;

    /* 0x0 source rect */
    /* SDL_FRect src = { 16.0f, 16.0f, 0.0f, 0.0f }; */
    /* 16x0 source rect */
    SDL_FRect src = { 0.0f, 18.0f, 32.0f, 0.0f };

    SDL_RenderTexture(s->renderer, s->sprite, &src, &s->sprite_rect);

/* end code */
    /* SDL_RenderTextureRotated(s->renderer, s->sprite, &src, &s->sprite_rect, 0, center, (SDL_RendererFlip)s->scale_direction); */

I get this screenshot, which is what I expect, because I'm defining a 32-width, zero-height source rect:

2023-11-19_14-19-00

(The black outer edges are from the smiley face's outline, and the inner black stripe is from its nose.)

But this removes the call to SDL_GetRectIntersectionFloat entirely, which is a very big change in behavior. I'm not sure why it's designed to clip the source rect to the texture, but I'm sure there's a good reason and that there are projects that depend on it. Maybe, since zero-sized source rects don't do anything at all right now, the following would be better? (Still gives what I expect.)

    if (srcrect) {
        if (!SDL_GetRectIntersectionFloat(srcrect, &real_srcrect, &real_srcrect)) {
            real_srcrect = *srcrect;
        }
    }

Or maybe a version of (or change to) SDL_GetRectIntersectionFloat that supports zero-sized rects would be better. Not sure.