Things that I think could be improved in gfx-rs

[tomaka]

@kvark encouraged me to post a mega-issue about things that I don't like with gfx-rs, so here it is.

Documentation

• More examples.
• Document the differences between device, render and gfx and what each of these crates is supposed to do. It took me some time to figure out that only gfx is supposed to be used.
• Not only document what a trait/enum is, but also where and how it should be used. For example if an object doesn't have a constructor (like FrontEnd or DrawList), it should be documented how to create one.
• Everything that shouldn't be used by the user must be either private or marked with #[doc(hidden)]. This includes Handle and VertexFormat.
• Document a little bit what a buffer, a surface, a frame buffer, etc. are. Don't assume that all gfx-rs users will be experts in 3D rendering.
• Document whether [[f32, ..4], ..4] is supposed to be row-major or column-major.
• The Frame type is too confusing. The front end returns "the main frame", but frames can be created with Frame::new(). The docs say it's "the result of rendering", but I'd expect the pixel data to be in there if it was the case. What exactly is a frame?
• Same for Mesh.
• I have never heard the Shell word before when talking about 3D rendering. This word is confusing.

Safety

• Remove all type Foo = Bar in favor of struct Foo(Bar).
• Remove all delete_* functions. RAII should handle everything.
• Rust doesn't allow "empty" objects (eg. null pointers), so gfx-rx shouldn't allow empty textures or empty buffers either.
• Texture data format should be inferred using Rust's type system. For example if the user uploads some texture data whose type is &[(f32, f32, f32)] or &[[f32, ..3]], you can infer that it has 3 gl_float components.
• Similarly, the type of the texture (1D, 2D, 3D) can be auto-detected depending on the function parameters. If the user passes a height of 1 and a depth of 1 to create_texture, then it's a 1D texture.
• Why does DrawList have a reset method? The user should just create a new list instead of resetting it.
• Index buffer are too difficult to use and too unsafe. Just like textures, the type of indices should be inferred from the rust type used in the buffer instead of requiring the user to pass a U16 or U32 enum value.
• create_frame_buffer should not return a u32.
• Mesh should contain the BufferHandle to ensure that the buffer is not destroyed as long as the mesh exists.

Usability

• Why is connect_program not part of the draw list? More often than not, you have a matrix uniform which must be updated at each frame. The fact that it's not part of the draw list makes draw list not reusable because you need to recreate a new CustomShell object at each frame.
• ParamDirectory should be much easier to create.

[kvark]

This is an impressive list! Let me address some of its points (other points I mainly agree with):

More examples.

We have #210, which will show multi-threading and working with frames. Anything else you think requires explicit coverage?

It took me some time to figure out that only gfx is supposed to be used.

This is arguable. All examples use only gfx now, and the tutorial (in our blog) also used only this crate. There is no reason to think you need anything else, but I do agree that we could have said it more explicitly in the docs.

#[doc(hidden)]. This includes Handle and VertexFormat.

Users may find the second one useful, but I agree on the Handle.

Same for Mesh

Mesh seems to be pretty well documented. Could you revisit mesh.rs?

Remove all delete_* functions. RAII should handle everything.

I wish we had this level of convenience, but it's by far not trivial to provide this as an alternative that is only suitable for single-threaded execution.

If the user passes a height of 1 and a depth of 1 to create_texture, then it's a 1D texture.

This is possible, but there are more types. How do you infer Texture2DArray or TextureCube?

Why does DrawList have a reset method? The user should just create a new list instead of resetting it.

Because resetting the draw list avoids re-allocating the memory. Besides, we are going to have the create_draw_list on the Device, which might live in a separate thread from the one that populates the draw list.

Why is connect_program not part of the draw list?

You are only supposed to use it once when the program is linked. Today, we have a convenient link_program which does both linking and connection with parameters. Once you got the Shell (to be renamed into Program), you can either re-use it (program.data.your_matrix = each frame), or clone() it to use with different parameters.

I realize that this needs to be demonstrated and explained. We've been in flux, and my article draft on shader parameters is still a draft. I'll update it once we stabilize more.

[kvark]

Rust doesn't allow "empty" objects (eg. null pointers), so gfx-rx shouldn't allow empty textures or empty buffers either.

Fixed by #220

Index buffer are too difficult to use and too unsafe.

Fixed by #217

[brendanzab]

Thanks a bunch for the feedback!

[kvark]

create_frame_buffer should not return a u32.

Fixed by #240

[tomaka]

This is possible, but there are more types. How do you infer Texture2DArray or TextureCube?

In simple-gl-rs, I'm passing an additional arraySize parameter when creating textures.
TextureCube would need a separate function.

We have #210, which will show multi-threading and working with frames. Anything else you think requires explicit coverage?

Ideally most OpenGL features should be covered by examples, but this is a lot of work and is not urgent.

[kvark]

In simple-gl-rs, I'm passing an additional arraySize parameter when creating textures.
TextureCube would need a separate function.

To be fair, I believe we did consider that before implementing the texturing support, because that's pretty much what Claymore had. I can't tell you why we ended up settling with an explicit kind in TextureInfo, but we are going to re-evaluate it for sure. @cmr what do you think?

[emberian]

I chose to do the explicit kind because I feel it is less error-prone and is easier to read/understand.

[kvark]

Remove all delete_* functions. RAII should handle everything.
Mesh should contain the BufferHandle to ensure that the buffer is not destroyed as long as the mesh exists.

Moved into a separate issue #288 for RAII.

[kvark]

Remove all type Foo = Bar in favor of struct Foo(Bar).

Covered by #145

[kvark]

This becomes slightly obsolete, and a lot of points are already done. Lowering the priority.

[kvark]

Not planning on doing these changes in pre-ll