Skip to content

fix(ww3d2): Prevent undefined behavior when writing dynamic vertices and reading screenshot pixels #3671

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.

Sign up for GitHub

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

Draft
CryoTheRenegade wants to merge 1 commit into
base: main
Choose a base branch
from
+21 −37
Draft

fix(ww3d2): Prevent undefined behavior when writing dynamic vertices and reading screenshot pixels #3671

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 6 additions & 4 deletions Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DScreenshot.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,11 @@ static DWORD WINAPI screenshotThreadFunc(LPVOID param)
// Convert A8R8G8B8/X8R8G8B8 to R8G8B8
for (unsigned int y = 0; y < height; ++y)
{
const unsigned int* srcLine = reinterpret_cast<const unsigned int*>(data->pixelData + y * data->pitch);
const unsigned char* srcLine = data->pixelData + y * data->pitch;
for (unsigned int x = 0; x < width; ++x)
{
const unsigned int argb = srcLine[x];
unsigned int argb;
memcpy(&argb, srcLine + x * sizeof(argb), sizeof(argb));
const unsigned int index = 3 * (x + y * width);
image[index + 0] = (unsigned char)(argb >> 16); // r
image[index + 1] = (unsigned char)(argb >> 8); // g
Expand All @@ -98,10 +99,11 @@ static DWORD WINAPI screenshotThreadFunc(LPVOID param)
// Convert R5G6B5 to R8G8B8
for (unsigned int y = 0; y < height; ++y)
{
const unsigned short* srcLine = reinterpret_cast<const unsigned short*>(data->pixelData + y * data->pitch);
const unsigned char* srcLine = data->pixelData + y * data->pitch;
for (unsigned int x = 0; x < width; ++x)
{
const unsigned short rgb = srcLine[x];
unsigned short rgb;
memcpy(&rgb, srcLine + x * sizeof(rgb), sizeof(rgb));
const unsigned int index = 3 * (x + y * width);
image[index + 0] = (unsigned char)((rgb & 0xF800) >> 8); // r
image[index + 1] = (unsigned char)((rgb & 0x07E0) >> 3); // g
Expand Down
25 changes: 8 additions & 17 deletions Core/Libraries/Source/WWVegas/WW3D2/seglinerenderer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1139,26 +1139,17 @@ void SegLineRendererClass::Render
{
DynamicVBAccessClass::WriteLockClass Lock(&Verts);
unsigned int i;
unsigned char *vb=(unsigned char*)Lock.Get_Formatted_Vertex_Array();
const FVFInfoClass& fvfinfo=Verts.FVF_Info();

const unsigned int verticesOffset = fvfinfo.Get_Location_Offset();
const unsigned diffuseOffset = fvfinfo.Get_Diffuse_Offset();
const unsigned textureOffset = fvfinfo.Get_Tex_Offset(0);
const unsigned vbSize = fvfinfo.Get_FVF_Size();
VertexFormatXYZNDUV2* vb=Lock.Get_Formatted_Vertex_Array();

for (i=0; i<vnum; i++)
{
// Copy Locations
Vector3 *vertex = reinterpret_cast<Vector3 *>(vb + verticesOffset);
vertex->X = vArray[i].x;
vertex->Y = vArray[i].y;
vertex->Z = vArray[i].z;
*reinterpret_cast<unsigned int *>(vb + diffuseOffset) = vArray[i].diffuse;
Vector2 *texture = reinterpret_cast<Vector2 *>(vb + textureOffset);
texture->U = vArray[i].u1;
texture->V = vArray[i].v1;
vb += vbSize;
// Copy the location, color and texture coordinates
vb[i].x = vArray[i].x;
vb[i].y = vArray[i].y;
vb[i].z = vArray[i].z;
vb[i].diffuse = vArray[i].diffuse;
vb[i].u1 = vArray[i].u1;
vb[i].v1 = vArray[i].v1;
}
}

Expand Down
23 changes: 7 additions & 16 deletions Core/Libraries/Source/WWVegas/WW3D2/streakRender.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1329,27 +1329,18 @@ void StreakRendererClass::RenderStreak
{
DynamicVBAccessClass::WriteLockClass Lock(&Verts);
unsigned int i;
unsigned char *vb=(unsigned char*)Lock.Get_Formatted_Vertex_Array();
const FVFInfoClass& fvfinfo=Verts.FVF_Info();

const unsigned verticesOffset = fvfinfo.Get_Location_Offset();
const unsigned diffuseOffset = fvfinfo.Get_Diffuse_Offset();
const unsigned textureOffset = fvfinfo.Get_Tex_Offset(0);
const unsigned vbSize = fvfinfo.Get_FVF_Size();
VertexFormatXYZNDUV2* vb=Lock.Get_Formatted_Vertex_Array();

for (i=0; i<vnum; i++)
{
DEBUG_ASSERTCRASH(vertexArray[i].x != (float)0xdeadbeef && vertexArray[i].y != (float)0xdeadbeef && vertexArray[i].z != (float)0xdeadbeef && vertexArray[i].u1 != (float)0xdeadbeeef && vertexArray[i].v1 != (float)0xdeadbeef, ("Uninitialized vertexArray[%d]", i));
DEBUG_ASSERTCRASH((! _isnan(vertexArray[i].x) && _finite(vertexArray[i].x) && ! _isnan(vertexArray[i].y) && _finite(vertexArray[i].y) && ! _isnan(vertexArray[i].z) && _finite(vertexArray[i].z)) , ("Bad vertexArray[%d]", i));
Vector3 *vertex = reinterpret_cast<Vector3 *>(vb + verticesOffset);
vertex->X = vertexArray[i].x;
vertex->Y = vertexArray[i].y;
vertex->Z = vertexArray[i].z;
*reinterpret_cast<unsigned int *>(vb + diffuseOffset) = DX8Wrapper::Convert_Color_Clamp(colors[MIN((i/2), point_cnt)]); // TODO: Does not work correctly when subdivision are not 0
Vector2 *texture = reinterpret_cast<Vector2 *>(vb + textureOffset);
texture->U = vertexArray[i].u1;
texture->V = vertexArray[i].v1;
vb += vbSize;
vb[i].x = vertexArray[i].x;
vb[i].y = vertexArray[i].y;
vb[i].z = vertexArray[i].z;
vb[i].diffuse = DX8Wrapper::Convert_Color_Clamp(colors[MIN((i/2), point_cnt)]); // TODO: Does not work correctly when subdivision are not 0
vb[i].u1 = vertexArray[i].u1;
vb[i].v1 = vertexArray[i].v1;
}
}

Expand Down
Loading