Merge pull request #11402 from Pokechu22/too-many-indices

IndexGenerator: Fix off-by-one in GetRemainingIndices
This commit is contained in:
Mai 2023-01-05 15:15:09 +00:00 committed by GitHub
commit cc14d60bbb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 40 additions and 21 deletions

View File

@ -328,12 +328,29 @@ void IndexGenerator::AddExternalIndices(const u16* indices, u32 num_indices, u32
u32 IndexGenerator::GetRemainingIndices(OpcodeDecoder::Primitive primitive) const
{
u32 max_index = USHRT_MAX;
u32 max_index = UINT16_MAX;
if (g_Config.UseVSForLinePointExpand() && primitive >= OpcodeDecoder::Primitive::GX_DRAW_LINES)
max_index >>= 2;
// -1 is reserved for primitive restart
// Although we reserve UINT16_MAX for primitive restart, we aren't allowed to use that as an
// actual index. But, the maximum number of vertices a game can send is UINT16_MAX, so up to
// 0xffff indices will be used by the game. These indices would be 0x0000 through 0xfffe,
// and since m_base_index gets incremented for each index used, after that m_base_index
// would be 0xffff and no indices remain. If a game uses 0xfffe vertices, assuming m_base_index
// started at 0 it would end at 0xfffe and one more index could be used. So, we do not need to
// subtract 1 from max_index to correctly reserve one index for primitive restart.
//
// Pocoyo Racing uses a draw command with 0xffff vertices, which previously caused issues; see
// https://bugs.dolphin-emu.org/issues/13136 for details.
return max_index - m_base_index - 1;
if (m_base_index > max_index) [[unlikely]]
{
PanicAlertFmt("GetRemainingIndices would overflow; we've already written too many indices? "
"base index {} > max index {}",
m_base_index, max_index);
return 0;
}
return max_index - m_base_index;
}

View File

@ -140,26 +140,11 @@ DataReader VertexManagerBase::PrepareForAdditionalData(OpcodeDecoder::Primitive
}
// Check for size in buffer, if the buffer gets full, call Flush()
if (!m_is_flushed &&
(count > m_index_generator.GetRemainingIndices(primitive) ||
count > GetRemainingIndices(primitive) || needed_vertex_bytes > GetRemainingSize()))
if (!m_is_flushed && (count > m_index_generator.GetRemainingIndices(primitive) ||
count > GetRemainingIndices(primitive) ||
needed_vertex_bytes > GetRemainingSize())) [[unlikely]]
{
Flush();
if (count > m_index_generator.GetRemainingIndices(primitive))
{
ERROR_LOG_FMT(VIDEO, "Too little remaining index values. Use 32-bit or reset them on flush.");
}
if (count > GetRemainingIndices(primitive))
{
ERROR_LOG_FMT(VIDEO, "VertexManager: Buffer not large enough for all indices! "
"Increase MAXIBUFFERSIZE or we need primitive breaking after all.");
}
if (needed_vertex_bytes > GetRemainingSize())
{
ERROR_LOG_FMT(VIDEO, "VertexManager: Buffer not large enough for all vertices! "
"Increase MAXVBUFFERSIZE or we need primitive breaking after all.");
}
}
m_cull_all = cullall;
@ -182,6 +167,23 @@ DataReader VertexManagerBase::PrepareForAdditionalData(OpcodeDecoder::Primitive
m_is_flushed = false;
}
// Now that we've reset the buffer, there should be enough space. It's possible that we still
// won't have enough space in a few rare cases, such as vertex shader line/point expansion with a
// ton of lines in one draw command, in which case we will either need to add support for
// splitting a single draw command into multiple draws or using bigger indices.
ASSERT_MSG(VIDEO, count <= m_index_generator.GetRemainingIndices(primitive),
"VertexManager: Too few remaining index values ({} > {}). "
"32-bit indices or primitive breaking needed.",
count, m_index_generator.GetRemainingIndices(primitive));
ASSERT_MSG(VIDEO, count <= GetRemainingIndices(primitive),
"VertexManager: Buffer not large enough for all indices! ({} > {}) "
"Increase MAXIBUFFERSIZE or we need primitive breaking after all.",
count, GetRemainingIndices(primitive));
ASSERT_MSG(VIDEO, needed_vertex_bytes <= GetRemainingSize(),
"VertexManager: Buffer not large enough for all vertices! ({} > {}) "
"Increase MAXVBUFFERSIZE or we need primitive breaking after all.",
needed_vertex_bytes, GetRemainingSize());
return DataReader(m_cur_buffer_pointer, m_end_buffer_pointer);
}