With Dear ImGui + software renderer, it draws:
- by default at 250 fps
- drops to 70 fps if you show the color picker
- drops to 10 fps if put the color picker fullscreen
This reintroduces the fix from 0e16ee8330, but just marks
the viewport state as dirty after a clear that needs to expand the
viewport to fill the render target, as we'll need to also reset
the orthographic projection state elsewhere, and that won't
happen if we clear the dirty flag here.
Fixes#4210.
(again.)
(...sorry...!)
The Renderer logical scaling code scales mouse coordinates, and needs to
take the window DPI into account on HIGHDPI windows. However, the
variable which tracks this, renderer->dpi_scale, is set once when the
renderer is created, and then not updated. In the event that the window
is moved to another screen, or the screen DPI otherwise changes, this
will be outdates, and potentially the coordinates will be all wrong.
So let's update the dpi_scale on the SIZE_CHANGED event: it's at least a
possibility that this will be issued on some OSes when DPI changes, and
it's otherwise already handled by SDL_Renderer's event filter.
Otherwise you might have set the viewport to the full size of
the render target in SDL's API but this change hasn't been
transmitted to Direct3D yet by the time we attempt to clear.
Fixes#4210.
The RenderDrawLinesWithRects and RenderDrawLinesWithRectsF functions can
sometimes call QueueCmdFillRects() with the data pointed to by frects
uninitialised. This can occur if none of the lines can be replaced with
rects, in which case the frects array is empty, and nrects is 0.
gcc 10.3.0 will detect this possibility, and print a warning like:
/home/david/Development/SDL/src/render/SDL_render.c: In function 'RenderDrawLinesWithRectsF':
/home/david/Development/SDL/src/render/SDL_render.c:2725:15: warning: '<unknown>' may be used uninitialized [-Wmaybe-uninitialized]
2725 | retval += QueueCmdFillRects(renderer, frects, nrects);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/david/Development/SDL/src/render/SDL_render.c:499:1: note: by argument 2 of type 'const SDL_FRect *' to 'QueueCmdFillRects' declared here
499 | QueueCmdFillRects(SDL_Renderer *renderer, const SDL_FRect * rects, const int count)
| ^~~~~~~~~~~~~~~~~
This is harmless, because when this is uninitialised, nrects is always
0, so QueueCmdFillRects() does nothing anyway. We therefore can work
around this by only calling QueueCmdFillRects() when nrects is nonzero.
Somewhat impressively, gcc recognises that this is now safe.
This is needed to support CHERI, and thus Arm's experimental Morello
prototype, where pointers are implemented using unforgeable capabilities
that include bounds and permissions metadata to provide fine-grained
spatial and referential memory safety, as well as revocation by sweeping
memory to provide heap temporal memory safety.
On most systems (anything with a flat memory hierarchy rather than using
segment-based addressing), size_t and uintptr_t are the same type.
However, on CHERI, size_t is just an integer offset, whereas uintptr_t
is still a capability as described above. Casting a pointer to size_t
will strip the metadata and validity tag, and casting from size_t to a
pointer will result in a null-derived capability whose validity tag is
not set, and thus cannot be dereferenced without faulting.
The audio and cursor casts were harmless as they intend to stuff an
integer into a pointer, but using uintptr_t is the idiomatic way to do
that and silences our compiler warnings (which our build tool makes
fatal by default as they often indicate real problems). The iconv and
egl casts were true positives as SDL_iconv_t and iconv_t are pointer
types, as is NativeDisplayType on most OSes, so this would have trapped
at run time when using the round-tripped pointers. The gles2 casts were
also harmless; the OpenGL API defines this argument to be a pointer type
(and uses the argument name "pointer"), but it in fact represents an
integer offset, so like audio and cursor the additional idiomatic cast
is needed to silence the warning.
Fixes SDL_CreateWindowAndRenderer (and similar situations) not choosing a Metal backend. See #3991.
Passing an explicit backend into CreateWindow, eg SDL_WINDOW_OPENGL or SDL_WINDOW_METAL, will still prevent the window from being used with other backend types.
This is caused by the Metal renderer recreating the window because by default we create an OpenGL window on macOS.
It turns out that at least on macOS 10.15, a window that has been initialized for OpenGL can also be used with Metal. So we'll skip recreating the window in that case.
UMU
#define SDL_COMPOSE_ERROR(str) SDL_STRINGIFY_ARG(__FUNCTION__) ", " str
I think SDL_STRINGIFY_ARG should be removed.
#define SDL_COMPOSE_ERROR(str) __FUNCTION__ ", " str
(verified with Visual Studio 2019)
C.W. Betts
I tested building commit http://hg.libsdl.org/SDL/rev/7adf3fdc19f3 on Mac Catalyst and found some issues:
* MTLFeatureSet_iOS_* enums aren't available under Mac Catalyst.
* OpenGL ES is unavailable under Mac Catalyst.
* Some Metal features are available under Catalyst but not iOS, such as displaySyncEnabled.
* Set Metal as the default renderer on Mac Catalyst
Attaching a patch that will make SDL2 build for Mac Catalyst.
Vincent Hamm
Xcode11 and ios13 added support for metal simulator.
Here is a quick and dirty patch to enable it. Pretty early and only tested on a few samples for now. Required mostly to enable metal support on correct version of ios, generate simulator compatible shaders and enforce buffer alignments on simulator (same as osx).
OpenGL leaves the final line segment open, SDL's software renderer does not,
so we need a tiny bit of trigonometry here to move one more pixel in the right
direction.
We now handle HiDPI correctly, and touches are clamped to the viewport. So
if you are rendering to a logical 640x480 in a 720p window, and touch the
letterboxing at point (640,700), it will report the touch at (0.5,1.0) instead
of outside the documented range.
This fixes a case where you render to the backbuffer, then render to a render
target, set the current target back to the backbuffer, and then present
without drawing anything else; in this circumstance, the Present command
would never happen.
Fixes Bugzilla #5011.
From hmk:
"When scaling is enabled (e.g. via SDL_RenderSetLogicalSize, size not equal
to window size), mouse motion events are also scaled. Small motions are
rounded up (SDL_max() when the value after scaling is less than 1), while
larger motions are truncated by the floating point -> integer conversion.
https://hg.libsdl.org/SDL/file/b18197f9bf9d/src/render/SDL_render.c#l658
The end result feels something like mouse reverse mouse acceleration + angle
snapping at low speeds, but less consistent (amount of truncation & rounding
depends on how fast the mouse is moved) and potentially much worse if the
scaling factor is large. This pretty much makes it useless for anything
where you need precise mouse aiming (think of games). I suspect this is why
aiming gets so terrible in some games that let you use scaling to reduce the
render resolution (e.g. Ion Fury).
With 4x4 scaling, I can reproduce a situation where it takes three fast flicks
of the mouse across the pad to undo one slow sweep across the pad. In other
words, extreme reverse acceleration. This does not happen when scaling is
disabled.
Furthermore, any game that uses relative mouse motion events for 3D camera
rotation probably wants the raw mouse deltas and not a value that depends on
scaling and resolution and rounding and truncation. Ideal camera rotation
just takes mouse input, multiplies it by sensitivity, and adds it to the
angle-in-radians or whatever measure is used for yaw & pitch. Pixels and
screen resolution or window dimensions should not be a part of the equation
at all, even if it could be implemented without rounding errors.
[...]
This [patch] completely eliminates angle snapping for me, and makes
sensitivity consistent. In other words, it's completely usable for, say,
aiming in a first person shooter."
Partially fixes Bugzilla #4811.
Konrad
It appears that I cannot use SDL_RenderReadPixels on a bound framebuffer (SDL_Texture set as render target) as it simply results in gibberish data. However, drawing that framebuffer into the default target (window surface) does render it correctly. Other backends (OpenGL, software, Direct3D) do work fine.
It looks to me like D3D11_RenderReadPixels just gets the general backbuffer and not the current render target and its backbuffer.
Here is the patch which actually fetches the current render target and its underlying ID3D11Resource which is ID3D11Texture2D.
Otherwise our cached state goes out of sync when updating a texture. Since
these state changes aren't necessary, they were removed instead of updating
the cached state.
Fixes Bugzilla #4998.
cmediaplayer
Hi, i already mentioned in the SDL discourse a bug that recreating of a texture occours pixel shader problem on direct3d renderer. There is no problem for direct3d11. You can see the issue by using my app named C Media Player which is available for Windows for free using my web site www.cmediaplayer.com. Just follow the steps:
*Open a media file
*When playing the file change the scale quality under the video menu.
*You will see the problem.
This is the OpenGL line drawing fix for Bugzilla #3182, but there's some
disagreement about what the renderers should do here, so I'm backing this out
until after 2.0.12 ships, and then we'll reevaluate all the renderer backends
to decide what's correct, and make them all work the same.
Likewise for the GLES1 and GLES2 renderers.
This solves the missing pixel at the end of a line and removes all the
heuristics for various platforms/drivers. It's possible we could still use
GL_LINE_STRIP with this and save some vertex buffer space, assuming this
doesn't upset some driver somewhere, but this seems to be a clean fix that
makes the GL renderers match the software renderer output.
Diamond-exit rule explanation:
http://graphics-software-engineer.blogspot.com/2012/04/rasterization-rules.html
Fixes Bugzilla #3182.
Konrad
This kind of blending is rather quite useful and in my opinion should be available for all renderers. I do need it myself, but since I didn't want to use a custom blending mode which is supported only by certain renderers (e.g. not in software which is quite important for me) I did write implementation of SDL_BLENDMODE_MUL for all renderers altogether.
SDL_BLENDMODE_MUL implements following equation:
dstRGB = (srcRGB * dstRGB) + (dstRGB * (1-srcA))
dstA = (srcA * dstA) + (dstA * (1-srcA))
Background:
https://i.imgur.com/UsYhydP.png
Blended texture:
https://i.imgur.com/0juXQcV.png
Result for SDL_BLENDMODE_MOD:
https://i.imgur.com/wgNSgUl.png
Result for SDL_BLENDMODE_MUL:
https://i.imgur.com/Veokzim.png
I think I did cover all possibilities within included patch, but I didn't write any tests for SDL_BLENDMODE_MUL, so it would be lovely if someone could do it.
Konrad
This was something rather trivial to add, but asked at least several times before (I did google about it as well).
It should be possible to dynamically change scaling mode of the texture. It is actually trivial task, but until now it was only possible with a hint before creating a texture.
I needed it for my game as well, so I took the liberty of writing it myself.
This patch adds following functions:
SDL_SetTextureScaleMode(SDL_Texture * texture, SDL_ScaleMode scaleMode);
SDL_GetTextureScaleMode(SDL_Texture * texture, SDL_ScaleMode *scaleMode);
That way you can change texture scaling on the fly.
warning: either cast from 'int' to 'size_t' (aka 'unsigned long') is ineffective, or there is loss of precision before the conversion [bugprone-misplaced-widening-cast]
Sylvain
I think what happening with the software renderer is:
* you're somehow in background (so texture creation is not possible)
* it resizes and wants to push a SDL_WINDOWEVENT_SIZE_CHANGED
It call:
https://hg.libsdl.org/SDL/file/a010811d40dd/src/render/SDL_render.c#l683
* GetOutputSize
* SW_GetOutputSize
* SW_ActivateRenderer
* SDL_GetWindowSurface
* SDL_CreateWindowFramebuffer which is mapped to SDL_CreateWindowTexture
and it ends up re-creating the surface/a texture, while being in background
Sylvain
Currently SDL_CreateTextureFromSurface picks first valid format, and do a conversion.
format = renderer->info.texture_formats[0];
for (i = 0; i < renderer->info.num_texture_formats; ++i) {
if (!SDL_ISPIXELFORMAT_FOURCC(renderer->info.texture_formats[i]) &&
SDL_ISPIXELFORMAT_ALPHA(renderer->info.texture_formats[i]) == needAlpha) {
format = renderer->info.texture_formats[i];
break;
It could try to find a better format, for instance :
if SDL_Surface has no Amask, but a colorkey :
if surface fmt is RGB888, try to pick ARGB8888 renderer fmt
if surface fmt is BGR888, try to pick ABGR8888 renderer fmt
else
try to pick the same renderer format as surface fmt
if no format has been picked, use the fallback.
I think it goes with bug 4290 fastpath BlitNtoN
when you expand a surface with pixel format of size 24 to 32, there is a fast path possible.
So with this issue:
- if you have a surface with colorkey (RGB or BGR, not palette), it takes a renderer format where the conversion is faster.
(it avoids, if possible, RGB -> ABGR which means switching RGB to BGR)
- if you have a surface ABGR format, it try to take the ABGR from the renderer.
(it avoids, if possible, ABGR -> ARGB, which means switch RGB to BGR)
Sylvain
OpenGLES2 SDL renderer has support for textures ARGB, ABGR, RGB and BGR, whereas OpenGL SDL renderer only had ARGB.
If you think it's worth adding it, here's a patch. I quickly tried and it worked, but there may be missing things or corner case.
This is the best option for macOS and iOS, the only platforms with Metal.
Pre-Metal versions of these platforms will fall back to OpenGL (ES), as
appropriate.
Huge thanks to Alexander Szpakowski, who worked incredibly hard to get the
Metal renderer to such a high-quality state!
./src/render/SDL_render.c(2168): Error! E1054: Expression must be constant
./src/render/SDL_render.c(2168): Error! E1054: Expression must be constant
./src/render/SDL_render.c(2175): Error! E1054: Expression must be constant
./src/render/SDL_render.c(2175): Error! E1054: Expression must be constant
./src/render/SDL_render.c(2322): Error! E1054: Expression must be constant
./src/render/SDL_render.c(2322): Error! E1054: Expression must be constant
./src/render/SDL_render.c(2322): Error! E1054: Expression must be constant
./src/render/SDL_render.c(2322): Error! E1054: Expression must be constant
./src/render/SDL_render.c(2329): Error! E1054: Expression must be constant
./src/render/SDL_render.c(2329): Error! E1054: Expression must be constant
./src/render/SDL_render.c(2329): Error! E1054: Expression must be constant
./src/render/SDL_render.c(2329): Error! E1054: Expression must be constant
./src/render/software/SDL_render_sw.c(602): Error! E1054: Expression must be constant
./src/render/software/SDL_render_sw.c(602): Error! E1054: Expression must be constant
./src/render/software/SDL_render_sw.c(602): Error! E1054: Expression must be constant
./src/render/software/SDL_render_sw.c(602): Error! E1054: Expression must be constant
https://bugzilla.libsdl.org/show_bug.cgi?id=4350
We can't safely call GL_DestroyRenderer() until GL_LoadFunctions()
succeeds because we will be missing functions that we try to use
when activating the renderer for destruction if we have an GL context.
Sylvain
Re-opening this issue.
It fixes the test-case, but it introduces a regression with another bug (bug #4313).
So here's a new patch that activate cropping of the source surface to solve the issue.
It also reverts the wrong changeset.
It prevents unneeded colorkey error message.
Include guards in most changed files were missing, I added them keeping
the same style as other SDL files. In some cases I moved the include
guards around to be the first thing the header has to take advantage of
any possible improvements compiler may have for inclusion guards.
I have no idea if this works (or if it ever worked, having now examined this
code), as I have no way to compile or test this.
If it's broken, send patches. :)
This means it doesn't have to block while the current frame finishes using the
vertex buffer; it just moves on to the next, probably-not-in-use buffer.
- high-level filters out duplicate render commands from the queue so
backends don't have to.
- Setting draw color is now a render command, so backends can put color
information into the vertex buffer to upload with everything else instead
of setting it with slower dynamic data later.
- backends can request that they always batch, even for legacy programs,
since the lowlevel API can deal with it (Metal, and eventually Vulkan
and such...)
- high-level makes sure the queue has at least one setdrawcolor and
setviewport command before any draw calls, so the backends don't ever have
to manage cases where this hasn't been explicitly set yet.
- backends allocating vertex buffer space can specify alignment, and the
high-level will keep track of gaps in the buffer between the last used
positions and the aligned data that can be used for later allocations
(Metal and such need to specify some constant data on 256 byte boundaries,
but we don't want to waste all that space we had to skip to meet alignment
requirements).
It now uses a growable linked list that keeps a pool of allocated items for
reuse, and reallocs the vertex array as necessary. Testsprite2 can scale to
20,000 (or more!) draws now without drama.
This moves all the rendering to a command list that is flushed to the GL as
necessary, making most common activities upload a single vertex buffer per
frame and dramatically reducing state changes. In pathological cases,
like Emscripten running on iOS's Safari, performance can go from a dozen
draw calls killing your performance to 1000 draw calls running smoothly.
This is work in progress, and not ready to ship. Among other things, it has
a hardcoded array that isn't checked for overflow. But the basic idea is
sound!
The only place angle is activated and causes effect is RenderCopyEx. All other
methods which use vertex shader, leave angle disabled and cause useless sin/cos
calculation in shader.
To get around shader's interface is changed to a vector that contains results
of sin and cos. To behave properly when disabled, cos value is set with offset
-1.0 making 0.0 default when deactivated.
As nice side effect it simplifies GLES2_UpdateVertexBuffer: All attributes are
vectors now.
Additional background:
* On RaspberryPi it gives a performace win for operations. Tested with
[1] numbers go down for 5-10% (not easy to estimate due to huge variation).
* SDL_RenderCopyEx was tested with [2]
* It works around left rotated display caused by low accuracy sin implemetation
in RaspberryPi/VC4 [3]
[1] https://github.com/schnitzeltony/sdl2box
[2] https://github.com/schnitzeltony/sdl2rendercopyex
[3] https://github.com/anholt/mesa/issues/110
Signed-off-by: Andreas M?ller <schnitzeltony@gmail.com>
duckgrease
SDL_RenderCopyEx blits wrong image (in some cases it's bunch of alternating horizontal lines, some cases it's image from the wrong coordinate, and in some cases it's just a bunch of garbled pixels), when the following conditions are met:
- Use software renderer.
- Enable either horizontal or vertical flip.
- source and destination rectangles must have same width and height, and must be smaller than the size of the texture.
- source rectangle's X and Y coordinates must be 0.
Sylvain
Patch a few warnings when using:
-Wmissing-prototypes -Wdocumentation -Wdocumentation-unknown-command
They are automatically enabled with -Wall
Anthony @ POW Games
SDL_CreateTextureFromSurface makes an internal call to SDL_GetColorKey which can return an error and spams the error log with "Surface doesn't have a colorkey" even though the original function didn't return an error.
Olli-Samuli Lehmus
If one creates a window with the SDL_WINDOW_FULLSCREEN_DESKTOP flag, and creates a render target with SDL_SetHint(SDL_HINT_RENDER_SCALE_QUALITY, "linear"), and afterwards sets SDL_SetHint(SDL_HINT_RENDER_SCALE_QUALITY, "nearest"), after minimizing the window, the scale quality hint is lost on the render target. Textures however do keep their interpolation modes.
SDL now builds with gcc 7.2 with the following command line options:
-Wall -pedantic-errors -Wno-deprecated-declarations -Wno-overlength-strings --std=c99
- Use a single buffer for various non-changing constants accessed by the GPU, instead of multiple buffers.
- Do the half-pixel offset for points and lines using a transform matrix so we don't need a malloc when rendering.
- Don't add a half-pixel offset for other primitives and textures. This matches D3D and GL render behaviour.
- Remove the half-texel texture coordinate offset since it's not needed now that there's no more half-pixel position offset when rendering a texture.
- Don't try to set texture usage on iOS 8 since it doesn't exist there.
Eric wing
There is a tiny bug in the new overscan code for the SDL_renderer.
In SDL_renderer.c, line 1265, the if check for SDL_strcasecmp with "direct3d" needs to be inverted.
Instead of:
if(SDL_strcasecmp("direct3d", SDL_GetCurrentVideoDriver())) {
It should be:
if(0 == SDL_strcasecmp("direct3d", SDL_GetCurrentVideoDriver())) {
This bug causes the "overscan" mode to pretty much be completely ignored in all cases and all things remain letterboxed (as before the feature).
Yuri K. Schlesner
When using texture filtering, there are filtering artifacts visible on the edges of scaled textures, where the texture filtering pulls in texels from the other side of the texture. Using clamping texture modes wouldn't completely fix this since source rectangles don't need to cover the whole texture. (See screenshot attached in next post.)
The opengl driver uses clamping on textures and so avoid this at least in the cases where the source rect is the whole texture. The direct3d driver does not and so has problems in every case. I'm not sure if it can actually completely be fixed, but at least enabling clamping for direct3d would be one step in the right direction.
This works better for games where there may be a bunch of simulation logic that needs to be run before the next rendering pass, and prevents blocking if the next drawable is busy.
This isn't complete, but is enough to run testsprite2. It's currently
Mac-only; with a little work to figure out how to properly glue in a Metal
layer to a UIView, this will likely work on iOS, too.
This is only wired up to the configure script right now, and disabled by
default. CMake and Xcode still need their bits filled in as appropriate.
New functions get and set the YUV colorspace conversion mode:
SDL_SetYUVConversionMode()
SDL_GetYUVConversionMode()
SDL_GetYUVConversionModeForResolution()
SDL_ConvertPixels() converts between all supported RGB and YUV formats, with SSE acceleration for converting from planar YUV formats (YV12, NV12, etc) to common RGB/RGBA formats.
Added a new test program, testyuv, to verify correctness and speed of YUV conversion functionality.
Carlos
Angle supports GLES3 but when using these functions (SDL_CreateWindow and SDL_CreateRenderer), defaults again to GLES2.0.
A current workaround (hack) to retrieve a GLES3.0 context with Angle is:
1) set
SDL_GL_SetAttribute(SDL_GL_CONTEXT_MAJOR_VERSION, 3);
SDL_GL_SetAttribute(SDL_GL_CONTEXT_MINOR_VERSION, 0);
after InitSDL AND after calling SDL_CreateWindow (before SDL_CreateRenderer)
2) Comment lines 2032-2044 in SDL_render_gles2.c, funtion GLES2_CreateRenderer
window_flags = SDL_GetWindowFlags(window);
if (!(window_flags & SDL_WINDOW_OPENGL) ||
profile_mask != SDL_GL_CONTEXT_PROFILE_ES || major != RENDERER_CONTEXT_MAJOR || minor != RENDERER_CONTEXT_MINOR) {
changed_window = SDL_TRUE;
SDL_GL_SetAttribute(SDL_GL_CONTEXT_PROFILE_MASK, SDL_GL_CONTEXT_PROFILE_ES);
SDL_GL_SetAttribute(SDL_GL_CONTEXT_MAJOR_VERSION, RENDERER_CONTEXT_MAJOR);
SDL_GL_SetAttribute(SDL_GL_CONTEXT_MINOR_VERSION, RENDERER_CONTEXT_MINOR);
if (SDL_RecreateWindow(window, window_flags | SDL_WINDOW_OPENGL) < 0) {
goto error;
}
}
This retrives a GLES3 context as confirmed using glGetString(GL_VERSION). This should be fixed by modifying a few if's.
Sylvain
Few issues with YUV on SDL2 when using odd dimensions, and missing conversions from/back to YUV formats.
1) The big part is that SDL_ConvertPixels() does not convert to/from YUV in most cases. This now works with any format and also with odd dimensions,
by adding two internal functions SDL_ConvertPixels_YUV_to_ARGB8888 and SDL_ConvertPixels_ARGB8888_to_YUV (could it be XRGB888 ?).
The target format is hard coded to ARGB888 (which is the default in the internal of the software renderer).
In case of different YUV conversion, it will do an intermediate conversion to a ARGB8888 buffer.
SDL_ConvertPixels_YUV_to_ARGB8888 is somehow redundant with all the "Color*Dither*Mod*".
But it allows some completeness of SDL_ConvertPixels to handle all YUV format.
It also works with odd dimensions.
Moreover, I did some benchmark(SDL_ConvertPixel vs Color32DitherYV12Mod1X and Color32DitherYUY2Mod1X).
gcc-6.3 and clang-4.0. gcc performs better than clang. And, with gcc, SDL_ConvertPixels() performs better (20%) than the two C function Color32Dither*().
For instance, to convert 10 times a 3888x2592 image, it takes ~195 ms with SDL_ConvertPixels and ~235 ms with Color32Dither*().
Especially because of gcc vectorize feature that optimises all conversion loops (-ftree-loop-vectorize).
Nb: I put no image pitch for the YUV buffers. because it complexify a little bit the code and the API :
There would be some ambiguity when setting the pitch exactly to image width:
would it a be pitch of image width (for luma and chroma). or just contiguous data ? (could set pitch=0 for the later).
2) Small issues with odd dimensions:
If width "w" is odd, luma plane width is still "w" whereas chroma planes will be "(w + 1)/2". Almost the same for odd h.
Solution is to strategically substitute "w" by "(w+1)/2" at the good places ...
- In the repository, SDL_ConvertPixels() handles YUV only if yuv source format is exactly the same as YUV destination format.
It basically does a memcpy of pixels, but it's done incorrectly when width or height is odd (wrong size of chroma planes). This is fixed.
- SDL Renderers don't support odd width/height for YUV textures.
This is fixed for software, opengl, opengles2. (opengles 1 does not support it and fallback to software rendering).
This is *not* fixed for D3D and D3D11 ... (and others, psp ?)
Only *two* Dither function are fixed ... not sure if others are really used.
- This is not possible to create a NV12/NV12 texture with the software renderer, whereas other renderers allow it.
This is fixed, by using SDL_ConvertPixels underneath.
- It was not possible to SDL_UpdateTexture() of format NV12/NV21 with the software renderer. this is fixed.
Here's also two testcases:
- that do all combination of conversion.
- to test partial UpdateTexture
Anthony
This is what's making the software renderer crash with rotated destination rectangles of w or h = 0:
SDL_SetHint(SDL_HINT_RENDER_SCALE_QUALITY, "2");
Martin Gerhardy
just for easier debugging issues in the own code...
SDL_CreateRenderer should maybe also use this macro
Ryan C. Gordon
I'll go one better: it should have an SDL_assert().
UX-admin
I am compiling with the Sun Studio 12 u2 compiler. There are multiple issues with the build, but this particular issue appears to be that it is illegal to declare a union of a struct of floats and a float. While GCC 4.8.1 does not flag this as an error, Sun Studio is much more standards compliant and strict, halting further compilation with an error.
Simon Hug
The bug is in the GL_ResetState and GLES_ResetState functions which get called after a new GL context is created. These functions set the cached current color to transparent black, but the GL specification says the initial color is opaque white.
The attached patch changes the values to 0xffffffff to reflect the initial state of the current color. Should the ResetState functions get called anywhere else in the future, this probably has to call the GL functions itself to ensure that the colors match.
Littlefighter19
When trying to mirror something on the PSP, I've stumbled upon the problem,
that using SDL_RenderCopyEx with SDL_FLIP_HORIZONTAL flips the image vertically, vise-versa SDL_FLIP_VERTICAL flips the image horizontally.
Proposed patch would be swapping the check in line 944 with the one in line 948 in SDL_render_psp.c
Daniel
SDL_RenderReadPixels with SDL_RENDERER_SOFTWARE reads pixels from wrong coordinates.
SW_RenderReadPixels adjusts the rect coordinates according to the viewport. But since this is already done by SDL_RenderReadPixels, the final rect has x2 bigger X and Y.
Eric wing
Hi, I think I found a bug when using SDL_WINDOW_ALLOW_HIGHDPI with SDL_RenderSetLogicalSize on iOS. I use SDL_RenderSetLogicalSize for all my stuff. I just tried turning on SDL_WINDOW_ALLOW_HIGHDPI on iOS and suddenly all my touch/mouse positions are really broken/far-off-the-mark.
I actually don't have a real retina device (still) so I'm seeing this using the iOS simulator with a 6plus template.
Attached is a simple test program that can reproduce the problem. It uses RenderSetLogicalSize and draws some moving happy faces (to show the boundaries/space of the LogicalSize and that it is working correctly for that part).
When you click/touch, it will draw one more happy face where your button point is.
If you comment out SDL_WINDOW_ALLOW_HIGHDPI, everything works as expected. But if you compile with it in, the mouse coordinates seem really far off the mark. (Face appears far up and to the left.)
Alex Szpakowski on the mailing list suggests the problem is
"I believe this is a bug in SDL_Render?s platform-agnostic mouse coordinate scaling code. It assumes the units of the mouse coordinates are always in pixels, which isn?t the case where high-DPI is involved (regardless of whether iOS is used) ? they?re actually in ?DPI independent? coordinates (which matches the window size, but not the renderer output size)."
Additionally, if this is correct, the Mac under Retina is also probably affected too and "as well as any other platform SDL adds high-dpi support for in the future".
felix
The functions in src/render/SDL_yuv_mmx.c contain the following inline assembly snippet:
/* tap dance to workaround the inability to use %%ebx at will... */
/* move one thing to the stack... */
"pushl $0\n" /* save a slot on the stack. */
"pushl %%ebx\n" /* save %%ebx. */
"movl %0, %%ebx\n" /* put the thing in ebx. */
"movl %%ebx,4(%%esp)\n" /* put the thing in the stack slot. */
"popl %%ebx\n" /* get back %%ebx (the PIC register). */
Here's how it ended up in a binary on my old laptop:
0xb5c17dbd <ColorRGBDitherYV12MMX1X+93>: push $0x0
0xb5c17dbf <ColorRGBDitherYV12MMX1X+95>: push %ebx
0xb5c17dc0 <ColorRGBDitherYV12MMX1X+96>: mov 0xc(%esp),%ebx
0xb5c17dc4 <ColorRGBDitherYV12MMX1X+100>: mov %ebx,0x4(%esp)
0xb5c17dc8 <ColorRGBDitherYV12MMX1X+104>: pop %ebx
Apparently the compiler, oblivious to the fact that the assembly snippet manipulates the %esp register, decided to refer to the operand via that same register instead of via %ebp (I believe -fomit-frame-pointer enables this). This causes %ebx to be loaded with the wrong value, which later leads to a null pointer dereference.
Recent GCC can use the %ebx register normally: <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=47602#c16>. There is even an explicit constraint "b" for allocating it.
Tom Seddon
GL_ActivateRenderer may call GL_UpdateViewport, which leaves the GL_PROJECTION matrix selected. But after GL_ResetState, the GL_MODELVIEW matrix is selected, suggesting that's the intended default state.
It seems at least like these should be consistent. Presumably GL_UpdateViewport should be doing a glMatrixMode(GL_MODELVIEW) before it finishes.
Richard Russell
Resuming from a suspended state results in a black screen. This only happens when using GLES 1.1 (GLES 2 resumes correctly) and when the render target has been changed using SDL_SetRenderTarget. This problem is new in 2.0.4.
The attached test case demonstrates the issue.
Sylvain Becker has apparently found a fix as follows:
"In the opengles leaf function (in 'src/render/opengles/SDL_render_gles.c'), it appears there is a call to 'GLES_ActivateRenderer' in 'GLES_SetRenderTarget', which is not present in opengles2. When commenting out this 'GLES_ActivateRenderer', it seems to resume fine".
This appears to fix the testcase perfectly, but I don't know whether it could have any undesirable side-effects.
felix
Here's a snippet of SDL_DestroyRenderer from hg revision 10746:7540ff5d0e0e:
SDL_Texture *texture = NULL;
SDL_Texture *nexttexture = NULL;
/* ... */
for (texture = renderer->textures; texture; texture = nexttexture) {
nexttexture = texture->next;
SDL_DestroyTexture(texture);
}
SDL_DestroyTexture removes the texture from the linked list pointed to by the renderer and ends up calling SDL_DestroyTextureInternal, which contains this:
if (texture->native) {
SDL_DestroyTexture(texture->native);
}
If it happens that texture->native is an alias of nexttexture two stack frames up, SDL_DestroyRenderer will end up trying to destroy an already freed texture. I've had this very situation happen in dosemu2.
Bug introduced in revision 10650:a8253d439914, which has a somewhat ironic description of "Fixed all known static analysis bugs"...
Simon Hug
The software renderer produces incorrect results when blending textures at an angle with certain blend modes. It seems that there were some edge cases that weren't considered when the SW_RenderCopyEx function was last changed. Or another bug possibly covered up the problem. (More on that in another bug report.)
Most of the issues come from the fact that the rotating function sets a black colorkey. This is problematic because black is most likely appearing in the surface and the final blit will ignore these pixels. Unless a colorkey is already set (the software renderer currently never sets one), it's very hard to find a free color. Of course it could scan over the whole image until one is found, but that seems inefficient.
The following blend modes have issues when drawn at an angle.
NONE: The black pixels get ignored, making them essentially transparent. This breaks the 'dstRGBA = srcRGBA' definition of the NONE blend mode.
MOD: Again, the black pixels get ignored. This also breaks the 'dstRGB = dstRGB * srcRGB' definition of the MOD blend mode, where black pixels would make the destination black as well. A white colorkey will work though, with some preparations.
BLEND: There are some issues when blending a texture with a translucent RGBA target texture. I - uh - forgot what the problem here exactly is.
This patch fixes the issues mentioned above. It mainly changes the code so it tries to do things without the colorkey and removes the automatic format conversion part from the SDLgfx_rotateSurface function. Getting the format right is something the caller has to do now and the required code has been added to the SW_RenderCopyEx function.
There's a small change to the SW_CreateTexture function. RLE encoding a surface with an alpha mask can be a lossy process. Depending on how the user uses the RGBA channels, this may be undesired. The change that surfaces with an alpha mask don't get encoded makes the software renderer consistent with the other renderers.
The SW_RenderCopyEx function now does these steps: Lock the source surface if necessary. Create a clone of the source by using the pixel buffer directly. Check the format and set a flag if a conversion is necessary. Check if scaling or cropping is necessary and set the flag for that as well. Check if color and alpha modulation has to be done before the rotate. Check if the source is an opaque surface. If not, it creates a mask surface that is necessary for the NONE blend mode. If any of the flags were set, a new surface is created and the source will be converted, scaled, cropped, and modulated. The rest of the function stays somewhat the same. The mask also needs to be rotated of course and then there is the NONE blend mode...
It's surprisingly hard to get the pixel from a rotated surface to the destination buffer without affecting the pixel outside the rotated area. I found a way to do this with three blits which is pretty hard on the performance. Perhaps someone has an idea how to do this faster?
As mentioned above, the SDLgfx_rotateSurface now only takes 8-bit paletted or 32-bit with alpha mask surfaces. It additionally sets the new surfaces up for the MOD blend mode.
I shortly tested the 8-bit path of SDLgfx_rotateSurface and it seemed to work so far. This path is not used by the software renderer anyway.
Sylvain 2016-11-07 08:49:34 UTC
when rotated +90 or -90, some transparent lines appears, though there is no Alpha or ColorKey.
if you set a dummy colorkey, it will remove the line ...
if you set a some alpha mod, the +90/-90 get transparent but not the 0/180 ...
Call SDL_GL_GetDrawableSize() directly because we may be in the initialization path and SDL_GetRendererOutputSize() will fail because the renderer magic isn't set up yet.
Daniel Gibson
Ok, I followed the simple approach of just making SDL_PIXELFORMAT_RGBA32 an alias of SDL_PIXELFORMAT_RGBA8888/SDL_PIXELFORMAT_ABGR8888, depending on endianess. And I did the same for SDL_PIXELFORMAT_ARGB32, .._BGRA, .._ABGR.
SDL_GetPixelFormatName() will of course return SDL_PIXELFORMAT_RGBA8888 (or SDL_PIXELFORMAT_ABGR8888) instead of SDL_PIXELFORMAT_RGBA32, but as long as that's mentioned in the docs it shouldn't be a problem.
D39 does not support negative viewport values which the current implementation relies on.
D3D11 does support negative viewport values so that will continue working.
Refer to Bug 2799.
Adam M.
When doing a rotated texture copy with the software renderer, where the angle is a multiple of 90 degrees, one or two edges of the image get cut off. This is because of the following line in sw_rotate.c:
if ((unsigned)dx < (unsigned)sw && (unsigned)dy < (unsigned)sh) {
which is effectively saying:
if (dx >= 0 && dx < src->w-1 && dy >= 0 && dy < src->h-1) {
As a result, it doesn't process pixels in the right column or bottom row of the source image (except when they're accessed as part of the bilinear filtering for nearby pixels). This causes it to look like the edges are cut off, and it's especially obvious with an exact multiple of 90 degrees.
Nitz
In function SDLgfx_rotateSurface:
rz_dst =
SDL_CreateRGBSurface(SDL_SWSURFACE, dstwidth, dstheight + GUARD_ROWS,
rz_src->format->Rmask, rz_src->format->Gmask,
rz_src->format->Bmask, rz_src->format->Amask);
Here rz_src get De-referenced without NULL check, which is risky.
Daniel
Seems like check of the visibility of renderer (renderer->hidden) is missing in SDL_RenderCopyEx.
In SDL_RenderCopy it should be done much earlier (after checking support for RenderCopyEx, line 1750).
Yann Dirson
When attempting to force use of opengles2 renderer with:
int wanted_renderer = -1;
for (int i = 0; i < numrenderers; i++) {
SDL_RendererInfo renderer_info;
if (SDL_GetRenderDriverInfo(i, &renderer_info) != 0) {
SDL_LogError(SDL_LOG_CATEGORY_APPLICATION, "Couldn't get renderer driver info: %s\n",
SDL_GetError());
quit(2);
}
std::cerr << "Renderer " << i << " '" << renderer_info.name << "': flags=0x"
<< std::hex << renderer_info.flags << std::dec
<< ", " << renderer_info.num_texture_formats << " texture formats, max="
<< renderer_info.max_texture_width << "x"
<< renderer_info.max_texture_height << "\n";
if (!strcmp(renderer_info.name, "opengles2")) {
std::cerr << " selecting!\n";
wanted_renderer = i;
}
}
renderer = SDL_CreateRenderer(window, wanted_renderer, 0);
... on banana pi or raspberry pi I get an error like the following (the actual
context profile value varies, being used uninitialized)
ERROR: Couldn't create renderer: Unknown OpenGL context profile 900
With this patch I get the following, which should help more pointing to a real problem:
ERROR: Couldn't create renderer: Failed getting OpenGL glGetString entry point
I pushed a patch (based on master branch of unofficial git mirror):
550389c89f
I'll be opening a different bug for the underlying issue.
ny00
Using the OpenGL ES 1.1 renderer, after updating a texture with SDL_UpdateTexture (or SDL_UnlockTexture), a following call to SDL_RenderFillRect draws a rectangle with the wrong color (which appears to be the same as the texture's top-left pixel).
Comparing SDL_render_gles.c:GLES_UpdateTexture to SDL_render_gl.c:GL_UpdateTexture, a missing call to glDisable appears to be the cause. After adding it back, the bug is resolved.
Simon Hug
The description of the SDL_RenderClear function in the SDL_render.h header says the following:
"This function clears the entire rendering target, ignoring the viewport."
The word "entire" implies that the clipping rectangle set with SDL_RenderSetClipRect also gets ignored. This is left somewhat ambiguous if only the viewport is mentioned. Minor thing, but let's see what the implementations actually do.
The software renderer ignores the clipping rectangle when clearing. It even has a comment on this: /* By definition the clear ignores the clip rect */
Most other render drivers (opengl, opengles, opengles2, direct3d, and psp [I assume. Can't test it.]) use the scissor test for the ClipRect and don't disable it when clearing. Clearing will only happen within the clipping rectangle for these drivers.
An exception is direct3d11 which uses a clear function that ignores the scissor test.
Simon Hug
When updating the viewport in GLES_UpdateViewport, the OpenGL ES renderer doesn't flip the projection matrix for target textures. The lines, rectangles and textures (if drawn with glDrawArrays) are upside down when drawing to target textures.
Simon Hug
The OpenGL ES 2 renderer does not check the target texture format when using SDL_RenderReadPixels and just always uses ABGR8888. This can result in swapped or wrong colors.
The attached patch adds a check and selects the target texture format, if a texture is set as the target.
Simon Hug
All OpenGL renderers always flip the rows of the pixels that come from glReadPixels. This is unnecessary for target textures since these are already top down.
Also, the rect->y value can be used directly for target textures for the same reason. I don't see any code that would handle the logical render size for target textures. Or am I missing something?
The attached patch makes the renderers only the flip rows if the data comes from the default framebuffer.
Simon Hug
The GL_SetBlendMode and GLES_SetBlendMode functions of the opengl and opengles renderers call the glTexEnvf to set the texture env mode to either GL_MODULATE (the default) or GL_REPLACE for the NONE blend mode. Using GL_REPLACE disables color and alpha modulation for textures.
These glTexEnv calls were put in the SetBlendMode function back in 2006 [1], but there the NONE code still used the GL_DECAL mode. The GL_REPLACE mode came in 2008 [2]. I'm a bit confused why that wasn't always GL_MODULATE and a bit surprised nobody reported that yet (unless I missed it). I guess only a few use the gles renderer and the newish shaders mask the issue.
Simon Hug
The GL_CreateTexture function doesn't have any checks for the case where the driver doesn't support the framebuffer object extension. It will call into GL_GetFBO which will call the non-existent glGenFramebuffersEXT.
Also, for some reason GL_CreateContext always sets the SDL_RENDERER_TARGETTEXTURE info flag, even if it is not supported. Changeset cc226dce7536 [1] makes this change, but doesn't explain why. It seems to me like the code would already have taken care of this [2].
The attached patch adds some checks and stops SDL from reporting render target support if there is none. The application can then properly inform the user instead of just crashing.
Simon Hug
It seems not everyone implemented glDrawTexfOES the same. Intel and Mesa ignore the viewport entirely, whereas the Raspberry Pi implementation offsets the coordinates and does viewport clipping.
The glDrawTexfOES extension text [1] for the function says "Xs and Ys are given directly in window (viewport) coordinates." I guess this wasn't clear enough.
Alex Szpakowski
Honestly I'd probably remove that codepath from SDL_Render entirely. It's an OpenGL ES 1-specific extension that isn't likely to give huge performance gains and adds additional maintenance overhead to SDL_Render while also having bugs in some drivers (as seen here).
Kai Sterker
There are already patches available from mingw64 that fix the issue
https://github.com/Alexpux/MINGW-packages/tree/master/mingw-w64-SDL2
With those applied, I could compile SDL2 without problems. But of course, it would be preferable if SDL built cleanly from source.
The Apple TV remote is currently exposed as a joystick with its touch surface treated as two axes. Key presses are also generated when its buttons and touch surface are used.
A new hint has been added to help deal with deciding whether to background the app when the remote's menu button is pressed: SDL_HINT_APPLE_TV_CONTROLLER_UI_EVENTS.
Ryan C. Gordon
Sylvain
Ozkan Sezer
Sam Lantinga
David Gow
Jessica Clarke
Ivan Epifanov
Vanfanel
uyjulian
José Cadete
Alex Szpakowski
hmk
Cameron Gutman
Cameron Gutman
Micha? Janiszewski
Andreas M?ller
sezero
Philipp Wiesemann
David Ludwig
Steffen Pankratz
Eric Wing
Ethan Lee
