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

[Feature request] Generate texture mipmap #4156

Open
rom1v opened this issue Mar 7, 2021 · 20 comments · May be fixed by #10254
Open

[Feature request] Generate texture mipmap #4156

rom1v opened this issue Mar 7, 2021 · 20 comments · May be fixed by #10254
Assignees
Milestone

Comments

@rom1v
Copy link

rom1v commented Mar 7, 2021

I would like a feature to enable mipmap generation on a given SDL_Texture.

For now, if the renderer is opengl >= 3 or opengl es >= 3, then I use:

SDL_Texture *texture = ...;
SDL_GL_BindTexture(texture, NULL, NULL); 
glGenerateMipmap(GL_TEXTURE_2D);

It works very well. However, I have no solution for other renderers (direct3d, metal, etc.).

Do you think such a feature could be added to SDL directly?

Or at least, is it possible to generate mipmaps by calling the native APIs (with some ifdefs) for the other renderers, similar to glGenerateMipmap() for OpenGL? (but I want to keep using the SDL API like SDL_Texture for the remaining code)

Ref: Genymobile/scrcpy#40 (comment)

@bibendovsky
Copy link

IMO The 3D mipmapping has no sense in SDL 2D renderer.

@rom1v
Copy link
Author

rom1v commented Mar 16, 2021

It makes sense if you downscale (especially by a factor of more than 2). See Genymobile/scrcpy#40 (comment).

@cjxgm
Copy link

cjxgm commented Mar 17, 2021

@bibendovsky
Texture prefiltering is a useful technique to reduce aliasing when textures are scaled down. (Linear filtering has its effect only when scaling up).
Mipmapping and anisotropic filtering are just two hardware provided mechanisms of texture prefiltering for uniform (keeping aspect-ratio) and non-uniform (not keeping aspect-ratio) downscaling, respectively.

@bibendovsky
Copy link

@cjxgm Thanks for explanation.
Just never used such technique for 2D.

@bibendovsky
Copy link

@rom1v According to this commit you not only need to generate mipmaps but also to modify texture properties - minification filter and LOD bias.

@rom1v
Copy link
Author

rom1v commented Mar 17, 2021

@bibendovsky Hmm, yes, that's true.

@JianYuhaha
Copy link

I have a problem.
When I run the scrcpy, it reminds me that Trilinear filtering disabled (not an OpenGL renderer).
I don't know whether it's a mobile phone problem or a computer problem.

@dingogames
Copy link

I saw that SDL now includes SDL_RenderGeometry - this makes SDL a lot more useful for making many types of 2d games. However without mipmapping the use cases are much more limited - you can't really scale down graphics because they turn into an aliased mess. So that's something to consider - mipmapping goes hand in hand with the new SDL_RenderGeometry functionality.

@slouken slouken added this to the 3.2.0 milestone Nov 7, 2023
@slouken
Copy link
Collaborator

slouken commented Nov 7, 2023

@icculus, does this make sense for SDL3?

@flibitijibibo
Copy link
Collaborator

Running through SDL_Render issues tagged for 3.0 ABI, sorry if you get multiple e-mails for this...

FWIW the GPU API at #9312 includes support for generating mipmaps and is supported on all backends, so it could also be exposed to SDL_Render if needed.

@icculus
Copy link
Collaborator

icculus commented Jul 12, 2024

I don't think we're going to add this to the 2D API.

@icculus icculus closed this as not planned Won't fix, can't repro, duplicate, stale Jul 12, 2024
@danpla
Copy link

danpla commented Jul 12, 2024

@icculus Why not?

@icculus
Copy link
Collaborator

icculus commented Jul 12, 2024

Because it's a 2D API.

For special cases that can benefit from it, like scrcpy using it to do downscaling on the hardware, they can force a specific renderer and/or write some custom code to interact with the SDL backend.

@danpla
Copy link

danpla commented Jul 12, 2024

As far as I understand, the special case is actually not using mipmapping, e.g. when you have a guarantee that images will not be scaled down. Because if they do, billinear filtering will only help with small downscaling factors, and larger factors will still give you aliasing.

The aliasing problem is not specific to "real" 3D rendering, so it seems odd that SDL has bilinear filtering while mipmapping is considered out of scope, as if it were something unrelated.

@dingogames
Copy link

I also have to argue the case for this. I’ve used quite a few different 2D APIs over the past 20 years or so, and they’ve pretty much all supported mipmapping. It's just such a broadly useful feature! The idea that mipmapping is mostly a 3D feature doesn’t square with my experience.

One super common use case is any game with the idea of “zooming”... and that’s a LOT of games. Maybe MOST games? If you size your assets exactly right for the current display resolution and only do a small amount of zooming, you might be fine. But if your assets are sized for, say, 4K and you’re running at 1080p, ANY amount of zooming will look horrible. Think of how many 2D games have a camera that can zoom out:

  • Zooming out for a more tactical view/map
  • Zooming out to frame the scene/level differently
  • Split-screen views
  • Stretchy co-op camera zoom to show both players
  • Slight camera zoom animation common in many games

Really, there are endless reasons to zoom out.

Even if you just have a simple application with a graphical UI. If you don't size the UI art assets correctly you're going to get aliasing artifacts when you run at a low resolution.

You can fix SOME of these issues by optimizing and managing assets for different screen resolutions. However, that can be a lot of work and it doesn’t even solve all the issues.

This isn’t a niche use case. If you’re making a production-ready 2D game that supports different resolutions, you want mipmapping.

@slouken slouken reopened this Jul 13, 2024
icculus added a commit to icculus/SDL that referenced this issue Jul 13, 2024
This is only implemented for the OpenGL and OpenGL ES 2 backends currently,
and is completely untested, beyond it compiles on my laptop.

Fixes libsdl-org#4156.
@icculus
Copy link
Collaborator

icculus commented Jul 13, 2024

Don't have the strength to win fights this week, so what the heck, here's a first shot at this in #10254.

I don't know all the ramifications and corner cases, we should probably have something in the docs saying "don't use this unless you know what you're doing, as it could have surprising performance results for no difference in quality if you just slap it around your code."

Metal will need the same code that texture updating requires, where it ends any current command encoder and switching to a Blit encoder. The higher level flushes the SDL renderer command queue if the texture is used in it, like it does for render target switching, etc, so the state is correct between previous draws and future ones if the texture is going to change at this point. It's smart enough to not flush the queue if the texture isn't pending use, though, same as other actions.

Vulkan and later D3D revisions offer something for this...D3D9 (I recall maybe incorrectly) only offered this in the D3DX utility library that we don't use, and there it was probably just manually uploading mips from the CPU. We could roll that ourselves, but I'm inclined to leave it unsupported on D3D9.

@icculus
Copy link
Collaborator

icculus commented Jul 13, 2024

Anyhow, please do try this out, it's completely untested and I want feedback on whether this is what you want/need. It should work on the OpenGL and OpenGLES2 backends in that PR.

icculus added a commit to icculus/SDL that referenced this issue Jul 16, 2024
This is only implemented for the OpenGL and OpenGL ES 2 backends currently,
and is completely untested, beyond it compiles on my laptop.

Fixes libsdl-org#4156.
@slouken
Copy link
Collaborator

slouken commented Jul 21, 2024

Has anyone tested this? Does anyone have an SDL rendering test case that shows how this improves things?

@icculus
Copy link
Collaborator

icculus commented Jul 21, 2024

The PR needs work still.

@icculus
Copy link
Collaborator

icculus commented Jul 29, 2024

In the interest of time, I'm bumping this out of the 3.0 ABI milestone.

The idea is the PR needs to change to specify mipmapping as a texture creation property, as opposed to a function that generates mipmaps on demand...which is to say we don't need to change the API to add it later.

(EDIT: if we decide to do this.)

@icculus icculus modified the milestones: 3.0 ABI, 3.2.0 Jul 29, 2024
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

Successfully merging a pull request may close this issue.

9 participants