Yatekii / imgui-wgpu-rs

Dear imgui renderer for wgpu-rs.
MIT License
256 stars 86 forks source link

Update to wgpu-rs 0.8 #54

Closed klashenriksson closed 3 years ago

klashenriksson commented 3 years ago

Hello!

wgpu-rs 0.8 was just released and had some minor changes to structure names and parameters. I have a fork ready which is updated to use wgpu 0.8, should I do a PR of that to bring this up to 0.8? :)

klashenriksson commented 3 years ago

update: I spoke too soon. There's a runtime crash with error message that index buffers must be multiple of 4, which seems a bit weird... Crash occurs when uploading index buffers:

        for draw_list in draw_data.draw_lists() {
            self.vertex_buffers
                .push(self.upload_vertex_buffer(device, draw_list.vtx_buffer()));
            self.index_buffers
                .push(self.upload_index_buffer(device, draw_list.idx_buffer()));
        }

starting at line 498 in lib.rs

Uriopass commented 3 years ago

Any update? Did you manage to get it working? I might take a look if you didn't :-)

benmkw commented 3 years ago

@Uriopass maybe you can look at https://github.com/Yatekii/imgui-wgpu-rs/pull/52#issuecomment-832811358 which still at least has the 4 byte issue

klashenriksson commented 3 years ago

Hi @Uriopass ! I did get it working, however the fix is certainly not the most effective one, but works fine for my cases atm. Upon index upload I just convert the indices to u32's instead of u16's.

Have a look at https://github.com/klashenriksson/imgui-wgpu-rs/commits/wgpu-v0.8 (and https://github.com/klashenriksson/imgui-wgpu-rs/commit/3c01713adac29c3c7b0414a00f6985118128d58f for the alignment fix). I dont really have the bandwidth to do a proper fix for this atm but maybe my code can help get you started :)

Uriopass commented 3 years ago

Found the issue upstream, buffer initialization was not done properly when providing unaligned data :-)

klashenriksson commented 3 years ago

Nice! Good find :)

benmkw commented 3 years ago

@Uriopass nice, it still seems to be an issue that TriangleList can not be used with u16 indices though?

I get [0.094421 ERROR]()(no module): Unexpected varying type: Array { base: [1], size: Constant([5]), stride: 4 } when using u32 as the index format (which is wrong as far as I can see) although rendering works, I'm not sure why though 😄 when I change it to u16 I get strip index format was not set to None but to Some(Uint16) while using the non-strip topology TriangleList

Uriopass commented 3 years ago

when I change it to u16 I get strip index format was not set to None but to Some(Uint16) while using the non-strip topology TriangleList

I'm pretty sure imgui uses a TriangleList topology :) you should keep the PrimitiveTopology::TriangleList and keep strip_index_format to None.

I do get the error "[0.094421 ERROR]()(no module): Unexpected varying type: Array { base: [1], size: Constant([5]), stride: 4 }" however I think that might just be a naga problem and it works for me it's not critical.

benmkw commented 3 years ago

you should keep the PrimitiveTopology::TriangleList

yes thats what I figured as well but when you look at upload_index_buffer and DrawIdx you can see that its u16 so it may only work by luck ...

keep strip_index_format to None

this makes the indices be u32

Uriopass commented 3 years ago

this makes the indices be u32

No it doesn't, the indices format is determined at the set_index_buffer time in the renderpass. https://docs.rs/wgpu/0.8.1/wgpu/struct.RenderPass.html#method.set_index_buffer

the strip_index_format should only be specified when in TriangleStrip mode, thus why you get the error.

benmkw commented 3 years ago

ah right somehow mixed None and Default in my mind and did not know enough about the API, thanks for the explanation 👍