dariomanesku / cmft

Cross-platform open-source command-line cubemap filtering tool.
Other
825 stars 94 forks source link

KTX roundng #29

Open andy-thomason opened 7 years ago

andy-thomason commented 7 years ago

Thanks for an excellent tool. I am using it in the cubemap examples for my Vulkan helper library, Vookoo.

I have seen a potential bug in the KTX write code when rounding is following GL_UNPACK_ALIGNMENT

The image size does not seem to take account of the change in row pitch due to rounding and reports a smaller value that required. This may just be a misunderstanding of the KTX format, so please ignore this if it is an incorrect assumption.

dariomanesku commented 7 years ago

Have a look at imageSaveKtx(): https://github.com/dariomanesku/cmft/blob/a8c246a4b59670092f34209bf8247f79337526ed/src/cmft/image.cpp#L5231

Unpack alignment is taken into account when writing data: https://github.com/dariomanesku/cmft/blob/a8c246a4b59670092f34209bf8247f79337526ed/src/cmft/image.cpp#L5293

but faceSize is not affected by that: https://github.com/dariomanesku/cmft/blob/a8c246a4b59670092f34209bf8247f79337526ed/src/cmft/image.cpp#L5298

Try recalculating faceSize with rounding and see if that solves the problem.

andy-thomason commented 7 years ago

Thanks, Dario.

I would suggest changing

const uint32_t pitch = width bytesPerPixel; const uint32_t faceSize = pitch height;

For

const uint32_t pitch = width bytesPerPixel; const uint32_t roundedPitch = (width bytesPerPixel + KTX_UNPACK_ALIGNMENT-1) & ~(KTX_UNPACK_ALIGNMENT-1); const uint32_t faceSize = roundedPitch * height;

As you suggest.

I'll try this the next time I am at home with my Linux machine.

On Sun, Apr 9, 2017 at 11:35 PM, Dario Manesku notifications@github.com wrote:

Have a look at imageSaveKtx(): https://github.com/dariomanesku/cmft/blob/a8c246a4b59670092f34209bf8247f 79337526ed/src/cmft/image.cpp#L5231

Unpack alignment is taken into account when writing data: https://github.com/dariomanesku/cmft/blob/a8c246a4b59670092f34209bf8247f 79337526ed/src/cmft/image.cpp#L5293

but faceSize is not affected by that: https://github.com/ dariomanesku/cmft/blob/a8c246a4b59670092f34209bf8247f 79337526ed/src/cmft/image.cpp#L5298

Try recalculating faceSize with rounding and see if that solves the problem.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/dariomanesku/cmft/issues/29#issuecomment-292818506, or mute the thread https://github.com/notifications/unsubscribe-auth/ABe_XFNIF99qfFThWAkdIQ2CIBvrhgacks5ruV0VgaJpZM4M1h9z .