Skip to content

Conversation

@hrydgard
Copy link
Owner

@hrydgard hrydgard commented Mar 4, 2023

Just something I noticed we didn't do, while poking around the texture code.

Might marginally increase texture upload performance on some GPUs, but mainly just the right thing to do.

For example, on Intel, this is 64.

@hrydgard hrydgard added the Vulkan label Mar 4, 2023
@hrydgard hrydgard added this to the v1.15.0 milestone Mar 4, 2023
int bpp = VkFormatBytesPerPixel(actualFmt);
int stride = (mipWidth * bpp + 15) & ~15; // output stride
int uploadSize = stride * mipHeight;
int optimalStrideAlignment = std::max(4, (int)vulkan->GetPhysicalDeviceProperties().properties.limits.optimalBufferCopyRowPitchAlignment);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This isn't actually used? Shouldn't we have int byteStride = std::max(optimalStrideAlignment, RoundUpToPowerOf2(mipWidth * bpp)); or something (more complex if it wouldn't always be a power of two)?

-[Unknown]

Copy link
Owner Author

@hrydgard hrydgard Mar 5, 2023

Choose a reason for hiding this comment

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

Ugh, it was supposed to be passed in as the second parameter in RoundUpToPower2 (there's now a verison that takes that, which is useful sometimes). I think it's guaranteed to be a power of two, I don't see any non-pow2 values in vulkan.gpuinfo.org either.

…account

Might marginally increase texture upload performance on some GPUs, but
mainly just the right thing to do.

For example, on Intel, this is 64.
@hrydgard hrydgard force-pushed the use-optimal-image-stride branch from 33b4600 to cc9c01b Compare March 5, 2023 16:33
@unknownbrackets unknownbrackets merged commit 7a52f3a into master Mar 5, 2023
@unknownbrackets unknownbrackets deleted the use-optimal-image-stride branch March 5, 2023 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants