Assorted SDL improvements #4927

Clownacy · 2022-01-22T14:10:06Z

No description provided.

This is (kind of) an OpenGL-only function, which should be avoided when SDL2 isn't using OpenGL. The only alternative that is recommended is SDL_GetRendererOutputSize, which limits this fix to the SDL_Renderer backend. Still, I think it's better than nothing. I say that SDL_GL_GetDrawableSize is "kind of" OpenGL-only because it does technically work even when SDL2 isn't using OpenGL. It's just that it becomes a shim to SDL_GetWindowSize, which is not suitable for high-DPI usage because it reflects the size of the window in screen coordinates, not actual pixels, so it really should be avoided when not using OpenGL.

ocornut · 2022-01-24T11:17:45Z

Hello,

Thank you for the PR.

I don't think using SDL_memset etc are required nor provide any extra value, could you undo that and squash?
For calls to SDL_Renderer functions in SDL.h you would need to check the compatible SDL version. If the function was introduced after SDL 2.0.4 it needs appropriate ifdef.
I think we need a general solution for situation where neither SDL_Renderer neither OpenGL are used.

Clownacy · 2022-01-24T11:48:34Z

Thanks for the feedback! I'm not at a PC right now to make any changes, but I can respond to a few things here:

I think the point of the SDL libc functions is to support platforms that don't have an adequate C standard library, or to avoid having the user's program become dependent on said library. An instance I saw where the libc replacement functions were recommended was for avoiding a dependency on a particular version of msvcr###.dll. Though, if Dear ImGui itself uses libc functions directly without any way of shimming them, then I guess using the SDL libc functions really is pointless.

According to its documentation, SDL_GetRendererOutputSize was introduced in SDL 2.0.0, so I don't think it needs an ifdef.

I'm not sure what can be done to add a general solution for when neither SDL_Renderer nor OpenGL are being used. I think that would have to be tackled in a separate PR by someone else.

Clownacy · 2022-01-27T12:55:33Z

The SDL libc commit has been reverted. Sorry for the delay.

SDL_PIXELFORMAT_RGBA32 is intended for byte arrays where the channels are specifically in the RGBA order. However, this is not what GetTexDataAsRGBA32 does: rather, it constructs an array where each pixel is an unsigned int in ABGR order. On little-endian platforms, this does indeed result in an RGBA byte order, however, on big-endian platforms, this results in an ABGR byte order, which does not match the PIXELFORMAT enum. What should be used is the SDL_PIXELFORMAT_ABGR8888 enum, which specifies that the pixels are in native-endian ABGR, which they are.

`SDL_WINDOW_OPENGL` is only necessary when using OpenGL directly, bypassing SDL2's Renderer API. There's a similar flag for doing the same with Vulkan - `SDL_WINDOW_VULKAN`. I imagine that this flag was used because of the SDL backend's dependency on an OpenGL-specific function, but that has been eliminated as of ccf4c24.

…re (#4927) SDL_PIXELFORMAT_RGBA32 is intended for byte arrays where the channels are specifically in the RGBA order. However, this is not what GetTexDataAsRGBA32 does: rather, it constructs an array where each pixel is an unsigned int in ABGR order. On little-endian platforms, this does indeed result in an RGBA byte order, however, on big-endian platforms, this results in an ABGR byte order, which does not match the PIXELFORMAT enum. What should be used is the SDL_PIXELFORMAT_ABGR8888 enum, which specifies that the pixels are in native-endian ABGR, which they are.

…rSDLRenderer(). Use SDL_GetRendererOutputSize() instead of SDL_GL_GetDrawableSize() when bound to a SDL_Renderer. (#4927) This is (kind of) an OpenGL-only function, which should be avoided when SDL2 isn't using OpenGL. The only alternative that is recommended is SDL_GetRendererOutputSize, which limits this fix to the SDL_Renderer backend. Still, I think it's better than nothing. I say that SDL_GL_GetDrawableSize is "kind of" OpenGL-only because it does technically work even when SDL2 isn't using OpenGL. It's just that it becomes a shim to SDL_GetWindowSize, which is not suitable for high-DPI usage because it reflects the size of the window in screen coordinates, not actual pixels, so it really should be avoided when not using OpenGL.

ocornut · 2022-02-04T15:51:59Z

Thank you !
Now merged as c39192b and c6cab1f

…re (ocornut#4927) SDL_PIXELFORMAT_RGBA32 is intended for byte arrays where the channels are specifically in the RGBA order. However, this is not what GetTexDataAsRGBA32 does: rather, it constructs an array where each pixel is an unsigned int in ABGR order. On little-endian platforms, this does indeed result in an RGBA byte order, however, on big-endian platforms, this results in an ABGR byte order, which does not match the PIXELFORMAT enum. What should be used is the SDL_PIXELFORMAT_ABGR8888 enum, which specifies that the pixels are in native-endian ABGR, which they are.

…rSDLRenderer(). Use SDL_GetRendererOutputSize() instead of SDL_GL_GetDrawableSize() when bound to a SDL_Renderer. (ocornut#4927) This is (kind of) an OpenGL-only function, which should be avoided when SDL2 isn't using OpenGL. The only alternative that is recommended is SDL_GetRendererOutputSize, which limits this fix to the SDL_Renderer backend. Still, I think it's better than nothing. I say that SDL_GL_GetDrawableSize is "kind of" OpenGL-only because it does technically work even when SDL2 isn't using OpenGL. It's just that it becomes a shim to SDL_GetWindowSize, which is not suitable for high-DPI usage because it reflects the size of the window in screen coordinates, not actual pixels, so it really should be avoided when not using OpenGL.

… + Backends: SDL_Renderer: Explicitely call SDL_SetTextureScaleMode(). (#4927)

Clownacy mentioned this pull request Jan 22, 2022

Remove SDL_WINDOW_OPENGL from SDL_Renderer example #4923

Closed

Clownacy changed the title ~~Avoiding using SDL_GL_GetDrawableSize~~ Assorted SDL improvements Jan 22, 2022

ocornut added the backends label Jan 24, 2022

Remove stray parameter in the sdlrenderer example

eb3fedc

Clownacy force-pushed the sdl-renderer-output-size branch from 9aef59d to eb3fedc Compare January 27, 2022 12:54

Clownacy added 2 commits January 30, 2022 22:42

ocornut closed this Feb 4, 2022

ocornut added a commit that referenced this pull request Apr 7, 2022

Added comments about requirement for bilinear filtering. (#5156, #3245)…

f7f3047

… + Backends: SDL_Renderer: Explicitely call SDL_SetTextureScaleMode(). (#4927)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Assorted SDL improvements #4927

Assorted SDL improvements #4927

Clownacy commented Jan 22, 2022 •

edited

Loading

ocornut commented Jan 24, 2022

Clownacy commented Jan 24, 2022 •

edited

Loading

Clownacy commented Jan 27, 2022

ocornut commented Feb 4, 2022

Assorted SDL improvements #4927

Assorted SDL improvements #4927

Conversation

Clownacy commented Jan 22, 2022 • edited Loading

ocornut commented Jan 24, 2022

Clownacy commented Jan 24, 2022 • edited Loading

Clownacy commented Jan 27, 2022

ocornut commented Feb 4, 2022

Clownacy commented Jan 22, 2022 •

edited

Loading

Clownacy commented Jan 24, 2022 •

edited

Loading