Skip to content
This repository was archived by the owner on Oct 31, 2025. It is now read-only.

Safeness bug in StorageImage2d::write#453

Merged
eddyb merged 3 commits intomainfrom
unsafe-mutable-storage-buffer
Mar 1, 2021
Merged

Safeness bug in StorageImage2d::write#453
eddyb merged 3 commits intomainfrom
unsafe-mutable-storage-buffer

Conversation

@Jasper-Bekkers
Copy link
Copy Markdown
Contributor

@Jasper-Bekkers Jasper-Bekkers commented Feb 26, 2021

It's pretty easy to see why this is unsafe, if multiple threads write to the same coordinate race-conditions happen (for example calling this with Coord(0, 0) in a pixel shader that covers more then one pixel would trample data).

Ultimately this should be addressed through something like #216 and some higher level abstractions on top of our buffer types, but since we don't have those for now, marking this as unsafe seems to be the only thing we can do for now.

It's pretty easy to see why this is unsafe, if multiple threads write to the same coordinate race-conditions happen.

Ultimately this should be addressed through something like #216 and some higher level abstractions on top of our buffer types, but since we don't have those for now, marking this as unsafe seems to be the only thing we can do for now.
Copy link
Copy Markdown
Contributor

@eddyb eddyb left a comment

Choose a reason for hiding this comment

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

Seems straight-forward enough, cc @khyperia for visibility.

EDIT: oh can't land anyway, failed CI

@eddyb eddyb enabled auto-merge (squash) February 26, 2021 19:22
@XAMPPRocky
Copy link
Copy Markdown
Member

LGTM, just need to update the test to use unsafe.

image.write(glam::UVec2::new(0, 1), texels);

Copy link
Copy Markdown
Contributor

@khyperia khyperia left a comment

Choose a reason for hiding this comment

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

LGTM once the test is fixed

@eddyb eddyb merged commit 2f4c67e into main Mar 1, 2021
@eddyb eddyb deleted the unsafe-mutable-storage-buffer branch March 1, 2021 15:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants