diff --git a/Source/Core/VideoCommon/IndexGenerator.cpp b/Source/Core/VideoCommon/IndexGenerator.cpp index 5d86561ffd..03d0815a13 100644 --- a/Source/Core/VideoCommon/IndexGenerator.cpp +++ b/Source/Core/VideoCommon/IndexGenerator.cpp @@ -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; } diff --git a/Source/Core/VideoCommon/VertexManagerBase.cpp b/Source/Core/VideoCommon/VertexManagerBase.cpp index 88d7c47fba..85e2b48556 100644 --- a/Source/Core/VideoCommon/VertexManagerBase.cpp +++ b/Source/Core/VideoCommon/VertexManagerBase.cpp @@ -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); }