Repository navigation
pbit related quality fix for BC7 texture encoding - #93
Rich Geldreich (richgel999) wants to merge 4 commits into
Conversation
…ETS, the overall output error increases across the kodim test corpus. This is incorrect: the overall error should decrease. Other BC7 encoders don't behave in this way, so I investigated further. I modified D3DX_BC7::Refine() to call D3DXDecodeBC7() on the encoded block, computed the actual error, and compared that vs. the error computed by Refine(). They differed, which is incorrect. The bug is caused by D3DX_BC7::Refine() not correctly computing the block error in a way that factors in the actual coded pbits. It computes the error using the endpoints before the LSB's are slammed to the pbits, causing the encoder to choose blocks that actually increase error. This will wreck quality in pbit modes. The fix introduces a new helper function D3DX_BC7::FixEndpointPBits(), which Refine() calls after it computes new endpoints. It could be easily improved further for more gains (it doesn't round the component values correctly factoring in the pbits), but it's a massive improvement. With this fix applied the encoding quality across my test corpus increased from 44.25 to 46.02 with 3 subsets enabled, and 45.64 with 3 subsets disabled. (My encoder and ispc_texcomp get >46.5 dB with 3 subsets enabled, so thi
|
Thanks. I'm working on a validator test to get better regression coverage when taking BC changes. Once I'm able to run a baseline I can try your change and verify it works as advertised. |
|
Yes, independent validation is really necessary and I'll wait on submitting any more improvements until you have this going. I have another set of simple changes that introduces a new flag (BC_FLAGS_FASTER_BC7) that improves encoding speed by ~10x. Quality (without 3 subsets enabled) across my corpus is reduced by 1 dB (from 45.64 to 44.65), but it's still stronger than the original encoder with the pbit bug (which was 44.4 dB). Without the flag the encoder works as before, and it's combinable with the other flags (3 subsets and quick). The changes are very simple because they just leverage the existing encoder and add several heuristics used by other encoders. The first fix disables mode 7 on completely opaque blocks. Mode 7 isn't useful because the other modes are just as strong for opaque cases. There's no quality loss from this change, but it's noticeably faster (12429 vs. 9756 secs). The other improvements: Only refine a single partition for each mode, use bounding box approximation vs. full PCA in OptimizeRGB/OptimizeRGBA when estimating partitions, use mode 1's estimated partition for mode 3, and disable the deeper exhaustive refinement. |
…ncoding at higher quality than the original encoder before the pbit bug fixes (avg. 44.4 dB original, vs. 44.65 dB with this flag across 31 images). In this mode: - Mode 7 is skipped if the block is entirely opaque (it's redundant) - Only a single partition is examined per mode after partition estimation - Mode 3 uses the same estimated partition as mode 1 (they're both 2 subset modes with 64 partitions) - When estimating partitions in RoughMSE(), only the RGB(A) bounding box is used to estimate the principle axis (van Waveren's approximation) - Disabling exhaustive endpoint optimization in fast mode This flag is combinable with the other flags (3 subsets, mode 6 only), which gives the encoder a number of useful perf vs. quality options.
|
I've checked in the code for the new flag: I've tested with both opaque and complex transparent textures. |
|
Should probably fix the indentation formatting. The existing code uses spaces, but these changes use hard tabs. |
|
Ok - I'll fix this within a day or two. |
|
Sorry I thought this was the pull request for that issue. If not, please submit it as a distinct PR. Thanks! |
|
BTW - ImageMagick can compare images and output PSNR: I like using tools like this because it's a good independent comparison point. My PSNR's are compatible with this tool. |
|
I appreciate the contribution, but this pull request seems to merge a lot of distinct optimizations into one big change so I'm having trouble validating each part of it... I'm going to try to tease apart the different fixes here, in particular the mode 7/opaque skip case and the original fix for this issue which I want to try out independent of your new suggested "FASTER" flag... |
|
I've taken the fix with |
|
I've taken the fix to skip mode 7 for opaque blocks in this commit. If you'd like to clean up the conflicts and update the PR for just the 'faster' mode, I can look at taking those changes. Thanks again! |
|
Thanks Chuck. I've been busy on our universal format, but once I go back to BC7 I can fix up the changelist for faster mode. |
|
Have you had a chance to look at the conflicts? |
|
Any update? |
|
Hi - Sorry, we had to ship our universal codec. It looks like you've already merged all of the key bug fixes. The p-bit thing was the most important. |
When I call DirectX::D3DXEncodeBC7() with DirectX::BC_FLAGS_USE_3SUBSETS, the overall output error increases across the kodim test corpus. This is incorrect: the overall error should decrease. Other BC7 encoders don't behave in this way, so I investigated further. I modified D3DX_BC7::Refine() to call D3DXDecodeBC7() on the encoded block, computed the actual error, and compared that vs. the error computed by Refine(). They differed, which is incorrect.
The bug is caused by D3DX_BC7::Refine() not correctly computing the block error in a way that factors in the actual coded pbits. It computes the error using the endpoints before the LSB's are slammed to the pbits, causing the encoder to choose blocks that actually increase error. This will wreck quality in pbit modes.
The fix introduces a new helper function D3DX_BC7::FixEndpointPBits(), which Refine() calls after it computes new endpoints. It could be easily improved further for more gains (it doesn't round the component values correctly factoring in the pbits), but it's a massive improvement.
With this fix applied the encoding quality across my test corpus increased from 44.25 to 46.02 with 3 subsets enabled, and 45.64 with 3 subsets disabled. Tested with opaque and transparent textures.