Encoding BC OptimizeAlpha bug

[GregSlazinski]

Hello,

https://github.com/microsoft/DirectXTex/blob/master/DirectXTex/BC.h#L252

Codes

iStep = ((6 == cSteps) && (pPoints[iPoint] <= fX * 0.5f)) ? 6u : 0u;
iStep = ((6 == cSteps) && (pPoints[iPoint] >= (fY + 1.0f) * 0.5f)) ? 7u : (cSteps - 1);

should be replaced with:

iStep = ((6 == cSteps) && (pPoints[iPoint] <= (fX + MIN_VALUE) * 0.5f)) ? 6u : 0u;
iStep = ((6 == cSteps) && (pPoints[iPoint] >= (fY + MAX_VALUE) * 0.5f)) ? 7u : (cSteps - 1);

Because it should take into account when MIN_VALUE!=0 for "bool bRange"

We're calculating if the value is closer to MIN_VALUE or the first end-point, so the compared value should be the average between MIN_VALUE and fX.

Also those values can be actually precomputed before the loop starts.

My codes:

      Flt low, high;
      if(steps==6)
      {
         pSteps[6]=MIN_VALUE;
         pSteps[7]=MAX_VALUE;

          low=Avg(fX, MIN_VALUE);
         high=Avg(fY, MAX_VALUE);
      }else
      { // never pass
          low=-FLT_MAX;
         high= FLT_MAX;
      }

      // Evaluate function, and derivatives
      Flt dX=0, dY=0, d2X=0, d2Y=0;
      FREP(16)
      {
         Flt value=pPoints[i],
             fDot =(value-fX)*fScale;
         Int step;
         if(fDot<=     0)step=(value<=low ) ? 6 :       0;else
         if(fDot>=fSteps)step=(value>=high) ? 7 : steps-1;else
                         step=RoundPos(fDot);

[walbourn]

Thanks. That bug dates back to the D3DX10/D3DX11 implementation:

if(fDot <= 0.0f)
    iStep = ((6 == cSteps) && (pPoints[iPoint] <= fX * 0.5f)) ? 6 : 0;
else if(fDot >= fSteps)
    iStep = ((6 == cSteps) && (pPoints[iPoint] >= (fY + 1.0f) * 0.5f)) ? 7 : (cSteps - 1);
else
    iStep = D3D10F2I(fDot + 0.5f);

Note this only matters for BC4_SNORM and BC5_SNORM.

[walbourn]

In trying to validate this change, comparing the old version vs. new version results show only a minor difference in PSNR--in some case better, in others worse. That's probably to be expected for most image test corpi which are all UNORM.

Looks like I need to generate some floating-point images with real sign values in it to see if it fixed a 'real' case.

[GregSlazinski]

This affects only SNORM images, for UNORM the results should be exactly the same. You can use your existing images for testing SNORM just convert them to float in memory after loading: Color value = color value * 2 - 1 To convert from 0..1 to - 1..1 range

[walbourn]

Fair enough. Using the -x2bias on my PNG tests does indeed show most cases are better with this fix. It's all of course very subtle. No wonder the bug went unnoticed for a long time...

[walbourn]

Fixed in this commit