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.

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

Correct provoking vertex for lighting when flat shading #11577

Merged
merged 3 commits into from
Nov 23, 2018
Merged

Correct provoking vertex for lighting when flat shading #11577

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
11a8857
SoftGPU: Correct clipping for flat shading.
unknownbrackets Nov 23, 2018
cd6783f
GPU: Use provoking normal for lighting with flat.
unknownbrackets Nov 23, 2018
529efde
GE Debugger: Fix display of raw normals.
unknownbrackets Nov 23, 2018
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
49 changes: 24 additions & 25 deletions GPU/Common/SoftwareTransformCommon.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,9 +191,9 @@ void SoftwareTransform(
fog_slope = std::signbit(fog_slope) ? -65535.0f : 65535.0f;
}

int colorIndOffset = 0;
int provokeIndOffset = 0;
if (params->provokeFlatFirst) {
colorIndOffset = ColorIndexOffset(prim, gstate.getShadeMode(), gstate.isModeClear());
provokeIndOffset = ColorIndexOffset(prim, gstate.getShadeMode(), gstate.isModeClear());
}

VertexReader reader(decoded, decVtxFormat, vertType);
Expand All @@ -206,8 +206,8 @@ void SoftwareTransform(
reader.ReadPos(vert.pos);

if (reader.hasColor0()) {
if (colorIndOffset != 0 && index + colorIndOffset < maxIndex) {
reader.Goto(index + colorIndOffset);
if (provokeIndOffset != 0 && index + provokeIndOffset < maxIndex) {
reader.Goto(index + provokeIndOffset);
reader.ReadColor0_8888(vert.color0);
reader.Goto(index);
} else {
Expand Down Expand Up @@ -247,10 +247,24 @@ void SoftwareTransform(
Vec3f worldnormal(0, 0, 1);
reader.ReadPos(pos);

float ruv[2] = { 0.0f, 0.0f };
if (reader.hasUV())
reader.ReadUV(ruv);

// Read all the provoking vertex values here.
Vec4f unlitColor;
if (provokeIndOffset != 0 && index + provokeIndOffset < maxIndex)
reader.Goto(index + provokeIndOffset);
if (reader.hasColor0())
reader.ReadColor0(unlitColor.AsArray());
else
unlitColor = Vec4f::FromRGBA(gstate.getMaterialAmbientRGBA());
if (reader.hasNormal())
reader.ReadNrm(normal.AsArray());

if (!skinningEnabled) {
Vec3ByMatrix43(out, pos, gstate.worldMatrix);
if (reader.hasNormal()) {
reader.ReadNrm(normal.AsArray());
if (gstate.areNormalsReversed()) {
normal = -normal;
}
Expand All @@ -259,9 +273,9 @@ void SoftwareTransform(
}
} else {
float weights[8];
// TODO: For flat, are weights from the provoking used for color/normal?
reader.Goto(index);
reader.ReadWeights(weights);
if (reader.hasNormal())
reader.ReadNrm(normal.AsArray());

// Skinning
Vec3f psum(0, 0, 0);
Expand Down Expand Up @@ -291,20 +305,7 @@ void SoftwareTransform(
}
}

// Perform lighting here if enabled. don't need to check through, it's checked above.
Vec4f unlitColor = Vec4f(1, 1, 1, 1);
if (reader.hasColor0()) {
if (colorIndOffset != 0 && index + colorIndOffset < maxIndex) {
reader.Goto(index + colorIndOffset);
reader.ReadColor0(&unlitColor.x);
reader.Goto(index);
} else {
reader.ReadColor0(&unlitColor.x);
}
} else {
unlitColor = Vec4f::FromRGBA(gstate.getMaterialAmbientRGBA());
}

// Perform lighting here if enabled.
if (gstate.isLightingEnabled()) {
float litColor0[4];
float litColor1[4];
Expand Down Expand Up @@ -338,10 +339,6 @@ void SoftwareTransform(
}
}

float ruv[2] = {0.0f, 0.0f};
if (reader.hasUV())
reader.ReadUV(ruv);

// Perform texture coordinate generation after the transform and lighting - one style of UV depends on lights.
switch (gstate.getUVGenMode()) {
case GE_TEXMAP_TEXTURE_COORDS: // UV mapping
Expand All @@ -354,6 +351,8 @@ void SoftwareTransform(

case GE_TEXMAP_TEXTURE_MATRIX:
{
// TODO: What's the correct behavior with flat shading? Provoked normal or real normal?
Copy link
Owner

Choose a reason for hiding this comment

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

My guess would be real normal, flat shading is not really related to the texture pipe at all I think..


// Projection mapping
Vec3f source;
switch (gstate.getUVProjMode()) {
Expand Down
34 changes: 23 additions & 11 deletions GPU/Software/Clipper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,10 +176,10 @@ void ProcessRect(const VertexData& v0, const VertexData& v1)
}

// Four triangles to do backfaces as well. Two of them will get backface culled.
ProcessTriangle(*topleft, *topright, *bottomright);
ProcessTriangle(*bottomright, *topright, *topleft);
ProcessTriangle(*bottomright, *bottomleft, *topleft);
ProcessTriangle(*topleft, *bottomleft, *bottomright);
ProcessTriangle(*topleft, *topright, *bottomright, buf[3]);
ProcessTriangle(*bottomright, *topright, *topleft, buf[3]);
ProcessTriangle(*bottomright, *bottomleft, *topleft, buf[3]);
ProcessTriangle(*topleft, *bottomleft, *bottomright, buf[3]);
} else {
// through mode handling
VertexData buf[4];
Expand Down Expand Up @@ -271,10 +271,17 @@ void ProcessLine(VertexData& v0, VertexData& v1)
Rasterizer::DrawLine(data[0], data[1]);
}

void ProcessTriangle(VertexData& v0, VertexData& v1, VertexData& v2)
{
void ProcessTriangle(VertexData& v0, VertexData& v1, VertexData& v2, const VertexData &provoking) {
if (gstate.isModeThrough()) {
Rasterizer::DrawTriangle(v0, v1, v2);
// In case of cull reordering, make sure the right color is on the final vertex.
if (gstate.getShadeMode() == GE_SHADE_FLAT) {
VertexData corrected2 = v2;
corrected2.color0 = provoking.color0;
corrected2.color1 = provoking.color1;
Rasterizer::DrawTriangle(v0, v1, corrected2);
} else {
Rasterizer::DrawTriangle(v0, v1, v2);
}
return;
}

Expand Down Expand Up @@ -339,14 +346,19 @@ void ProcessTriangle(VertexData& v0, VertexData& v1, VertexData& v2)
return;
}

for (int i = 0; i+3 <= numIndices; i+=3)
{
if(indices[i] != SKIP_FLAG)
{
for (int i = 0; i + 3 <= numIndices; i += 3) {
if (indices[i] != SKIP_FLAG) {
VertexData data[3] = { *Vertices[indices[i]], *Vertices[indices[i+1]], *Vertices[indices[i+2]] };
data[0].screenpos = TransformUnit::ClipToScreen(data[0].clippos);
data[1].screenpos = TransformUnit::ClipToScreen(data[1].clippos);
data[2].screenpos = TransformUnit::ClipToScreen(data[2].clippos);

if (gstate.getShadeMode() == GE_SHADE_FLAT) {
// So that the order of clipping doesn't matter...
data[2].color0 = provoking.color0;
data[2].color1 = provoking.color1;
}

Rasterizer::DrawTriangle(data[0], data[1], data[2]);
}
}
Expand Down
2 changes: 1 addition & 1 deletion GPU/Software/Clipper.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ namespace Clipper {

void ProcessPoint(VertexData& v0);
void ProcessLine(VertexData& v0, VertexData& v1);
void ProcessTriangle(VertexData& v0, VertexData& v1, VertexData& v2);
void ProcessTriangle(VertexData& v0, VertexData& v1, VertexData& v2, const VertexData &provoking);
void ProcessRect(const VertexData& v0, const VertexData& v1);

}
32 changes: 17 additions & 15 deletions GPU/Software/TransformUnit.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,12 +342,12 @@ void TransformUnit::SubmitPrimitive(void* vertices, void* indices, GEPrimitiveTy
case GE_PRIM_TRIANGLES:
{
if (!gstate.isCullEnabled() || gstate.isModeClear()) {
Clipper::ProcessTriangle(data[0], data[1], data[2]);
Clipper::ProcessTriangle(data[2], data[1], data[0]);
Clipper::ProcessTriangle(data[0], data[1], data[2], data[2]);
Clipper::ProcessTriangle(data[2], data[1], data[0], data[2]);
} else if (!gstate.getCullMode()) {
Clipper::ProcessTriangle(data[2], data[1], data[0]);
Clipper::ProcessTriangle(data[2], data[1], data[0], data[2]);
} else {
Clipper::ProcessTriangle(data[0], data[1], data[2]);
Clipper::ProcessTriangle(data[0], data[1], data[2], data[2]);
}
break;
}
Expand Down Expand Up @@ -413,7 +413,8 @@ void TransformUnit::SubmitPrimitive(void* vertices, void* indices, GEPrimitiveTy
vreader.Goto(vtx);
}

data[(data_index++) % 3] = ReadVertex(vreader);
int provoking_index = (data_index++) % 3;
data[provoking_index] = ReadVertex(vreader);
if (outside_range_flag) {
// Drop all primitives containing the current vertex
skip_count = 2;
Expand All @@ -427,14 +428,14 @@ void TransformUnit::SubmitPrimitive(void* vertices, void* indices, GEPrimitiveTy
}

if (!gstate.isCullEnabled() || gstate.isModeClear()) {
Clipper::ProcessTriangle(data[0], data[1], data[2]);
Clipper::ProcessTriangle(data[2], data[1], data[0]);
Clipper::ProcessTriangle(data[0], data[1], data[2], data[provoking_index]);
Clipper::ProcessTriangle(data[2], data[1], data[0], data[provoking_index]);
} else if ((!gstate.getCullMode()) ^ ((data_index - 1) % 2)) {
// We need to reverse the vertex order for each second primitive,
// but we additionally need to do that for every primitive if CCW cullmode is used.
Clipper::ProcessTriangle(data[2], data[1], data[0]);
Clipper::ProcessTriangle(data[2], data[1], data[0], data[provoking_index]);
} else {
Clipper::ProcessTriangle(data[0], data[1], data[2]);
Clipper::ProcessTriangle(data[0], data[1], data[2], data[provoking_index]);
}
}
break;
Expand Down Expand Up @@ -466,7 +467,8 @@ void TransformUnit::SubmitPrimitive(void* vertices, void* indices, GEPrimitiveTy
vreader.Goto(vtx);
}

data[2 - ((data_index++) % 2)] = ReadVertex(vreader);
int provoking_index = 2 - ((data_index++) % 2);
data[provoking_index] = ReadVertex(vreader);
if (outside_range_flag) {
// Drop all primitives containing the current vertex
skip_count = 2;
Expand All @@ -480,14 +482,14 @@ void TransformUnit::SubmitPrimitive(void* vertices, void* indices, GEPrimitiveTy
}

if (!gstate.isCullEnabled() || gstate.isModeClear()) {
Clipper::ProcessTriangle(data[0], data[1], data[2]);
Clipper::ProcessTriangle(data[2], data[1], data[0]);
Clipper::ProcessTriangle(data[0], data[1], data[2], data[provoking_index]);
Clipper::ProcessTriangle(data[2], data[1], data[0], data[provoking_index]);
} else if ((!gstate.getCullMode()) ^ ((data_index - 1) % 2)) {
// We need to reverse the vertex order for each second primitive,
// but we additionally need to do that for every primitive if CCW cullmode is used.
Clipper::ProcessTriangle(data[2], data[1], data[0]);
Clipper::ProcessTriangle(data[2], data[1], data[0], data[provoking_index]);
} else {
Clipper::ProcessTriangle(data[0], data[1], data[2]);
Clipper::ProcessTriangle(data[0], data[1], data[2], data[provoking_index]);
}
}
break;
Expand All @@ -508,7 +510,7 @@ bool TransformUnit::GetCurrentSimpleVertices(int count, std::vector<GPUDebugVert
u16 indexLowerBound = 0;
u16 indexUpperBound = count - 1;

if ((gstate.vertType & GE_VTYPE_IDX_MASK) != GE_VTYPE_IDX_NONE) {
if (count > 0 && (gstate.vertType & GE_VTYPE_IDX_MASK) != GE_VTYPE_IDX_NONE) {
const u8 *inds = Memory::GetPointer(gstate_c.indexAddr);
const u16 *inds16 = (const u16 *)inds;
const u32 *inds32 = (const u32 *)inds;
Expand Down
5 changes: 5 additions & 0 deletions Windows/GEDebugger/TabVertices.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ void CtrlVertexList::FormatVertColRaw(wchar_t *dest, int row, int col) {
const u8 *pos = vert + decoder->posoff;
const u8 *tc = vert + decoder->tcoff;
const u8 *color = vert + decoder->coloff;
const u8 *norm = vert + decoder->nrmoff;

switch (col) {
case VERTEXLIST_COL_X:
Expand All @@ -208,6 +209,10 @@ void CtrlVertexList::FormatVertColRaw(wchar_t *dest, int row, int col) {
FormatVertColRawColor(dest, color, decoder->col);
break;

case VERTEXLIST_COL_NX: FormatVertColRawType(dest, norm, decoder->nrm, 0); break;
case VERTEXLIST_COL_NY: FormatVertColRawType(dest, norm, decoder->nrm, 1); break;
case VERTEXLIST_COL_NZ: FormatVertColRawType(dest, norm, decoder->nrm, 2); break;

default:
wcscpy(dest, L"Invalid");
break;
Expand Down