Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

re_renderer: delegate 2D texture padding to Queue::write_texture #975

Merged
merged 1 commit into from
Jan 30, 2023

Conversation

teh-cmc
Copy link
Member

@teh-cmc teh-cmc commented Jan 28, 2023

I was working on some unrelated wgpu stuff, and stumbled upon this and this in the docs, which state that manually padding 2D textures is unnecessary when uploading them via a Queue as it already handles alignment internally.

Obviously this would need to come back in some form (that would likely look slightly different though) once we introduce our own staging belts for upload, but in the mean time this effectively annihilates all padding related performance issues.

I imagine I'm late to the party and all of this has been known for a while and there's a reason we've decided not to take advantage of this, in which case feel free to close this and at least there's a documented trace of that decision now! :)


colmap in debug before:
image

colmap in debug after:
image

Copy link
Member

@Wumpf Wumpf left a comment

Choose a reason for hiding this comment

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

I had no idea about this, that's why 😄

Sooo but there is a catch here I have to assume: Currently, we don't track any overhead that happens inside the submit and wgpu frame maintenance calls where all that padding is happening now instead! (because that stuff is in eframe and we don't have any profiling scopes spanning that :(( )

I.e. the big question here then is if the overhead just moved to somewhere where we don't see it right now. Before we haven't answered that I wouldn't want to merge this

Copy link
Member

@Wumpf Wumpf left a comment

Choose a reason for hiding this comment

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

The padding is happening here https://github.com/gfx-rs/wgpu/blob/master/wgpu-core/src/device/queue.rs#L756
which means it's done without delay so still covered by our profiling scope!

So it seems that this loop of copy_nonoverlapping is just so much more efficient than copy_from_slice Oo
But this also kinda restores my faith into copy speeds. Was surprised it is that low for us

@Wumpf
Copy link
Member

Wumpf commented Jan 28, 2023

kinda curious if the difference is that big in release mode as well 🤔
I mean copy_from_slice without optimization is probably a loop that copies every element one after the other, but it does make me wonder if it is still that bad with optimization
(I do care about our dev builds though, so this is no reason to ever suffer this slowdown when it matters like this)

@teh-cmc teh-cmc merged commit c9209e8 into main Jan 30, 2023
@teh-cmc teh-cmc deleted the cmc/renderer/delegate_padding branch January 30, 2023 08:03
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.

None yet

2 participants