microsoft / DirectXTK

The DirectX Tool Kit (aka DirectXTK) is a collection of helper classes for writing DirectX 11.x code in C++
https://walbourn.github.io/directxtk/
MIT License
2.57k stars 511 forks source link

CMO multi-stream data missing pointer arithmetic typo? #121

Closed alessiot89 closed 6 years ago

alessiot89 commented 6 years ago

File: ModelLoadCMO.cpp Lines: 681-691

for( size_t v = 0; v < nVerts; ++v )
{
    *reinterpret_cast<VertexPositionNormalTangentColorTexture*>( ptr ) = *sptr;
    ++sptr;

    auto skinv = reinterpret_cast<VertexPositionNormalTangentColorTextureSkinning*>( ptr );
    skinv->SetBlendIndices( *reinterpret_cast<const XMUINT4*>( skinptr->boneIndex ) );
    skinv->SetBlendWeights( *reinterpret_cast<const XMFLOAT4*>( skinptr->boneWeight ) );

    ptr += stride;
}

Shouldn't the SkinningVertex pointer (skinptr) advance as well when combining non-skinning and skinning vertex data? Like:

for( size_t v = 0; v < nVerts; ++v )
{
    *reinterpret_cast<VertexPositionNormalTangentColorTexture*>( ptr ) = *sptr;
    ++sptr;

    auto skinv = reinterpret_cast<VertexPositionNormalTangentColorTextureSkinning*>( ptr );
    skinv->SetBlendIndices( *reinterpret_cast<const XMUINT4*>( skinptr->boneIndex ) );
    skinv->SetBlendWeights( *reinterpret_cast<const XMFLOAT4*>( skinptr->boneWeight ) );

    ++skinptr
    ptr += stride;
}

Or simply using the loop variable as array index to remember they both have the same number of elements?

for( size_t v = 0; v < nVerts; ++v )
{
    *reinterpret_cast<VertexPositionNormalTangentColorTexture*>( ptr ) = sptr[v];

    auto skinv = reinterpret_cast<VertexPositionNormalTangentColorTextureSkinning*>( ptr );
    skinv->SetBlendIndices( *reinterpret_cast<const XMUINT4*>( skinptr[v].boneIndex ) );
    skinv->SetBlendWeights( *reinterpret_cast<const XMFLOAT4*>( skinptr[v].boneWeight ) );

    ptr += stride;
}
walbourn commented 6 years ago

Thanks for the report. Fixed in this commit for the next release of DirectX Tool Kit.

Note that the DirectX 12 version doesn't support CMO so this bug only impacts the DirectX 11 version.

dominichu93 commented 6 years ago

Unsubscribe ᐧ

On Mon, Feb 12, 2018 at 6:25 PM, Chuck Walbourn notifications@github.com wrote:

Thanks for the report. Fixed in this commit https://github.com/Microsoft/DirectXTK/commits/master for the next release of DirectX Tool Kit.

Note that the DirectX 12 version doesn't support CMO so this bug only impacts the DirectX 11 version.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Microsoft/DirectXTK/issues/121#issuecomment-365014963, or mute the thread https://github.com/notifications/unsubscribe-auth/AgkvvQno_7g3m0PcAMw6JR8NXmpOkujsks5tUIH9gaJpZM4R_Q7r .