From 991b325f10e0083f577cfe0136b06ee053f9e3ed Mon Sep 17 00:00:00 2001 From: walbourn_cp Date: Thu, 28 Mar 2013 00:14:06 -0700 Subject: [PATCH] Code review feedback --- DDSTextureLoader/DDSTextureLoader.cpp | 23 +++++++++++++---------- DDSTextureLoader/DDSTextureLoader.h | 19 ++++++++++--------- DirectXTex/BC.h | 2 +- DirectXTex/DirectXTexCompress.cpp | 2 +- DirectXTex/DirectXTexD3D11.cpp | 1 - DirectXTex/DirectXTexDDS.cpp | 8 ++++++-- DirectXTex/DirectXTexTGA.cpp | 1 - Texconv/texconv.cpp | 4 ++++ 8 files changed, 35 insertions(+), 25 deletions(-) diff --git a/DDSTextureLoader/DDSTextureLoader.cpp b/DDSTextureLoader/DDSTextureLoader.cpp index 5bd454e..37f66a0 100644 --- a/DDSTextureLoader/DDSTextureLoader.cpp +++ b/DDSTextureLoader/DDSTextureLoader.cpp @@ -1,7 +1,7 @@ //-------------------------------------------------------------------------------------- // File: DDSTextureLoader.cpp // -// Function for loading a DDS texture and creating a Direct3D 11 runtime resource for it +// Functions for loading a DDS texture and creating a Direct3D 11 runtime resource for it // // Note these functions are useful as a light-weight runtime loader for DDS files. For // a full-featured DDS file reader, writer, and texture processing pipeline see @@ -215,7 +215,7 @@ static HRESULT LoadTextureDataFromFile( _In_z_ const wchar_t* fileName, // create enough space for the file data ddsData.reset( new (std::nothrow) uint8_t[ FileSize.LowPart ] ); - if (!ddsData ) + if (!ddsData) { return E_OUTOFMEMORY; } @@ -720,7 +720,6 @@ static DXGI_FORMAT GetDXGIFormat( const DDS_PIXELFORMAT& ddpf ) } - //-------------------------------------------------------------------------------------- static DXGI_FORMAT MakeSRGB( _In_ DXGI_FORMAT format ) { @@ -865,8 +864,8 @@ static HRESULT CreateD3DResources( _In_ ID3D11Device* d3dDevice, _In_ bool forceSRGB, _In_ bool isCubeMap, _In_reads_(mipCount*arraySize) D3D11_SUBRESOURCE_DATA* initData, - _Out_opt_ ID3D11Resource** texture, - _Out_opt_ ID3D11ShaderResourceView** textureView ) + _Outptr_opt_ ID3D11Resource** texture, + _Outptr_opt_ ID3D11ShaderResourceView** textureView ) { if ( !d3dDevice || !initData ) return E_POINTER; @@ -955,9 +954,13 @@ static HRESULT CreateD3DResources( _In_ ID3D11Device* d3dDevice, desc.BindFlags = bindFlags; desc.CPUAccessFlags = cpuAccessFlags; if ( isCubeMap ) + { desc.MiscFlags = miscFlags | D3D11_RESOURCE_MISC_TEXTURECUBE; + } else + { desc.MiscFlags = miscFlags & ~D3D11_RESOURCE_MISC_TEXTURECUBE; + } ID3D11Texture2D* tex = nullptr; hr = d3dDevice->CreateTexture2D( &desc, @@ -972,7 +975,7 @@ static HRESULT CreateD3DResources( _In_ ID3D11Device* d3dDevice, memset( &SRVDesc, 0, sizeof( SRVDesc ) ); SRVDesc.Format = format; - if (isCubeMap) + if ( isCubeMap ) { if (arraySize > 6) { @@ -1093,8 +1096,8 @@ static HRESULT CreateTextureFromDDS( _In_ ID3D11Device* d3dDevice, _In_ unsigned int cpuAccessFlags, _In_ unsigned int miscFlags, _In_ bool forceSRGB, - _Out_opt_ ID3D11Resource** texture, - _Out_opt_ ID3D11ShaderResourceView** textureView ) + _Outptr_opt_ ID3D11Resource** texture, + _Outptr_opt_ ID3D11ShaderResourceView** textureView ) { HRESULT hr = S_OK; @@ -1222,7 +1225,7 @@ static HRESULT CreateTextureFromDDS( _In_ ID3D11Device* d3dDevice, break; case D3D11_RESOURCE_DIMENSION_TEXTURE2D: - if (isCubeMap) + if ( isCubeMap ) { // This is the right bound because we set arraySize to (NumCubes*6) above if ((arraySize > D3D11_REQ_TEXTURE2D_ARRAY_AXIS_DIMENSION) || @@ -1278,7 +1281,7 @@ static HRESULT CreateTextureFromDDS( _In_ ID3D11Device* d3dDevice, { case D3D_FEATURE_LEVEL_9_1: case D3D_FEATURE_LEVEL_9_2: - if (isCubeMap) + if ( isCubeMap ) { maxsize = 512 /*D3D_FL9_1_REQ_TEXTURECUBE_DIMENSION*/; } diff --git a/DDSTextureLoader/DDSTextureLoader.h b/DDSTextureLoader/DDSTextureLoader.h index 729e22e..e88013b 100644 --- a/DDSTextureLoader/DDSTextureLoader.h +++ b/DDSTextureLoader/DDSTextureLoader.h @@ -1,7 +1,7 @@ //-------------------------------------------------------------------------------------- // File: DDSTextureLoader.h // -// Function for loading a DDS texture and creating a Direct3D 11 runtime resource for it +// Functions for loading a DDS texture and creating a Direct3D 11 runtime resource for it // // Note these functions are useful as a light-weight runtime loader for DDS files. For // a full-featured DDS file reader, writer, and texture processing pipeline see @@ -33,6 +33,7 @@ #define _In_reads_(exp) #define _Out_writes_(exp) #define _In_reads_bytes_(exp) +#define _Outptr_opt_ #endif #ifndef _Use_decl_annotations_ @@ -52,16 +53,16 @@ namespace DirectX HRESULT CreateDDSTextureFromMemory( _In_ ID3D11Device* d3dDevice, _In_reads_bytes_(ddsDataSize) const uint8_t* ddsData, _In_ size_t ddsDataSize, - _Out_opt_ ID3D11Resource** texture, - _Out_opt_ ID3D11ShaderResourceView** textureView, + _Outptr_opt_ ID3D11Resource** texture, + _Outptr_opt_ ID3D11ShaderResourceView** textureView, _In_ size_t maxsize = 0, _Out_opt_ DDS_ALPHA_MODE* alphaMode = nullptr ); HRESULT CreateDDSTextureFromFile( _In_ ID3D11Device* d3dDevice, _In_z_ const wchar_t* szFileName, - _Out_opt_ ID3D11Resource** texture, - _Out_opt_ ID3D11ShaderResourceView** textureView, + _Outptr_opt_ ID3D11Resource** texture, + _Outptr_opt_ ID3D11ShaderResourceView** textureView, _In_ size_t maxsize = 0, _Out_opt_ DDS_ALPHA_MODE* alphaMode = nullptr ); @@ -75,8 +76,8 @@ namespace DirectX _In_ unsigned int cpuAccessFlags, _In_ unsigned int miscFlags, _In_ bool forceSRGB, - _Out_opt_ ID3D11Resource** texture, - _Out_opt_ ID3D11ShaderResourceView** textureView, + _Outptr_opt_ ID3D11Resource** texture, + _Outptr_opt_ ID3D11ShaderResourceView** textureView, _Out_opt_ DDS_ALPHA_MODE* alphaMode = nullptr ); @@ -88,8 +89,8 @@ namespace DirectX _In_ unsigned int cpuAccessFlags, _In_ unsigned int miscFlags, _In_ bool forceSRGB, - _Out_opt_ ID3D11Resource** texture, - _Out_opt_ ID3D11ShaderResourceView** textureView, + _Outptr_opt_ ID3D11Resource** texture, + _Outptr_opt_ ID3D11ShaderResourceView** textureView, _Out_opt_ DDS_ALPHA_MODE* alphaMode = nullptr ); } \ No newline at end of file diff --git a/DirectXTex/BC.h b/DirectXTex/BC.h index 3d30244..3574bf1 100644 --- a/DirectXTex/BC.h +++ b/DirectXTex/BC.h @@ -50,7 +50,7 @@ const uint16_t F16S_MASK = 0x8000; // f16 sign mask const uint16_t F16EM_MASK = 0x7fff; // f16 exp & mantissa mask const uint16_t F16MAX = 0x7bff; // MAXFLT bit pattern for XMHALF -#define SIGN_EXTEND(x,nb) ((((x)&(1<<((nb)-1)))?((~0)<<(nb)):0)|(x)) +#define SIGN_EXTEND(x,nb) ((((x)&(1<<((nb)-1)))?((~0)<<(nb)):0)|(x)) // Because these are used in SAL annotations, they need to remain macros rather than const values #define NUM_PIXELS_PER_BLOCK 16 diff --git a/DirectXTex/DirectXTexCompress.cpp b/DirectXTex/DirectXTexCompress.cpp index d6c202d..9e50378 100644 --- a/DirectXTex/DirectXTexCompress.cpp +++ b/DirectXTex/DirectXTexCompress.cpp @@ -481,7 +481,7 @@ bool _IsAlphaAllOpaqueBC( _In_ const Image& cImage ) XMVECTOR temp[16]; const uint8_t *pPixels = cImage.pixels; - for( size_t h=0; h < cImage.height; h += 4 ) + for( size_t h = 0; h < cImage.height; h += 4 ) { const uint8_t *ptr = pPixels; for( size_t count = 0; count < cImage.rowPitch; count += sbpp ) diff --git a/DirectXTex/DirectXTexD3D11.cpp b/DirectXTex/DirectXTexD3D11.cpp index 166fd27..33fb777 100644 --- a/DirectXTex/DirectXTexD3D11.cpp +++ b/DirectXTex/DirectXTexD3D11.cpp @@ -746,7 +746,6 @@ HRESULT CaptureTexture( ID3D11Device* pDevice, ID3D11DeviceContext* pContext, ID for( UINT level = 0; level < desc.MipLevels; ++level ) { UINT index = D3D11CalcSubresource( level, item, desc.MipLevels ); - pContext->ResolveSubresource( pTemp.Get(), index, pSource, index, fmt ); } } diff --git a/DirectXTex/DirectXTexDDS.cpp b/DirectXTex/DirectXTexDDS.cpp index 5790577..aa0adc9 100644 --- a/DirectXTex/DirectXTexDDS.cpp +++ b/DirectXTex/DirectXTexDDS.cpp @@ -258,7 +258,7 @@ static HRESULT _DecodeDDSHeader( _In_reads_bytes_(size) LPCVOID pSource, size_t metadata.format = d3d10ext->dxgiFormat; if ( !IsValid( metadata.format ) ) { - HRESULT_FROM_WIN32( ERROR_INVALID_DATA ); + return HRESULT_FROM_WIN32( ERROR_INVALID_DATA ); } static_assert( TEX_MISC_TEXTURECUBE == DDS_RESOURCE_MISC_TEXTURECUBE, "DDS header mismatch"); @@ -467,7 +467,6 @@ HRESULT _EncodeDDSHeader( const TexMetadata& metadata, DWORD flags, case DXGI_FORMAT_B5G5R5A1_UNORM: memcpy_s( &ddpf, sizeof(ddpf), &DDSPF_A1R5G5B5, sizeof(DDS_PIXELFORMAT) ); break; case DXGI_FORMAT_B8G8R8A8_UNORM: memcpy_s( &ddpf, sizeof(ddpf), &DDSPF_A8R8G8B8, sizeof(DDS_PIXELFORMAT) ); break; // DXGI 1.1 case DXGI_FORMAT_B8G8R8X8_UNORM: memcpy_s( &ddpf, sizeof(ddpf), &DDSPF_X8R8G8B8, sizeof(DDS_PIXELFORMAT) ); break; // DXGI 1.1 - #ifdef DXGI_1_2_FORMATS case DXGI_FORMAT_B4G4R4A4_UNORM: memcpy_s( &ddpf, sizeof(ddpf), &DDSPF_A4R4G4B4, sizeof(DDS_PIXELFORMAT) ); break; #endif @@ -941,6 +940,11 @@ static HRESULT _CopyImage( _In_reads_bytes_(size) const void* pPixels, _In_ size assert( pixelSize <= size ); std::unique_ptr timages( new (std::nothrow) Image[nimages] ); + if ( !timages ) + { + return E_OUTOFMEMORY; + } + if ( !_SetupImageArray( (uint8_t*)pPixels, size, metadata, cpFlags, timages.get(), nimages ) ) { return E_FAIL; diff --git a/DirectXTex/DirectXTexTGA.cpp b/DirectXTex/DirectXTexTGA.cpp index f253e0d..4da2d20 100644 --- a/DirectXTex/DirectXTexTGA.cpp +++ b/DirectXTex/DirectXTexTGA.cpp @@ -1109,7 +1109,6 @@ HRESULT LoadFromTGAFile( LPCWSTR szFile, TexMetadata* metadata, ScratchImage& im for( size_t h = 0; h < img->height; ++h ) { _SwizzleScanline( pPixels, rowPitch, pPixels, rowPitch, mdata.format, tflags ); - pPixels += rowPitch; } } diff --git a/Texconv/texconv.cpp b/Texconv/texconv.cpp index f9d2ef9..c45ec63 100644 --- a/Texconv/texconv.cpp +++ b/Texconv/texconv.cpp @@ -212,6 +212,8 @@ inline static bool ispow2(size_t x) return ((x != 0) && !(x & (x - 1))); } +#pragma prefast(disable : 26018, "Only used with static internal arrays") + DWORD LookupByName(const WCHAR *pName, const SValue *pArray) { while(pArray->pName) @@ -361,6 +363,8 @@ void PrintUsage() //-------------------------------------------------------------------------------------- // Entry-point //-------------------------------------------------------------------------------------- +#pragma prefast(disable : 28198, "Command-line tool, frees all memory on exit") + int __cdecl wmain(_In_ int argc, _In_z_count_(argc) wchar_t* argv[]) { // Parameters and defaults