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

Rethink mipmap generation #906

Closed
kvark opened this issue Mar 18, 2016 · 6 comments
Closed

Rethink mipmap generation #906

kvark opened this issue Mar 18, 2016 · 6 comments

Comments

@kvark
Copy link
Member

kvark commented Mar 18, 2016

Our previous approach was to have generate_mipmap method on the Factory. It doesn't fit DX11 model, since even though there is GenerateMipmap method, it is a rendering operation, belongs to ID3D11DeviceContext, thus can't be executed by a factory, which only has ID3D11Device.

It seems straightforward to just move the mipmap generation into the CommandBuffer interface, since this is where our rendering is done. However, this doesn't work well for the major use-case - when we have the mip[0] contents right away and we want to upload it with all the mipmaps generated. Not only this is inconvenient, it also involves blowing up the data buffer, associated with the command buffer on GL and non-deferred DX11. This means - time wasted on the copy and allocation.
Edit: problem is, that we can't create a partially initialized texture in DX11/DX12, that's why we'd have to provide mip[0] contents through the command buffer.

As if it's not difficult enough already, DX12 doesn't have any mechanism to assist in mipmap generation (see #905). You are supposed to do it manually. We can, of course, implement some rendering inside the backend, but this rises the question - should we support mipmap generation at all?.. Once you start being serious about graphics, you end up loading all mipmaps from disk in some optimized GPU-friendly format anyway, since generating them takes time.

@emberian
Copy link
Contributor

My understanding also is that the drivers are not always good about high-quality mipmap generation.

I guess the image crate should just handle the downscaling, and we should only support uploading fully specified textures?

@kvark
Copy link
Member Author

kvark commented Mar 18, 2016

Problem with doing this on the image side is that it's going to be much slower, obviously. However, I do like this approach as a middle ground - for users that already care about smooth mipmap transitions but do not care enough to actually pack their textures into a proper format with mipmaps included. So I take it your (@cmr) voice is for removing the support for generating mipmaps, and I like it :)

If we think about how to still provide some automatic generation for the backends that can do it, maybe we can have command buffer (or even factory?) extensions like:

trait MipmapCommandBuffer: CommandBuffer {
 fn generate_mipmap(...);
}

Then some backends will implement it, and the user code can put a clause like where C: gfx::MipmapCommandBuffer where they want to use it.

This approach can be extended for other features that we don't strongly support in the core, while still wanting to provide.

@emberian
Copy link
Contributor

Could that extension be implemented with some sort of compatability fallback for DX12? For example, it could expand into commands that use compute shaders to generate the mipmaps, which should be much much faster than image.

@emberian
Copy link
Contributor

I guess that line of thinking leads to generalized middleware architectures for gfx?

@kvark
Copy link
Member Author

kvark commented Mar 18, 2016

@cmr it totally can be implemented in the backend itself, even if it doesn't support mipmap generation natively. However, that bring us to the problem I expressed initially - that texture uploading would have to go through the command/data buffer, because we can't create a texture with partially initialized contents, on DX11 at least. (That was probably unclear, I updated the issue description)

@sectopod
Copy link
Contributor

Since we are waiting for winapi-rs anyway, I've added generate_mipmap() to our CommandBuffer interface. We'll probably change it later when new backends are introduced, but for GL+DX11 is fairly straightforward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants