Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add FP16 NaN/infinity handling flags #100

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion DirectXTex/DirectXTex.h
Original file line number Diff line number Diff line change
Expand Up @@ -572,11 +572,15 @@ namespace DirectX
TEX_FILTER_FLOAT_X2BIAS = 0x200,
// Enable *2 - 1 conversion cases for unorm<->float and positive-only float formats

TEX_FILTER_FLOAT16_SATURATE_TO_INF = 0x400,
TEX_FILTER_FLOAT16_KEEP_NANS = 0x800,
// Float to half16 conversion options

TEX_FILTER_RGB_COPY_RED = 0x1000,
TEX_FILTER_RGB_COPY_GREEN = 0x2000,
TEX_FILTER_RGB_COPY_BLUE = 0x4000,
// When converting RGB to R, defaults to using grayscale. These flags indicate copying a specific channel instead
// When converting RGB to RG, defaults to copying RED | GREEN. These flags control which channels are selected instead.
// When converting RGB to RG, defaults to copying RED | GREEN. These flags control which channels are selected instead

TEX_FILTER_DITHER = 0x10000,
// Use ordered 4x4 dithering for any required conversions
Expand Down
35 changes: 29 additions & 6 deletions DirectXTex/DirectXTexConvert.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1659,9 +1659,7 @@ bool DirectX::Internal::StoreScanline(
for (size_t icount = 0; icount < (size - sizeof(XMHALF4) + 1); icount += sizeof(XMHALF4))
{
if (sPtr >= ePtr) break;
XMVECTOR v = *sPtr++;
v = XMVectorClamp(v, g_HalfMin, g_HalfMax);
XMStoreHalf4(dPtr++, v);
XMStoreHalf4(dPtr++, *sPtr++);
Copy link
Member

@walbourn walbourn Mar 19, 2023

Choose a reason for hiding this comment

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

I'm a little nervous about removing the clamp behavior here, but it's probably the best choice if we can do it in a way that doesn't regress behavior.

The main places where the clamping is now 'missing' is:

  • BC Decompression - this shouldn't ever have specials
  • TransformImage - The pixel function used needs to clamp if they care
  • ConvertToRGBA32 helper - not using float16 formats as the target
  • Custom resize, Custom mip generation, CopyRectange, NormalMap generation, PM Alpha conversion - If the input has specials, the output will have them.

I think the ConvertFromR32G32B32A32 helpers needs an explicit clamp added for the float16 cases. This is the primarily used for WIC interactions where WIC doesn't support the format.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

* BC Decompression - this shouldn't ever have specials

BC6H signed can have -INF, even though it's not recommended.

}
return true;
}
Expand Down Expand Up @@ -1852,9 +1850,7 @@ bool DirectX::Internal::StoreScanline(
for (size_t icount = 0; icount < (size - sizeof(HALF) + 1); icount += sizeof(HALF))
{
if (sPtr >= ePtr) break;
float v = XMVectorGetX(*sPtr++);
v = std::max<float>(std::min<float>(v, 65504.f), -65504.f);
*(dPtr++) = XMConvertFloatToHalf(v);
*(dPtr++) = XMConvertFloatToHalf(XMVectorGetX(*sPtr++));
}
return true;
}
Expand Down Expand Up @@ -3723,6 +3719,27 @@ void DirectX::Internal::ConvertScanline(
}
}
}

// Half-float sanitization
if (((out->flags & (CONVF_FLOAT | CONVF_DEPTH)) == CONVF_FLOAT)
&& (out->datasize == 16)
&& ((flags & (TEX_FILTER_FLOAT16_SATURATE_TO_INF | TEX_FILTER_FLOAT16_KEEP_NANS)) != (TEX_FILTER_FLOAT16_SATURATE_TO_INF | TEX_FILTER_FLOAT16_KEEP_NANS)))
{
const XMVECTOR zero = XMVectorZero();
XMVECTOR* ptr = pBuffer;
for (size_t i = 0; i < count; ++i, ++ptr)
{
XMVECTOR v = *ptr;

if (!(flags & TEX_FILTER_FLOAT16_SATURATE_TO_INF))
v = XMVectorClamp(v, g_HalfMin, g_HalfMax);

if (!(flags & TEX_FILTER_FLOAT16_KEEP_NANS))
v = XMVectorSelect(v, zero, XMVectorIsNaN(v));

*ptr = v;
}
}
}


Expand Down Expand Up @@ -4441,6 +4458,12 @@ namespace
return false;
}

if (filter & (TEX_FILTER_FLOAT16_SATURATE_TO_INF | TEX_FILTER_FLOAT16_KEEP_NANS))
Copy link
Member

Choose a reason for hiding this comment

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

Before I can sign off here, I really need to see what WIC does. Does it preserve specials or always clamp them?

{
// Float16 specials preservation not supported by WIC code paths
return false;
}

// Check for special cases
#if (defined(_XBOX_ONE) && defined(_TITLE)) || defined(_GAMING_XBOX)
if (sformat == DXGI_FORMAT_R16G16B16A16_FLOAT
Expand Down