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

Minor API conveniences I want to add #117

Closed
icefoxen opened this issue Apr 29, 2019 · 4 comments · Fixed by gfx-rs/gfx#2771
Closed

Minor API conveniences I want to add #117

icefoxen opened this issue Apr 29, 2019 · 4 comments · Fixed by gfx-rs/gfx#2771

Comments

@icefoxen
Copy link
Contributor

icefoxen commented Apr 29, 2019

Impl Default for SamplerInfo and the types it contains. ViewKind as well? MipLevel? Look at TextureBuilder::new() for preferred values.

Refactor sprite example to have a separate method for creating textures?

Can Mesh and Image be Clone?

Things that need docs: resource::Kind,

@zakarumych
Copy link
Member

resource::Kind is gfx_hal::image::Kind reexported.
Shouldn't be that way.

@icefoxen
Copy link
Contributor Author

SamplerInfo and ViewKind are also part of gfx-hal. Will submit enhancements to them.

Mesh can't be Clone because it contains a Buffer and IndexBuffer, which need destruction via free_buffers() or whatever. Might be convenient to add a method that creates a copy of a mesh anyway, even if it has to take a queue and factory to do it. ...that said you can't really modify an existing mesh, so why would you want an exact copy of it instead of just reusing it? Maybe to make resource management easier. Same goes for Image; it can't be Clone 'cause it contains GPU-managed data, but being able to make a copy or near-copy of an existing image might be handy.

Actually, Mesh contains Escape<Buffer<B>> but just IndexBuffer<B>. Should the index buffer be wrapped in an Escape too? Looks like it, or its underlying Buffer, could be.

bors bot added a commit to gfx-rs/gfx that referenced this issue May 15, 2019
2771: Expand docs and add some Default impl's for image r=kvark a=icefoxen

Fixes amethyst/rendy#117
PR checklist:
- [x] `make` succeeds (on *nix)
- [x] `make reftests` succeeds
- [x] tested examples with the following backends: vulkan
- [x] `rustfmt` run on changed code


Co-authored-by: Simon Heath <icefox@dreamquest.io>
@zakarumych
Copy link
Member

IndexBuffer is just Escape<Buffer<B>> + IndexType.

I can't see use case for copying meshes and images, but it can be easy implemented as stand-alone function. Although stand-alone function suffers from bad discoverability.

@icefoxen
Copy link
Contributor Author

Derp, I could have sworn that I saw IndexBuffer containing just a Buffer<B> with no Escape.
Okay, I think that's all. Thank you!

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.

2 participants