Skip to content

pbit related quality fix for BC7 texture encoding - #93

Closed
Rich Geldreich (richgel999) wants to merge 4 commits into
microsoft:masterfrom
richgel999:master
Closed

Rich Geldreich (richgel999) wants to merge 4 commits into
microsoft:masterfrom
richgel999:master

Conversation

@richgel999

Copy link
Copy Markdown

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.

…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
@msftclas

Microsoft Contribution License Agreements (msftclas) commented Apr 24, 2018 •

Copy link
Copy Markdown

CLA assistant check
All CLA requirements met.

@walbourn

Copy link
Copy Markdown
Collaborator

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.

@richgel999

Rich Geldreich (richgel999) commented Apr 24, 2018 •

Copy link
Copy Markdown
Author

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.

                                                CPU Time     RGB PSNR       
No flags:
Orig/unmodified:                                12505.338093 45.638377
After my changes, remark out m7 for opaque      12429.388720 45.638377
After my changes, skip m7 for opaque:           9755.917509  45.638270

BC_FLAGS_FASTER_BC7:
Only refine a single partition:                 3358.698415  45.360886
Estimated partition selection:                  2884.865429  45.037020
Disable less useful modes for opaque:           2821.762252  45.017103
Same partition for 1,3, less refinement:        1189.376734  44.648705

BC_FLAGS_FORCE_BC7_MODE6:
BC_FLAGS_FORCE_BC7_MODE6                        471.434405   41.863792
BC_FLAGS_FORCE_BC7_MODE6|BC_FLAGS_FASTER_BC7    188.634432   41.070086

…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.
@richgel999

Copy link
Copy Markdown
Author

I've checked in the code for the new flag:
d3f1d3c

I've tested with both opaque and complex transparent textures.

@elasota

Copy link
Copy Markdown
Contributor

Should probably fix the indentation formatting. The existing code uses spaces, but these changes use hard tabs.

@richgel999

Copy link
Copy Markdown
Author

Ok - I'll fix this within a day or two.

@walbourn

Copy link
Copy Markdown
Collaborator

Sorry I thought this was the pull request for that issue. If not, please submit it as a distinct PR. Thanks!

@richgel999

Copy link
Copy Markdown
Author

BTW - ImageMagick can compare images and output PSNR:
http://www.imagemagick.org/Usage/compare/#compare

I like using tools like this because it's a good independent comparison point. My PSNR's are compatible with this tool.

@walbourn

Chuck Walbourn (walbourn) commented Jul 4, 2018 •

Copy link
Copy Markdown
Collaborator

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...

@walbourn

Copy link
Copy Markdown
Collaborator

I've taken the fix with FixEndpointPBits in this commit, so this PR now fails due to conflicts...

@walbourn

Copy link
Copy Markdown
Collaborator

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!

@richgel999

Copy link
Copy Markdown
Author

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.

@walbourn

Copy link
Copy Markdown
Collaborator

Have you had a chance to look at the conflicts?

@walbourn

Copy link
Copy Markdown
Collaborator

Any update?

@richgel999

Copy link
Copy Markdown
Author

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.
Best regards,
-Rich

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bc Related to DirectX Block Compression bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants