Skip to content

Improve buffer and texture overrun error messages#885

Merged
bors[bot] merged 1 commit intogfx-rs:masterfrom
cwfitzgerald:copy-buffer-to-buffer
Aug 19, 2020
Merged

Improve buffer and texture overrun error messages#885
bors[bot] merged 1 commit intogfx-rs:masterfrom
cwfitzgerald:copy-buffer-to-buffer

Conversation

@cwfitzgerald
Copy link
Copy Markdown
Member

Connections

Closes #884.

Description

The previous errors about buffer and texture overruns didn't tell the user any information about which buffer, how long wgpu thought the copy was, or how long it thought the buffer was. This makes the error message much better.

Unfortunately a braking change, so can't backport :(

Testing

Tested on #884's issue.

@cwfitzgerald cwfitzgerald requested a review from kvark August 19, 2020 15:13
}

#[derive(Clone, Debug)]
pub enum TextureErrorDimension {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto

Copy link
Copy Markdown
Contributor

@monocodus monocodus bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an autogenerated code review.

Checker summary (by rust_clippy):
The tool has found 0 warnings, 1 errors.

Copy link
Copy Markdown
Member

@kvark kvark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice, thank you!
Just a few notes here

Destination,
}

impl fmt::Display for ResourceSide {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not just derive it?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because then it would be the wrong capitalization for the error message

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, but it's an error message, so it's fine. I.e. you can put it into quotes to separate from the rest of the message
but it would save code, and that's a good thing to have

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

Z,
}

impl fmt::Display for TextureErrorDimension {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not derived?

bors bot added a commit that referenced this pull request Aug 19, 2020
886: [0.6] Backport buffer and texture overrun error messages r=kvark a=cwfitzgerald

**Connections**

#885 but neutered to be a non-breaking change.

**Description**

The original error message talked only about the destination buffer, which is very confusing as the cause of the overrun could be the source buffer.

Not worth a release on its own, but next time there's a release, we can get this fixed.

**Testing**

Strings only.


Co-authored-by: Connor Fitzgerald <connorwadefitzgerald@gmail.com>
@cwfitzgerald cwfitzgerald force-pushed the copy-buffer-to-buffer branch from f22b62c to 608eff1 Compare August 19, 2020 15:39
@cwfitzgerald cwfitzgerald force-pushed the copy-buffer-to-buffer branch from 608eff1 to f9265bc Compare August 19, 2020 15:45
Copy link
Copy Markdown
Member

@kvark kvark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!
bors r+

@bors
Copy link
Copy Markdown
Contributor

bors bot commented Aug 19, 2020

@bors bors bot merged commit 1d0e0ce into gfx-rs:master Aug 19, 2020
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 this pull request may close these issues.

Copy buffer-to-buffer error

2 participants