| Summary: | SDL_RenderCopy/CopyEx in software should optionally render 8bit alpha | ||
|---|---|---|---|
| Product: | SDL | Reporter: | Ellie <etc0de> |
| Component: | video | Assignee: | Sam Lantinga <slouken> |
| Status: | RESOLVED FIXED | QA Contact: | Sam Lantinga <slouken> |
| Severity: | normal | ||
| Priority: | P2 | CC: | adam |
| Version: | HG 2.0 | ||
| Hardware: | All | ||
| OS: | All | ||
| Attachments: |
proposed patch (v1)
proposed patch (v2) |
||
|
Description
Ellie
2012-07-20 13:26:50 UTC
When thinking more about this, I realized my suggested fix would lead to objects rotating with constant speed to display with 8bit alpha for a tiny moment and otherwise with 1bit alpha, which probably looks a bit stupid and inconsistent. Can we have a switch to turn 8bit alpha on for SDL_RenderCopyEx in software, no matter whether it is passed a 0 degree angle or not? Then application developers can fix this on their own in the most consistent, yet performance-conserving way. (e.g. switching it off consistently for objects which are known to rotate, even if they might rest with 0 degree at some point, and switching it on for objects which are known to not rotate ever but which need to be flipped at some point - that is something which only the application can judge since that requires deeper knowledge about the logic of the rendered sprites) May I bump this issue again? It would greatly improve render consistency if it was possible to have 8bit alpha with the software SDL_RenderCopyEx and rotation. The 8bit alpha at angle 0 can be done by using SDL_RenderCopy for that case (which is what I do now), but for all other angles there would need to be actual support in SDL! Sure, this sounds good. Do you have a patch in mind? Sorry I don't :) I'm not even sure if SDL_gfx supported this (where the new software rotation code originates from I suppose?). I just thought I'd bring it up again since it kinda ruins the software mode for a small test game I did - but then again I don't rely much on a working software mode, it's just nice to have. Maybe post on the mailing list and request one? I don't even think there should be a switch for 1-bit alpha. If the surface has an 8-bit alpha channel and is set to use alpha blending, the rendering should be done with 8-bit alpha, period. It's simply wrong to render a surface with an 8-bit alpha channel using only one bit of alpha. If the developer wants 1-bit alpha, let him convert the surface to a format with a 1-bit alpha channel, or use a color key. I do agree that calling SDL_RenderCopyEx with zero rotation should use the faster no-rotation algorithm, if it doesn't. *** Bug 2546 has been marked as a duplicate of this bug. *** This also happens with SDL_RenderCopy (which does not support flipping or rotation), as described in bug #2546. I'm not sure if they have the exact same cause, but since #2546 has been marked as a duplicate of this one, I hope both problems are fixed here. There are three problems in the code that I see. 1. SW_RenderCopyEx enables a color key on surface_scaled even if the source surface didn't have a color key. 2. SW_RenderCopyEx doesn't copy blend mode, color mod, or alpha mod from src to surface_scaled. 3. When SDL_BlitScaled(src, srcrect, surface_scaled, &tmp_rect) is called, it blends the src pixels into surface_scaled instead of overwriting them (if src has blending, etc. enabled). Created attachment 2191 [details]
proposed patch (v1)
I've attached a patch that 1) fixes the three problems that I mentioned, 2) adds the requested performance improvement of using the regular blit function if no rotation or flipping is needed, 3) avoids cloning the source surface if no stretching is required, and 4) fixes an apparent off-by-one error in the 32-bit rotation code, and 5) simplifies the rotation code slightly.
I think this is a complete fix for all the issues in this bug. Please test it.
Created attachment 2194 [details]
proposed patch (v2)
I reverted the off-by-one "fix" because it was erroneous.
Fixed, thanks! https://hg.libsdl.org/SDL/rev/5c4a85c5b648 There actually /is/ an off-by-one error in the smooth rotation code, by the way. But I can see that it's deliberate (perhaps due to laziness or concern about performance). I'll probably submit another patch to fix that tomorrow. (That's bug #3029.) |