Code review feedback

This commit is contained in:
Chuck Walbourn 2016-10-27 17:45:47 -07:00
parent 3950756b53
commit f45b7fb93b
10 changed files with 45 additions and 29 deletions

View File

@ -719,7 +719,7 @@ namespace
INTColor aIPixels[NUM_PIXELS_PER_BLOCK];
EncodeParams(const HDRColorA* const aOriginal, bool bSignedFormat) :
aHDRPixels(aOriginal), fBestErr(FLT_MAX), bSigned(bSignedFormat)
fBestErr(FLT_MAX), bSigned(bSignedFormat), aHDRPixels(aOriginal)
{
for (size_t i = 0; i < NUM_PIXELS_PER_BLOCK; ++i)
{

View File

@ -246,9 +246,9 @@ namespace DirectX
{
public:
ScratchImage()
: m_nimages(0), m_size(0), m_image(nullptr), m_memory(nullptr) {}
: m_nimages(0), m_size(0), m_metadata{}, m_image(nullptr), m_memory(nullptr) {}
ScratchImage(ScratchImage&& moveFrom)
: m_nimages(0), m_size(0), m_image(nullptr), m_memory(nullptr) { *this = std::move(moveFrom); }
: m_nimages(0), m_size(0), m_metadata{}, m_image(nullptr), m_memory(nullptr) { *this = std::move(moveFrom); }
~ScratchImage() { Release(); }
ScratchImage& __cdecl operator= (ScratchImage&& moveFrom);

View File

@ -802,7 +802,7 @@ HRESULT DirectX::Decompress(
return E_INVALIDARG;
if (IsTypeless(format) || IsPlanar(format) || IsPalettized(format))
HRESULT_FROM_WIN32(ERROR_NOT_SUPPORTED);
return HRESULT_FROM_WIN32(ERROR_NOT_SUPPORTED);
}
images.Release();

View File

@ -3673,6 +3673,7 @@ namespace
XMStoreFloat4A( &tmp, target ); \
\
auto dPtr = &dest[ index ]; \
if (dPtr >= ePtr) break; \
dPtr->x = static_cast<itype>( tmp.x ) & mask; \
dPtr->y = static_cast<itype>( tmp.y ) & mask; \
dPtr->z = static_cast<itype>( tmp.z ) & mask; \
@ -3727,6 +3728,7 @@ namespace
XMStoreFloat4A( &tmp, target ); \
\
auto dPtr = &dest[ index ]; \
if (dPtr >= ePtr) break; \
dPtr->x = static_cast<itype>( tmp.x ) & mask; \
dPtr->y = static_cast<itype>( tmp.y ) & mask; \
} \
@ -3775,7 +3777,9 @@ namespace
target = XMVectorMin( scalev, target ); \
target = XMVectorMax( (clampzero) ? g_XMZero : ( -scalev + g_XMOne ), target ); \
\
dest[ index ] = static_cast<type>( (selectw) ? XMVectorGetW( target ) : XMVectorGetX( target ) ) & mask; \
auto dPtr = &dest[ index ]; \
if (dPtr >= ePtr) break; \
*dPtr = static_cast<type>( (selectw) ? XMVectorGetW( target ) : XMVectorGetX( target ) ) & mask; \
} \
return true; \
} \
@ -3837,6 +3841,8 @@ bool DirectX::_StoreScanlineDither(
if (!sPtr)
return false;
const void* ePtr = reinterpret_cast<const uint8_t*>(pDestination) + size;
XMVECTOR vError = XMVectorZero();
switch (static_cast<int>(format))
@ -3903,6 +3909,7 @@ bool DirectX::_StoreScanlineDither(
XMStoreFloat4A(&tmp, target);
auto dPtr = &dest[index];
if (dPtr >= ePtr) break;
dPtr->x = static_cast<uint16_t>(tmp.x) & 0x3FF;
dPtr->y = static_cast<uint16_t>(tmp.y) & 0x3FF;
dPtr->z = static_cast<uint16_t>(tmp.z) & 0x3FF;
@ -3980,6 +3987,7 @@ bool DirectX::_StoreScanlineDither(
XMStoreFloat4A(&tmp, target);
auto dPtr = &dest[index];
if (dPtr >= ePtr) break;
*dPtr = (static_cast<uint32_t>(tmp.x) & 0xFFFFFF)
| ((static_cast<uint32_t>(tmp.y) & 0xFF) << 24);
}
@ -4067,6 +4075,7 @@ bool DirectX::_StoreScanlineDither(
XMStoreFloat4A(&tmp, target);
auto dPtr = &dest[index];
if (dPtr >= ePtr) break;
dPtr->x = static_cast<uint16_t>(tmp.x) & 0x1F;
dPtr->y = static_cast<uint16_t>(tmp.y) & 0x3F;
dPtr->z = static_cast<uint16_t>(tmp.z) & 0x1F;
@ -4115,6 +4124,7 @@ bool DirectX::_StoreScanlineDither(
XMStoreFloat4A(&tmp, target);
auto dPtr = &dest[index];
if (dPtr >= ePtr) break;
dPtr->x = static_cast<uint16_t>(tmp.x) & 0x1F;
dPtr->y = static_cast<uint16_t>(tmp.y) & 0x1F;
dPtr->z = static_cast<uint16_t>(tmp.z) & 0x1F;
@ -4169,6 +4179,7 @@ bool DirectX::_StoreScanlineDither(
XMStoreFloat4A(&tmp, target);
auto dPtr = &dest[index];
if (dPtr >= ePtr) break;
dPtr->x = static_cast<uint8_t>(tmp.x) & 0xFF;
dPtr->y = static_cast<uint8_t>(tmp.y) & 0xFF;
dPtr->z = static_cast<uint8_t>(tmp.z) & 0xFF;
@ -4222,7 +4233,9 @@ bool DirectX::_StoreScanlineDither(
XMFLOAT4A tmp;
XMStoreFloat4A(&tmp, target);
dest[index] = (static_cast<uint8_t>(tmp.x) & 0xF)
auto dPtr = &dest[index];
if (dPtr >= ePtr) break;
*dPtr = (static_cast<uint8_t>(tmp.x) & 0xF)
| ((static_cast<uint8_t>(tmp.y) & 0xF) << 4);
}
return true;

View File

@ -1786,7 +1786,7 @@ HRESULT DirectX::SaveToDDSMemory(
size_t rowPitch = images[index].rowPitch;
const uint8_t * __restrict sPtr = reinterpret_cast<const uint8_t*>(images[index].pixels);
const uint8_t * __restrict sPtr = images[index].pixels;
uint8_t * __restrict dPtr = reinterpret_cast<uint8_t*>(pDestination);
size_t lines = ComputeScanlines(metadata.format, images[index].height);
@ -1855,7 +1855,7 @@ HRESULT DirectX::SaveToDDSMemory(
size_t rowPitch = images[index].rowPitch;
const uint8_t * __restrict sPtr = reinterpret_cast<const uint8_t*>(images[index].pixels);
const uint8_t * __restrict sPtr = images[index].pixels;
uint8_t * __restrict dPtr = reinterpret_cast<uint8_t*>(pDestination);
size_t lines = ComputeScanlines(metadata.format, images[index].height);
@ -1986,7 +1986,7 @@ HRESULT DirectX::SaveToDDSFile(
return E_FAIL;
}
const uint8_t * __restrict sPtr = reinterpret_cast<const uint8_t*>(images[index].pixels);
const uint8_t * __restrict sPtr = images[index].pixels;
size_t lines = ComputeScanlines(metadata.format, images[index].height);
for (size_t j = 0; j < lines; ++j)
@ -2054,7 +2054,7 @@ HRESULT DirectX::SaveToDDSFile(
return E_FAIL;
}
const uint8_t * __restrict sPtr = reinterpret_cast<const uint8_t*>(images[index].pixels);
const uint8_t * __restrict sPtr = images[index].pixels;
size_t lines = ComputeScanlines(metadata.format, images[index].height);
for (size_t j = 0; j < lines; ++j)

View File

@ -287,14 +287,17 @@ namespace
//-------------------------------------------------------------------------------------
// FloatToRGBE
//-------------------------------------------------------------------------------------
inline void FloatToRGBE(_Out_writes_(width*4) uint8_t* pDestination, _In_reads_(width*bpp) const float* pSource, size_t width, int bpp)
inline void FloatToRGBE(_Out_writes_(width*4) uint8_t* pDestination, _In_reads_(width*fpp) const float* pSource, size_t width, _In_range_(3, 4) int fpp)
{
auto ePtr = pSource + width * fpp;
for (size_t j = 0; j < width; ++j)
{
if (pSource + 2 >= ePtr) break;
float r = pSource[0] >= 0.f ? pSource[0] : 0.f;
float g = pSource[1] >= 0.f ? pSource[1] : 0.f;
float b = pSource[2] >= 0.f ? pSource[2] : 0.f;
pSource += bpp;
pSource += fpp;
const float max_xy = (r > g) ? r : g;
float max_xyz = (max_xy > b) ? max_xy : b;
@ -872,15 +875,15 @@ HRESULT DirectX::SaveToHDRMemory(const Image& image, Blob& blob)
return HRESULT_FROM_WIN32(ERROR_NOT_SUPPORTED);
}
int bpp;
int fpp;
switch (image.format)
{
case DXGI_FORMAT_R32G32B32A32_FLOAT:
bpp = 4;
fpp = 4;
break;
case DXGI_FORMAT_R32G32B32_FLOAT:
bpp = 3;
fpp = 3;
break;
default:
@ -912,7 +915,7 @@ HRESULT DirectX::SaveToHDRMemory(const Image& image, Blob& blob)
auto sPtr = reinterpret_cast<const uint8_t*>(image.pixels);
for (size_t scan = 0; scan < image.height; ++scan)
{
FloatToRGBE(dPtr, reinterpret_cast<const float*>(sPtr), image.width, bpp);
FloatToRGBE(dPtr, reinterpret_cast<const float*>(sPtr), image.width, fpp);
dPtr += rowPitch;
sPtr += image.rowPitch;
}
@ -930,7 +933,7 @@ HRESULT DirectX::SaveToHDRMemory(const Image& image, Blob& blob)
auto sPtr = reinterpret_cast<const uint8_t*>(image.pixels);
for (size_t scan = 0; scan < image.height; ++scan)
{
FloatToRGBE(rgbe, reinterpret_cast<const float*>(sPtr), image.width, bpp);
FloatToRGBE(rgbe, reinterpret_cast<const float*>(sPtr), image.width, fpp);
sPtr += image.rowPitch;
size_t encSize = EncodeRLE(enc, rgbe, rowPitch, image.width);
@ -977,15 +980,15 @@ HRESULT DirectX::SaveToHDRFile(const Image& image, const wchar_t* szFile)
return HRESULT_FROM_WIN32(ERROR_NOT_SUPPORTED);
}
int bpp;
int fpp;
switch (image.format)
{
case DXGI_FORMAT_R32G32B32A32_FLOAT:
bpp = 4;
fpp = 4;
break;
case DXGI_FORMAT_R32G32B32_FLOAT:
bpp = 3;
fpp = 3;
break;
default:
@ -1059,7 +1062,7 @@ HRESULT DirectX::SaveToHDRFile(const Image& image, const wchar_t* szFile)
auto sPtr = reinterpret_cast<const uint8_t*>(image.pixels);
for (size_t scan = 0; scan < image.height; ++scan)
{
FloatToRGBE(rgbe, reinterpret_cast<const float*>(sPtr), image.width, bpp);
FloatToRGBE(rgbe, reinterpret_cast<const float*>(sPtr), image.width, fpp);
sPtr += image.rowPitch;
if (!WriteFile(hFile.get(), rgbe, static_cast<DWORD>(rowPitch), &bytesWritten, nullptr))
@ -1076,7 +1079,7 @@ HRESULT DirectX::SaveToHDRFile(const Image& image, const wchar_t* szFile)
auto sPtr = reinterpret_cast<const uint8_t*>(image.pixels);
for (size_t scan = 0; scan < image.height; ++scan)
{
FloatToRGBE(rgbe, reinterpret_cast<const float*>(sPtr), image.width, bpp);
FloatToRGBE(rgbe, reinterpret_cast<const float*>(sPtr), image.width, fpp);
sPtr += image.rowPitch;
size_t encSize = EncodeRLE(enc, rgbe, rowPitch, image.width);

View File

@ -783,7 +783,7 @@ bool ScratchImage::IsAlphaAllOpaque() const
if (!_LoadScanline(scanline.get(), img.width, pPixels, img.rowPitch, img.format))
return false;
XMVECTOR* ptr = scanline.get();
const XMVECTOR* ptr = scanline.get();
for (size_t w = 0; w < img.width; ++w)
{
XMVECTOR alpha = XMVectorSplatW(*ptr);

View File

@ -170,7 +170,7 @@ namespace
//-------------------------------------------------------------------------------------
HRESULT EvaluateImage_(
const Image& image,
std::function<void __cdecl(_In_reads_(width) const XMVECTOR* pixels, size_t width, size_t y)> pixelFunc)
std::function<void __cdecl(_In_reads_(width) const XMVECTOR* pixels, size_t width, size_t y)>& pixelFunc)
{
if (!pixelFunc)
return E_INVALIDARG;
@ -206,7 +206,7 @@ namespace
//-------------------------------------------------------------------------------------
HRESULT TransformImage_(
const Image& srcImage,
std::function<void __cdecl(_Out_writes_(width) XMVECTOR* outPixels, _In_reads_(width) const XMVECTOR* inPixels, size_t width, size_t y)> pixelFunc,
std::function<void __cdecl(_Out_writes_(width) XMVECTOR* outPixels, _In_reads_(width) const XMVECTOR* inPixels, size_t width, size_t y)>& pixelFunc,
const Image& destImage)
{
if (!pixelFunc)

View File

@ -201,15 +201,15 @@ namespace DirectX
_In_reads_bytes_(size) const void* pSource, _In_ size_t size, _In_ DXGI_FORMAT format, _In_ DWORD flags );
_Success_(return != false)
bool __cdecl _StoreScanline( void* pDestination, _In_ size_t size, _In_ DXGI_FORMAT format,
bool __cdecl _StoreScanline( _Out_writes_bytes_(size) void* pDestination, _In_ size_t size, _In_ DXGI_FORMAT format,
_In_reads_(count) const XMVECTOR* pSource, _In_ size_t count, _In_ float threshold = 0 );
_Success_(return != false)
bool __cdecl _StoreScanlineLinear( void* pDestination, _In_ size_t size, _In_ DXGI_FORMAT format,
bool __cdecl _StoreScanlineLinear( _Out_writes_bytes_(size) void* pDestination, _In_ size_t size, _In_ DXGI_FORMAT format,
_Inout_updates_all_(count) XMVECTOR* pSource, _In_ size_t count, _In_ DWORD flags, _In_ float threshold = 0 );
_Success_(return != false)
bool __cdecl _StoreScanlineDither( void* pDestination, _In_ size_t size, _In_ DXGI_FORMAT format,
bool __cdecl _StoreScanlineDither( _Out_writes_bytes_(size) void* pDestination, _In_ size_t size, _In_ DXGI_FORMAT format,
_Inout_updates_all_(count) XMVECTOR* pSource, _In_ size_t count, _In_ float threshold, size_t y, size_t z,
_Inout_updates_all_opt_(count+2) XMVECTOR* pDiffusionErrors );

View File

@ -332,7 +332,7 @@ namespace
}
#endif
PropVariantClear(&value);
(void)PropVariantClear(&value);
if (sRGB)
metadata.format = MakeSRGB(metadata.format);