Gfx examples framework #52

kvark · Feb 1, 2019

Fixes #37
~~It's not functional yet, I think it needs a proper resize event handler. We can do this incrementally.~~
Draws a cube 🎉 . Includes a few pieces:

the example framework
sampler support
vertex/index buffer description
buffer to texture copies

kvark · Feb 4, 2019

Mission accomplished (Metal):

grovesNL · Feb 4, 2019

grovesNL reviewed Feb 4, 2019

View changes

Awesome work porting the cube example over, this looks great! 🎉

grovesNL · Feb 4, 2019

gfx-examples/README.md

@@ -0,0 +1,6 @@
+# gfx pre-ll examples
+
+Original gfx-rs examples were growing for years, but then got abandoned once we changed the API to match vulkan:


Maybe this could be rephrased a bit:

The original gfx-rs examples had grown over several years, but they were abandoned when the gfx API was changed to match Vulkan.
https://github.com/gfx-rs/gfx/tree/pre-ll/examples

wgpu-rs is considered to be the spiritual successor of gfx pre-ll, so this is the new home for the examples.

grovesNL · Feb 4, 2019

gfx-examples/data/cube.frag

@@ -0,0 +1,13 @@
+#version 450
+#extension GL_ARB_separate_shader_objects : enable


We shouldn't actually need in either shader AFAIK (see Overv/VulkanTutorial#112)

grovesNL · Feb 4, 2019

gfx-examples/data/cube.frag

+
+void main() {
+    vec4 tex = texture(sampler2D(t_Color, s_Color), v_TexCoord);
+    float blend = dot(v_TexCoord-vec2(0.5,0.5), v_TexCoord-vec2(0.5,0.5));


This could be simplified a bit:

float mag = length(v_TexCoord - vec2(0.5)); o_Target = mix(tex, vec4(0.0), mag * mag);

grovesNL · Feb 4, 2019

gfx-examples/src/framework.rs

+    use std::path::PathBuf;
+
+    let base_path = PathBuf::from("data").join(name);
+    let code_vs = read_to_string(base_path.with_extension("vert")).unwrap();


nit: to keep framework light, maybe we could just have load_glsl and pass the cube.vert and ShaderType::Vertex, then we could use this for compute shaders too

grovesNL · Feb 4, 2019

wgpu-native/src/command/mod.rs

+        });
+    }
+
+    let bytes_per_texel = conv::map_texture_format(dst_texture.format).surface_desc().bits as u32 / 8;


nit: maybe have a constant for 8

grovesNL · Feb 4, 2019

wgpu-native/src/conv.rs

+    match address {
+        Am::ClampToEdge => W::Clamp,
+        Am::Repeat => W::Tile,
+        Am::MirrorRepeat => {


How does W::Mirror differ?

grovesNL · Feb 4, 2019

wgpu-native/src/device.rs

+    let desc_vbs = unsafe {
+        slice::from_raw_parts(desc.vertex_buffer_state.vertex_buffers, desc.vertex_buffer_state.vertex_buffers_count)
+    };
+    let mut vertex_buffers: Vec<hal::pso::VertexBufferDesc> = Vec::with_capacity(desc_vbs.len());


nit: we don't need the type annotations for either of these unless you prefer it

grovesNL · Feb 4, 2019

wgpu-rs/src/lib.rs

-        )
+impl<'a> Drop for SwapChainOutput<'a> {
+    fn drop(&mut self) {
+        wgn::wgpu_swap_chain_present(*self.swap_chain_id);


I think this could be a bit unexpected by users. If the concern is that the frame is accidentally held by the user after drop, maybe we could keep the explicit present and have an assert on drop or similar?

The swapchain output is now holding the swapchain from modification: they can't get new frames anyway. There is nothing the user can do to screw this up. They can only stop using the output and drop it.
We could totally have explicit "present", but I didn't find it useful. Open for re-consideration ;)

I think it's a bit unexpected not to have the usual present or swap_buffers at the end of rendering a frame (although WebGL does this implicitly too). Also drop causes some work to be performed here, which doesn't seem very common.

This could be confusing, especially if the user is trying to map calls between the WebGPU specification and wgpu-rs and doesn't notice where the missing present went. Principle of least astonishment and all that :)

That's how the W3C group agreed to do things on IDL side, interestingly.

grovesNL · Feb 4, 2019

grovesNL reviewed Feb 4, 2019

View changes

grovesNL · Feb 4, 2019

gfx-examples/src/cube.rs

+extern crate cgmath;
+extern crate wgpu;
+
+#[path="framework.rs"]


I think path= isn't commonly used so maybe we should just have framework or use an alias here

grovesNL · Feb 4, 2019

gfx-examples/src/cube.rs

@@ -0,0 +1,310 @@
+extern crate cgmath;
+extern crate wgpu;


We could have edition = "2018" in Cargo.toml and remove these extern crates

kvark · Feb 4, 2019

Amazing input, thank you @grovesNL !
I believe everything is addressed now. I also took the liberty of modifying the cube texture to show that, well, we have a texture:
$wgpu-cube-fractal$

bors r=grovesNL

bors · Feb 4, 2019

Build succeeded

continuous-integration/travis-ci/push

kvark · Feb 5, 2019

Update: this works on Vulkan 🎉 although I haven't checked for validation issues.
DX12 doesn't work because of missing update_buffer.

kvark requested a review from grovesNL Feb 1, 2019

kvark added some commits Feb 1, 2019

Borrow temporary queus and frames on Rust side

afe00aa

Basic gfx-examples framework

ed5499d

Sampler creation support

a735edd

gfx framework GLSL reading, most of the cube example

Loading status checks…

d4c8c88

kvark force-pushed the kvark:gfx-examples branch from b0f3c2a to d4c8c88 Feb 3, 2019

kvark added some commits Feb 3, 2019

rust: vertex/index binding

Loading status checks…

3d47436

Vertex buffer description in the pipelines

3ec4ff9

Buffer to texture copies

Loading status checks…

d50f819

Fix gfx-cube example

Loading status checks…

8f985dd

Cube cleanup and the new generated texture

Loading status checks…

c74f8e8

bors bot merged commit c74f8e8 into gfx-rs:master Feb 4, 2019
2 checks passed

2 checks passed

bors Build succeeded
Details

continuous-integration/travis-ci/pr The Travis CI build passed
Details

kvark deleted the kvark:gfx-examples branch Feb 4, 2019

kvark referenced this pull request Feb 4, 2019
Open
Support for the Vulkan coordinate system #449

gfx-rs/wgpu

Gfx examples framework #52

Gfx examples framework #52

kvark commented Feb 1, 2019 •

edited

kvark requested a review from grovesNL Feb 1, 2019

kvark added some commits Feb 1, 2019

kvark force-pushed the kvark:gfx-examples branch from `b0f3c2a` to `d4c8c88` Feb 3, 2019

kvark added some commits Feb 3, 2019

This comment has been minimized.

kvark commented Feb 4, 2019

grovesNL reviewed Feb 4, 2019

View changes

grovesNL left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

grovesNL reviewed Feb 4, 2019

View changes

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

kvark commented Feb 4, 2019

bors bot added a commit that referenced this pull request Feb 4, 2019

This comment has been minimized.

bors bot commented Feb 4, 2019

bors bot merged commit `c74f8e8` into gfx-rs:master Feb 4, 2019
2 checks passed

2 checks passed

kvark deleted the kvark:gfx-examples branch Feb 4, 2019

kvark referenced this pull request Feb 4, 2019

Support for the Vulkan coordinate system #449

This comment has been minimized.

kvark commented Feb 5, 2019

gfx-rs/wgpu

Join GitHub today

Gfx examples framework #52

Conversation

kvark commented Feb 1, 2019 • edited

kvark requested a review from grovesNL Feb 1, 2019

kvark added some commits Feb 1, 2019

kvark force-pushed the kvark:gfx-examples branch from b0f3c2a to d4c8c88 Feb 3, 2019

kvark added some commits Feb 3, 2019

This comment has been minimized.

kvark commented Feb 4, 2019

grovesNL reviewed Feb 4, 2019 View changes

grovesNL left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

grovesNL reviewed Feb 4, 2019 View changes

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

kvark commented Feb 4, 2019

bors bot added a commit that referenced this pull request Feb 4, 2019

This comment has been minimized.

bors bot commented Feb 4, 2019

Build succeeded

Hide details View details bors bot merged commit c74f8e8 into gfx-rs:master Feb 4, 2019 2 checks passed

2 checks passed

kvark deleted the kvark:gfx-examples branch Feb 4, 2019

kvark referenced this pull request Feb 4, 2019

This comment has been minimized.

kvark commented Feb 5, 2019

kvark commented Feb 1, 2019 •

edited

kvark force-pushed the kvark:gfx-examples branch from `b0f3c2a` to `d4c8c88` Feb 3, 2019

grovesNL reviewed Feb 4, 2019

View changes

grovesNL reviewed Feb 4, 2019

View changes

bors bot merged commit `c74f8e8` into gfx-rs:master Feb 4, 2019
2 checks passed