hal: Improve buffer documentation and cleanup error handling #2154

msiglreith · 2018-06-16T14:37:50Z

Fixes #issue
PR checklist:

make succeeds (on *nix)
make reftests succeeds
tested examples with the following backends:

kvark · 2018-06-19T16:57:17Z

src/hal/src/buffer.rs

+/// Index buffer view for `bind_index_buffer`.
+///
+/// Defines a buffer slice used for acquiring the indicies on draw commands.
+/// Indices are used to lookup the vertex indices in the vertex buffers.


"to lookup vertices" perhaps?

kvark · 2018-06-19T16:58:12Z

src/hal/src/buffer.rs

-    }
+/// Error creating a buffer.
+#[derive(Fail, Debug, Clone, PartialEq, Eq)]
+pub enum BufferCreationError {


what's the motivation behind having the Buffer* prefix for those errors? If we are exposing buffer as a module, I find it elegant to keep it buffer::CreationError. If we are using the fully qualified names, there is no need to expose the modules.

I'm not sure which direction we should go for the errors.
Currently having a error module containing DeviceCreationError and a few more. On the other side we could also move these errors into the different modules as done here with possible name shortening.

I'm (non-strongly) in favor of having short error names in respective modules (as opposed to putting them into a dedicated error module)

yeah I think this will be more clear. changed.

kvark

LGTM. Should we squash it?

msiglreith · 2018-06-21T21:41:13Z

bors r=kvark

2154: hal: Improve buffer documentation and cleanup error handling r=kvark a=msiglreith Fixes #issue PR checklist: - [ ] `make` succeeds (on *nix) - [ ] `make reftests` succeeds - [ ] tested examples with the following backends: Co-authored-by: msiglreith <[email protected]>

bors · 2018-06-22T02:31:59Z

Build failed

continuous-integration/appveyor/branch

kvark · 2018-06-22T03:41:12Z

bors retry

2154: hal: Improve buffer documentation and cleanup error handling r=kvark a=msiglreith Fixes #issue PR checklist: - [ ] `make` succeeds (on *nix) - [ ] `make reftests` succeeds - [ ] tested examples with the following backends: Co-authored-by: msiglreith <[email protected]>

bors · 2018-06-22T03:55:57Z

Build failed

continuous-integration/travis-ci/push

msiglreith · 2018-06-22T10:01:34Z

bors retry

msiglreith · 2018-06-22T10:31:09Z

bors r=kvark

bors · 2018-06-22T10:31:11Z

Not awaiting review

2154: hal: Improve buffer documentation and cleanup error handling r=kvark a=msiglreith Fixes #issue PR checklist: - [ ] `make` succeeds (on *nix) - [ ] `make reftests` succeeds - [ ] tested examples with the following backends: Co-authored-by: msiglreith <[email protected]>

bors · 2018-06-22T10:49:04Z

Build failed

continuous-integration/travis-ci/push

kvark · 2018-06-22T13:05:07Z

bors retry

kvark · 2018-06-22T17:52:12Z

bors retry

kvark · 2018-06-22T17:52:31Z

bors r+

bors · 2018-06-22T17:52:32Z

Not awaiting review

bors · 2018-06-22T17:52:33Z

Not awaiting review

2154: hal: Improve buffer documentation and cleanup error handling r=kvark a=msiglreith Fixes #issue PR checklist: - [ ] `make` succeeds (on *nix) - [ ] `make reftests` succeeds - [ ] tested examples with the following backends: Co-authored-by: msiglreith <[email protected]>

bors · 2018-06-22T18:07:11Z

Build failed

continuous-integration/travis-ci/push

kvark · 2018-06-22T22:07:57Z

Bors retry

…

On Jun 22, 2018, at 14:07, bors[bot] ***@***.***> wrote: Build failed continuous-integration/travis-ci/push — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

kvark · 2018-06-23T00:17:23Z

Bors r+

…

On Jun 22, 2018, at 18:07, Dzmitry Malyshau ***@***.***> wrote: Bors retry > On Jun 22, 2018, at 14:07, bors[bot] ***@***.***> wrote: > > Build failed > > continuous-integration/travis-ci/push > — > You are receiving this because you commented. > Reply to this email directly, view it on GitHub, or mute the thread.

bors · 2018-06-23T00:17:24Z

Not awaiting review

2154: hal: Improve buffer documentation and cleanup error handling r=kvark a=msiglreith Fixes #issue PR checklist: - [ ] `make` succeeds (on *nix) - [ ] `make reftests` succeeds - [ ] tested examples with the following backends: Co-authored-by: msiglreith <[email protected]>

bors · 2018-06-23T00:36:47Z

Build failed

continuous-integration/appveyor/branch

kvark · 2018-06-23T03:32:38Z

bors r+

2071: Remaping descriptor sets in the gl backend r=kvark a=ZeGentzy I'll rebase off master when I'm done. Uniforms in gl only have a bindings field, not a set one. This means that for shaders that use multiple sets to work, we must change where we are binding them. See page 14 for what I mean: https://www.khronos.org/assets/uploads/developers/library/2016-vulkan-devday-uk/4-Using-spir-v-with-spirv-cross.pdf PR checklist: - [ ] `make` succeeds (on *nix) - [ ] `make reftests` succeeds - [ ] tested examples with the following backends: 2149: [dx11] add memory flush/invalidate & image/buffer copies r=kvark a=fkaa Main changes are adding more robust implementation of `Memory`, adding flush/invalidate and adding image/buffer copies. Implements `Memory` like the following (for `HOST_VISIBLE` memory): ``` 0.........................size +----------------------------+ | Memory | +----------------------------+ A..B C.....D E...F 1 fixed-size `STAGING` buffer which gets used for reading back from resources.(and should be used to copy from/to on flush/invalidate): (0..size, ComPtr<Buffer>) 1 `Vec<u8>` which covers the whole memory range (0..size). This is pointer we hand out to users. flush/invalidate moves the affected regions into our `STAGING` buffer to get read/uploaded. *N* Resources: (A..B, ComPtr<Resource>), (C..D, ComPtr<Resource>), (E..F, ComPtr<Resource>), ``` Implements copying between images and buffers. Image<->Image copies are mostly handled by `CopySubresourceRegion` but some formats, while same size, cant be copied with this method: > Cannot invoke CopySubresourceRegion when the Formats of each Resource are not the same or at least castable to each other, unless one format is compressed (DXGI_FORMAT_R9G9B9E5_SHAREDEXP, or DXGI_FORMAT_BC[1,2,3,4,5]_* ) and the source format is similar to the dest according to: BC[1|4] ~= R16G16B16A16|R32G32, BC[2|3|5] ~= R32G32B32A32, R9G9B9E5_SHAREDEXP ~= R32. [ RESOURCE_MANIPULATION ERROR #281: ] These has to be done through compute shaders instead. Image->Buffer & Buffer->Image copies also have to be done through compute shaders, as `CopySubresourceRegion` can only copy between resources of same type (Image<->Image, Buffer<->Buffer). The following formats have Buffer->Image and Image->Buffer copies implemented with these changes: * `R8` * `Rg8` * `R16` * `Rg16` * `R32` Gets about 400 tests passed and equal amount failed in `dEQP-VK.api.copy_and_blit.core.image_to_image.all_formats.color` (mostly because `CopySubresourceRegion` failing to copy between some formats as mentioned above) Fixes #issue PR checklist: - [ ] `make` succeeds (on *nix) - [ ] `make reftests` succeeds - [ ] tested examples with the following backends: 2154: hal: Improve buffer documentation and cleanup error handling r=kvark a=msiglreith Fixes #issue PR checklist: - [ ] `make` succeeds (on *nix) - [ ] `make reftests` succeeds - [ ] tested examples with the following backends: Co-authored-by: Hal Gentz <[email protected]> Co-authored-by: Felix Kaaman <[email protected]> Co-authored-by: msiglreith <[email protected]>

bors · 2018-06-23T09:48:30Z

Build succeeded

kvark reviewed Jun 19, 2018

View reviewed changes

kvark approved these changes Jun 21, 2018

View reviewed changes

hal: Improve buffer documentation and cleanup error handling

8c8281c

msiglreith force-pushed the buffer_doc branch from c6ab725 to 8c8281c Compare June 21, 2018 21:41

bors bot merged commit 8c8281c into gfx-rs:master Jun 23, 2018

msiglreith deleted the buffer_doc branch June 23, 2018 10:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

hal: Improve buffer documentation and cleanup error handling #2154

hal: Improve buffer documentation and cleanup error handling #2154

msiglreith commented Jun 16, 2018

kvark Jun 19, 2018

kvark Jun 19, 2018

msiglreith Jun 21, 2018

kvark Jun 21, 2018

msiglreith Jun 21, 2018

kvark left a comment

msiglreith commented Jun 21, 2018

bors bot commented Jun 22, 2018

kvark commented Jun 22, 2018

bors bot commented Jun 22, 2018

msiglreith commented Jun 22, 2018

msiglreith commented Jun 22, 2018

bors bot commented Jun 22, 2018

bors bot commented Jun 22, 2018

kvark commented Jun 22, 2018

kvark commented Jun 22, 2018

kvark commented Jun 22, 2018

bors bot commented Jun 22, 2018

bors bot commented Jun 22, 2018

bors bot commented Jun 22, 2018

kvark commented Jun 22, 2018 via email

kvark commented Jun 23, 2018 via email

bors bot commented Jun 23, 2018

bors bot commented Jun 23, 2018

kvark commented Jun 23, 2018

bors bot commented Jun 23, 2018

hal: Improve buffer documentation and cleanup error handling #2154

hal: Improve buffer documentation and cleanup error handling #2154

Conversation

msiglreith commented Jun 16, 2018

kvark Jun 19, 2018

Choose a reason for hiding this comment

kvark Jun 19, 2018

Choose a reason for hiding this comment

msiglreith Jun 21, 2018

Choose a reason for hiding this comment

kvark Jun 21, 2018

Choose a reason for hiding this comment

msiglreith Jun 21, 2018

Choose a reason for hiding this comment

kvark left a comment

Choose a reason for hiding this comment

msiglreith commented Jun 21, 2018

bors bot commented Jun 22, 2018

Build failed

kvark commented Jun 22, 2018

bors bot commented Jun 22, 2018

Build failed

msiglreith commented Jun 22, 2018

msiglreith commented Jun 22, 2018

bors bot commented Jun 22, 2018

bors bot commented Jun 22, 2018

Build failed

kvark commented Jun 22, 2018

kvark commented Jun 22, 2018

kvark commented Jun 22, 2018

bors bot commented Jun 22, 2018

bors bot commented Jun 22, 2018

bors bot commented Jun 22, 2018

Build failed

kvark commented Jun 22, 2018 via email

kvark commented Jun 23, 2018 via email

bors bot commented Jun 23, 2018

bors bot commented Jun 23, 2018

Build failed

kvark commented Jun 23, 2018

bors bot commented Jun 23, 2018

Build succeeded