swung0x48/MobileGL
1
0
Fork 0
mirror of https://github.com/MobileGL-Dev/MobileGL synced 2026-09-08 20:28:32 +09:00
Code Issues Packages Projects Releases Wiki Activity

[Fix] (Backend): resolve primitive restart per draw - never for a non-indexed one, and never on an index the type cannot hold

Browse Source
This commit is contained in:
Swung0x48
2026-08-27 03:18:09 -04:00
parent e69e939d1a
commit 90b7a689c5
8 changed files with 486 additions and 131 deletions
Download Patch File Download Diff File Expand all files Collapse all files
MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp
+178 -87
View File
@@ -2467,13 +2467,18 @@ namespace MobileGL::MG_Backend::DirectGLES {
// makes it stale too. Both level comparisons sit INSIDE the >= 0 guard: a program
// with a control stage of its own - which is nearly all of them - still pays only
// the one integer test.
//
// Compared by BIT PATTERN, matching what DirectVulkan hashes into its module key.
// A float compare here would never settle for a NaN level - NaN != NaN - and every
// draw of that program would re-transpile, re-compile and re-link a byte-identical
// shader. glPatchParameterfv accepts NaN by design.
(twin->GetPassthroughTessControlPatchVertices() >= 0 &&
(twin->GetPassthroughTessControlPatchVertices() !=
static_cast<Int>(MG_State::pGLContext->GetPatchVertices()) ||
twin->GetPassthroughTessControlOuterLevel() !=
MG_State::pGLContext->GetPatchDefaultOuterLevel() ||
twin->GetPassthroughTessControlInnerLevel() !=
MG_State::pGLContext->GetPatchDefaultInnerLevel()))) {
!BitwiseEqual(twin->GetPassthroughTessControlOuterLevel(),
MG_State::pGLContext->GetPatchDefaultOuterLevel()) ||
!BitwiseEqual(twin->GetPassthroughTessControlInnerLevel(),
MG_State::pGLContext->GetPatchDefaultInnerLevel())))) {
twin->SyncToBackend(currentProgram);
}
g_currentDrawFrontendProgram = currentProgram.get();
@@ -3538,8 +3543,14 @@ namespace MobileGL::MG_Backend::DirectGLES {
// to rewrite the whole element array buffer rather than this draw's range - which is
// exactly what it does when no CPU-known count is handed to it. Held for the whole
// command loop so every command in the batch reads the rewritten copy.
//
// firstIndex counts ELEMENTS, so it survives a widened copy untouched; what does not
// survive is the type and the element size, which are re-taken from the substitution
// below for both the native and the CPU-unrolled path.
const ScopedRestartIndexSubstitution restart(type, /*count=*/0, /*indices=*/nullptr);
if (!restart.DrawIsValid()) return;
type = restart.IndexType();
indexSize = MG_Util::GetGLTypeSize(type);
const Bool useNative = drawIndirectBuffer != nullptr && SupportsNativeIndirectDraws();
if (useNative) {
// gl_BaseInstance must observe GPU-written command fields; expose the indirect
@@ -3852,8 +3863,7 @@ namespace MobileGL::MG_Backend::DirectGLES {
// ever restarts on the all-ones value of the index type. When the two agree - which
// includes every GL_PRIMITIVE_RESTART_FIXED_INDEX user - the render state push at
// SyncRenderState is the whole implementation and nothing here does any work. When they
// disagree the index DATA is rewritten into a scratch element array buffer, which is
// what DirectVulkan has always done (VulkanRenderer's RewriteRestartIndices).
// disagree the index DATA is rewritten into a scratch element array buffer.
//
// This used to throw instead. A throw here unwinds a C++ exception through the C GL ABI
// and takes the process down - the same hazard GL_Texture.cpp and RenderState.cpp
@@ -3875,63 +3885,82 @@ namespace MobileGL::MG_Backend::DirectGLES {
// trying: a draw that renders nothing is recoverable, a stall of that size is not.
constexpr SizeT kMaxRestartRewriteBytes = SizeT{1} << 26; // 64 MiB
SizeT RestartIndexTypeSize(GLenum indexType) {
// The index type one step wider than this one, or 0 when there is none. Widening is how
// an all-ones value that is a REAL vertex index keeps its meaning while the all-ones
// value of the destination type serves as the restart sentinel: a source that cannot
// spell 0xFFFF cannot collide with a 16-bit sentinel, and likewise 8 -> 16.
GLenum WiderIndexType(GLenum indexType) {
switch (indexType) {
case GL_UNSIGNED_BYTE: return 1;
case GL_UNSIGNED_SHORT: return 2;
case GL_UNSIGNED_INT: return 4;
case GL_UNSIGNED_BYTE: return GL_UNSIGNED_SHORT;
case GL_UNSIGNED_SHORT: return GL_UNSIGNED_INT;
default: return 0;
}
}
// The value GLES restarts on for this index type. Zero for a type that cannot index
// at all, which the caller treats as "nothing to do" - the driver will reject the
// draw on its own terms.
Uint32 FixedRestartIndexFor(GLenum indexType) {
switch (indexType) {
case GL_UNSIGNED_BYTE: return 0xFFu;
case GL_UNSIGNED_SHORT: return 0xFFFFu;
case GL_UNSIGNED_INT: return 0xFFFFFFFFu;
default: return 0;
Uint32 ReadIndex(const Uint8* source, SizeT i, SizeT indexSize) {
switch (indexSize) {
case 1: return source[i];
case 2: {
Uint16 narrow = 0;
std::memcpy(&narrow, source + i * 2, sizeof(narrow));
return narrow;
}
default: {
Uint32 wide = 0;
std::memcpy(&wide, source + i * 4, sizeof(wide));
return wide;
}
}
}
// Copies index data, replacing every occurrence of the application's arbitrary
// restart index with the fixed all-ones value - the only one GLES restarts on. An
// index that already equals the fixed value would then be indistinguishable from a
// restart, so it is nudged to the next-lowest value: it can only be a real index
// (the application's restart index is a different number), and the vertex it selects
// is outside any well-defined draw anyway, whereas leaving it alone would tear the
// primitive in two. Byte-for-byte the rule DirectVulkan applies.
void RewriteRestartIndices(const void* source, SizeT sizeBytes, GLenum indexType,
Uint32 applicationRestartIndex, Vector<Uint8>& output) {
output.resize(sizeBytes);
if (sizeBytes == 0 || source == nullptr) {
return;
void WriteIndex(Uint8* destination, SizeT i, SizeT indexSize, Uint32 value) {
switch (indexSize) {
case 1: destination[i] = static_cast<Uint8>(value); break;
case 2: {
const Uint16 narrow = static_cast<Uint16>(value);
std::memcpy(destination + i * 2, &narrow, sizeof(narrow));
break;
}
std::memcpy(output.data(), source, sizeBytes);
const auto rewrite = [&](auto* indices, auto fixedMax) {
const SizeT count = sizeBytes / sizeof(*indices);
for (SizeT i = 0; i < count; ++i) {
if (indices[i] == static_cast<decltype(fixedMax)>(applicationRestartIndex)) {
indices[i] = fixedMax;
} else if (indices[i] == fixedMax) {
indices[i] = static_cast<decltype(fixedMax)>(fixedMax - 1);
}
default: std::memcpy(destination + i * 4, &value, sizeof(value)); break;
}
}
// True when any index in the range already holds the type's all-ones value, i.e. when
// that value is doing double duty as a real vertex index and so cannot also be the
// restart sentinel. Only asked on the rare substitution path.
Bool ContainsFixedRestartIndex(const Uint8* source, SizeT indexCount, SizeT indexSize,
Uint32 fixedMax) {
for (SizeT i = 0; i < indexCount; ++i) {
if (ReadIndex(source, i, indexSize) == fixedMax) return true;
}
return false;
}
// Copies index data, replacing every occurrence of the application's restart index with
// the all-ones value of the DESTINATION type - the only one GLES restarts on. The
// destination may be wider than the source, which is what makes the copy lossless: a
// source index equal to the source's all-ones value zero-extends to something the wider
// sentinel can never equal, so it stays the vertex it was.
//
// Same width in and out is the degenerate case, used when the source contains no
// all-ones index at all (nothing to protect) or when there is no wider type to move to.
// In that last case only - a GL_UNSIGNED_INT stream that really does use index
// 0xFFFFFFFF while asking to restart on a different one - a legal index has to be
// nudged to 0xFFFFFFFE, because 32 bits cannot hold both meanings. The caller logs it;
// it is the one input this feature cannot represent.
void RewriteRestartIndices(const Uint8* source, SizeT indexCount, SizeT sourceIndexSize,
SizeT destinationIndexSize, Uint32 applicationRestartIndex,
Uint32 destinationFixedMax, Vector<Uint8>& output) {
output.resize(indexCount * destinationIndexSize);
for (SizeT i = 0; i < indexCount; ++i) {
Uint32 value = ReadIndex(source, i, sourceIndexSize);
if (value == applicationRestartIndex) {
value = destinationFixedMax;
} else if (value == destinationFixedMax) {
// Only reachable when no widening was possible; see above.
value = destinationFixedMax - 1;
}
};
switch (indexType) {
case GL_UNSIGNED_BYTE:
rewrite(reinterpret_cast<Uint8*>(output.data()), static_cast<Uint8>(0xFFu));
break;
case GL_UNSIGNED_SHORT:
rewrite(reinterpret_cast<Uint16*>(output.data()), static_cast<Uint16>(0xFFFFu));
break;
case GL_UNSIGNED_INT:
rewrite(reinterpret_cast<Uint32*>(output.data()), static_cast<Uint32>(0xFFFFFFFFu));
break;
default:
break;
WriteIndex(output.data(), i, destinationIndexSize, value);
}
}
@@ -3975,14 +4004,21 @@ namespace MobileGL::MG_Backend::DirectGLES {
}
} // namespace
Bool NeedsArbitraryRestartSubstitution(GLenum indexType) {
RestartSubstitutionKind ResolveRestartSubstitution(GLenum indexType) {
if (!MG_State::pGLContext->IsCapabilityEnabled(CapabilityInput::PrimitiveRestart) ||
MG_State::pGLContext->IsCapabilityEnabled(CapabilityInput::PrimitiveRestartFixedIndex)) {
return false;
return RestartSubstitutionKind::None;
}
const Uint32 fixedMax = FixedRestartIndexFor(indexType);
if (fixedMax == 0) return false;
return MG_State::pGLContext->GetPrimitiveRestartIndex() != fixedMax;
const Uint32 fixedMax = MG_Util::FixedRestartIndexForGLType(indexType);
if (fixedMax == 0) return RestartSubstitutionKind::None;
const Uint32 restartIndex = MG_State::pGLContext->GetPrimitiveRestartIndex();
if (restartIndex == fixedMax) return RestartSubstitutionKind::None;
// Strictly greater, never truncated. GL 4.6 core 10.3.6 compares the fetched index
// zero-extended against the full 32-bit state, so an index this type cannot hold matches
// nothing. Truncating instead - glPrimitiveRestartIndex(0x100) over GL_UNSIGNED_BYTE data
// becoming "restart on 0" - turns the most common index in any mesh into a restart.
if (restartIndex > fixedMax) return RestartSubstitutionKind::SuppressRestart;
return RestartSubstitutionKind::RewriteIndices;
}
void OnRestartSubstitutionContextDestroyed() {
@@ -3991,22 +4027,43 @@ namespace MobileGL::MG_Backend::DirectGLES {
g_restartStaging.shrink_to_fit();
}
ScopedSuppressedPrimitiveRestart::ScopedSuppressedPrimitiveRestart(RestartSubstitutionKind kind) {
if (kind != RestartSubstitutionKind::SuppressRestart) return;
// SyncRenderState turned the driver's fixed-index restart on because GL_PRIMITIVE_RESTART
// is enabled; for this draw's index type it would restart on a value the application
// never named. Toggled directly rather than through the render-state shadow, and put back
// in the destructor, so the shadow stays true and the next draw pays nothing.
g_GLESFuncs.glDisable(GL_PRIMITIVE_RESTART_FIXED_INDEX);
m_suppressed = true;
}
ScopedSuppressedPrimitiveRestart::~ScopedSuppressedPrimitiveRestart() {
if (!m_suppressed) return;
g_GLESFuncs.glEnable(GL_PRIMITIVE_RESTART_FIXED_INDEX);
}
ScopedRestartIndexSubstitution::ScopedRestartIndexSubstitution(GLenum indexType, GLsizei count,
const void* indices)
: m_indices(indices) {
if (!NeedsArbitraryRestartSubstitution(indexType)) {
: m_kind(ResolveRestartSubstitution(indexType)), m_capOverride(m_kind), m_indices(indices),
m_indexType(indexType) {
if (m_kind != RestartSubstitutionKind::RewriteIndices) {
return;
}
const SizeT indexSize = RestartIndexTypeSize(indexType);
const SizeT sourceIndexSize = MG_Util::GetGLTypeSize(indexType);
const Uint32 fixedMax = MG_Util::FixedRestartIndexForGLType(indexType);
const Uint32 applicationRestartIndex = MG_State::pGLContext->GetPrimitiveRestartIndex();
const auto& indexBuffer = BoundElementArrayBuffer();
const Uint8* source = nullptr;
SizeT indexCount = 0;
SizeT sourceByteOffset = 0;
if (indexBuffer) {
// The WHOLE buffer is rewritten, not just this draw's range, so that every index
// keeps its position: an indirect draw's firstIndex lives in GPU memory and
// cannot be adjusted from here. Same reasoning, same shape, as DirectVulkan.
// keeps its position: an indirect draw's firstIndex lives in GPU memory and cannot be
// adjusted from here. It is an ELEMENT index, so it survives widening unchanged.
const SizeT sizeBytes = indexBuffer->GetSize();
if (sizeBytes == 0) {
if (sizeBytes < sourceIndexSize) {
return; // Nothing to restart on; let the driver see the draw unchanged.
}
if (sizeBytes > kMaxRestartRewriteBytes) {
@@ -4022,36 +4079,64 @@ namespace MobileGL::MG_Backend::DirectGLES {
// shader write may have moved past it since the last sync.
indexBuffer->SyncPersistentMappedRange();
indexBuffer->SyncGpuWrites();
const Uint8* bytes = indexBuffer->MappedData();
if (bytes == nullptr) {
source = indexBuffer->MappedData();
if (source == nullptr) {
MGLOG_E_ONCE("Draw skipped: GL_PRIMITIVE_RESTART with restart index %u needs a CPU-readable copy of "
"the bound element array buffer and none is available.",
applicationRestartIndex);
m_valid = false;
return;
}
RewriteRestartIndices(bytes, sizeBytes, indexType, applicationRestartIndex, g_restartStaging);
indexCount = sizeBytes / sourceIndexSize;
sourceByteOffset = reinterpret_cast<SizeT>(indices);
} else {
// No element array buffer: `indices` is a client pointer, so only the draw's own
// range is readable and an indirect draw has nothing to read at all.
if (count <= 0 || indices == nullptr || indexSize == 0) {
if (count <= 0 || indices == nullptr || sourceIndexSize == 0) {
MGLOG_E_ONCE("Draw skipped: GL_PRIMITIVE_RESTART with restart index %u needs either a bound element "
"array buffer or a client index array with a CPU-known count.",
applicationRestartIndex);
m_valid = false;
return;
}
const SizeT sizeBytes = static_cast<SizeT>(count) * indexSize;
if (sizeBytes > kMaxRestartRewriteBytes) {
if (static_cast<SizeT>(count) * sourceIndexSize > kMaxRestartRewriteBytes) {
MGLOG_E_ONCE("Draw skipped: GL_PRIMITIVE_RESTART index rewrite of %zu bytes is past the %zu-byte "
"ceiling.",
sizeBytes, kMaxRestartRewriteBytes);
static_cast<SizeT>(count) * sourceIndexSize, kMaxRestartRewriteBytes);
m_valid = false;
return;
}
RewriteRestartIndices(indices, sizeBytes, indexType, applicationRestartIndex, g_restartStaging);
source = static_cast<const Uint8*>(indices);
indexCount = static_cast<SizeT>(count);
}
// Widen only when the source really does use the all-ones value as a vertex index -
// otherwise the sentinel is free and the copy stays the caller's width, which keeps the
// common substitution allocation-for-allocation identical to the narrow form.
GLenum destinationType = indexType;
SizeT destinationIndexSize = sourceIndexSize;
if (ContainsFixedRestartIndex(source, indexCount, sourceIndexSize, fixedMax)) {
const GLenum wider = WiderIndexType(indexType);
// An element-array offset that is not a whole number of indices cannot be rescaled
// into the widened copy, so such a draw keeps the narrow (lossy) form.
const Bool offsetIsWholeIndices = sourceIndexSize != 0 && (sourceByteOffset % sourceIndexSize) == 0;
if (wider != 0 && offsetIsWholeIndices &&
indexCount * MG_Util::GetGLTypeSize(wider) <= kMaxRestartRewriteBytes) {
destinationType = wider;
destinationIndexSize = MG_Util::GetGLTypeSize(wider);
} else {
MGLOG_E_ONCE("GL_PRIMITIVE_RESTART with restart index %u over index data that also uses the "
"all-ones index %u: this index type cannot spell both, so every all-ones index is "
"drawn as %u instead. Use GL_PRIMITIVE_RESTART_FIXED_INDEX, or keep the all-ones "
"value out of the index data.",
applicationRestartIndex, fixedMax, fixedMax - 1);
}
}
const Uint32 destinationFixedMax = MG_Util::FixedRestartIndexForGLType(destinationType);
RewriteRestartIndices(source, indexCount, sourceIndexSize, destinationIndexSize, applicationRestartIndex,
destinationFixedMax, g_restartStaging);
if (!UploadRestartScratch(g_restartStaging.size(), g_restartStaging.data())) {
MGLOG_E_ONCE("Draw skipped: could not allocate the scratch element array buffer for GL_PRIMITIVE_RESTART "
"index substitution.");
@@ -4061,9 +4146,13 @@ namespace MobileGL::MG_Backend::DirectGLES {
m_previousBinding = BoundElementArrayBufferId();
BufferImpl::BindBufferId(GL_ELEMENT_ARRAY_BUFFER, g_restartIndices.id);
m_substituted = true;
// The rewritten copy starts at byte 0 of the scratch buffer, so an EBO-sourced draw
// keeps the very offset it was given and a client-memory draw reads from the front.
m_indices = indexBuffer ? indices : nullptr;
m_indexType = destinationType;
// The rewritten copy starts at byte 0 of the scratch buffer and holds one
// destination-width element per source element, so an EBO-sourced draw keeps its ELEMENT
// offset (rescaled to the new width) and a client-memory draw reads from the front.
m_indices = indexBuffer
? reinterpret_cast<const void*>((sourceByteOffset / sourceIndexSize) * destinationIndexSize)
: nullptr;
}
ScopedRestartIndexSubstitution::~ScopedRestartIndexSubstitution() {
@@ -4080,7 +4169,7 @@ namespace MobileGL::MG_Backend::DirectGLES {
const ScopedRestartIndexSubstitution restart(type, count, indices);
if (!restart.DrawIsValid()) return;
ForEachViewportRoutingPass([&] {
g_GLESFuncs.glDrawElements(mode, count, type, restart.Indices());
g_GLESFuncs.glDrawElements(mode, count, restart.IndexType(), restart.Indices());
});
}
@@ -4112,7 +4201,7 @@ namespace MobileGL::MG_Backend::DirectGLES {
if (!restart.DrawIsValid()) return;
SetCurrentBaseVertex(basevertex);
ForEachViewportRoutingPass([&] {
g_GLESFuncs.glDrawElementsBaseVertex(mode, count, type, restart.Indices(), basevertex);
g_GLESFuncs.glDrawElementsBaseVertex(mode, count, restart.IndexType(), restart.Indices(), basevertex);
});
SetCurrentBaseVertex(0);
}
@@ -4381,7 +4470,8 @@ namespace MobileGL::MG_Backend::DirectGLES {
if (!restart.DrawIsValid()) return;
SetCurrentBaseVertex(basevertex);
ForEachViewportRoutingPass([&] {
g_GLESFuncs.glDrawRangeElementsBaseVertex(mode, start, end, count, type, restart.Indices(), basevertex);
g_GLESFuncs.glDrawRangeElementsBaseVertex(mode, start, end, count, restart.IndexType(), restart.Indices(),
basevertex);
});
SetCurrentBaseVertex(0);
}
@@ -4392,7 +4482,7 @@ namespace MobileGL::MG_Backend::DirectGLES {
const ScopedRestartIndexSubstitution restart(type, count, indices);
if (!restart.DrawIsValid()) return;
ForEachViewportRoutingPass([&] {
g_GLESFuncs.glDrawRangeElements(mode, start, end, count, type, restart.Indices());
g_GLESFuncs.glDrawRangeElements(mode, start, end, count, restart.IndexType(), restart.Indices());
});
}
@@ -4420,11 +4510,12 @@ namespace MobileGL::MG_Backend::DirectGLES {
SetCurrentBaseVertex(basevertex);
ForEachViewportRoutingPass([&] {
if (UseNativeBaseInstance()) {
g_GLESFuncs.glDrawElementsInstancedBaseVertexBaseInstanceEXT(mode, count, type, restart.Indices(),
instancecount, basevertex, baseinstance);
g_GLESFuncs.glDrawElementsInstancedBaseVertexBaseInstanceEXT(mode, count, restart.IndexType(),
restart.Indices(), instancecount,
basevertex, baseinstance);
} else {
g_GLESFuncs.glDrawElementsInstancedBaseVertex(mode, count, type, restart.Indices(), instancecount,
basevertex);
g_GLESFuncs.glDrawElementsInstancedBaseVertex(mode, count, restart.IndexType(), restart.Indices(),
instancecount, basevertex);
}
});
SetCurrentBaseVertex(0);
@@ -4455,10 +4546,10 @@ namespace MobileGL::MG_Backend::DirectGLES {
SetCurrentBaseInstance(baseinstance);
ForEachViewportRoutingPass([&] {
if (UseNativeBaseInstance()) {
g_GLESFuncs.glDrawElementsInstancedBaseInstanceEXT(mode, count, type, restart.Indices(), instancecount,
baseinstance);
g_GLESFuncs.glDrawElementsInstancedBaseInstanceEXT(mode, count, restart.IndexType(), restart.Indices(),
instancecount, baseinstance);
} else {
g_GLESFuncs.glDrawElementsInstanced(mode, count, type, restart.Indices(), instancecount);
g_GLESFuncs.glDrawElementsInstanced(mode, count, restart.IndexType(), restart.Indices(), instancecount);
}
});
SetCurrentBaseInstance(0);
@@ -4470,7 +4561,7 @@ namespace MobileGL::MG_Backend::DirectGLES {
const ScopedRestartIndexSubstitution restart(type, count, indices);
if (!restart.DrawIsValid()) return;
ForEachViewportRoutingPass([&] {
g_GLESFuncs.glDrawElementsInstanced(mode, count, type, restart.Indices(), instancecount);
g_GLESFuncs.glDrawElementsInstanced(mode, count, restart.IndexType(), restart.Indices(), instancecount);
});
}
MobileGL/MG_Backend/DirectGLES/Managers.h
+60 -14
View File
@@ -102,28 +102,64 @@ namespace MobileGL::MG_Backend::DirectGLES {
// Brings the whole draw-relevant frontend state onto the native ES context and binds
// the program; every GL draw entry point calls it exactly once before issuing draws.
void PrepareForDraw(DrawSyncFlags syncBits);
// Desktop GL restarts indexed primitives on an application-chosen index
// (glPrimitiveRestartIndex under GL_PRIMITIVE_RESTART); GLES core restarts only on the
// all-ones value of the index type (GL_PRIMITIVE_RESTART_FIXED_INDEX), which the render
// state push already enables for both caps. True when the two disagree for this index
// type, i.e. when the index data itself has to be rewritten for the draw to restart
// where the application asked. False - the overwhelmingly common answer - for a draw
// with restart disabled, with the fixed-index cap, or with a restart index that already
// equals the fixed value.
Bool NeedsArbitraryRestartSubstitution(GLenum indexType);
// What an indexed draw has to do about primitive restart before it can be issued.
//
// Desktop GL restarts on an application-chosen index (glPrimitiveRestartIndex under
// GL_PRIMITIVE_RESTART); GLES core restarts only on the all-ones value of the index type
// (GL_PRIMITIVE_RESTART_FIXED_INDEX), which the render-state push enables for BOTH caps.
// That leaves three cases, and the difference between the last two is not cosmetic - one
// adds restarts, the other has to take away restarts the driver would otherwise make.
enum class RestartSubstitutionKind : Uint8 {
// Nothing to do: restart is off, the fixed-index cap is on, or the application's
// restart index already IS the type's all-ones value. The overwhelmingly common answer.
None,
// The application's index is representable in this index type and differs from the
// all-ones value: the index DATA has to be rewritten so the driver restarts where the
// application asked.
RewriteIndices,
// The application's index cannot be held by this index type at all. GL 4.6 core 10.3.6
// compares the fetched index, zero-extended, against the full 32-bit
// PRIMITIVE_RESTART_INDEX, so no index can match and the draw restarts NOWHERE - but the
// render-state push has already enabled the driver's fixed-index restart, so the
// all-ones value has to be un-restarted for the duration of the draw.
SuppressRestart,
};
RestartSubstitutionKind ResolveRestartSubstitution(GLenum indexType);
// Turns the driver's fixed-index restart off for one draw and back on afterwards, for the
// SuppressRestart case above. Separate from the substitution below because the multi-draw
// tiers need it on its own: they rewrite the index stream themselves and only ever need the
// cap half. Inert for every other kind, and it never touches the render-state shadow - it
// puts the driver back exactly where SyncRenderState left it.
class ScopedSuppressedPrimitiveRestart {
public:
explicit ScopedSuppressedPrimitiveRestart(RestartSubstitutionKind kind);
~ScopedSuppressedPrimitiveRestart();
ScopedSuppressedPrimitiveRestart(const ScopedSuppressedPrimitiveRestart&) = delete;
ScopedSuppressedPrimitiveRestart& operator=(const ScopedSuppressedPrimitiveRestart&) = delete;
private:
Bool m_suppressed = false;
};
// Swaps in a scratch element array buffer holding a copy of the index data in which the
// application's restart index has been replaced by the value GLES restarts on. Inert
// (and free) unless NeedsArbitraryRestartSubstitution says otherwise. The swap lives for
// the object's lifetime, so it covers every pass of a viewport-routed draw, and the
// previous GL_ELEMENT_ARRAY_BUFFER name is restored on destruction - which matters
// beyond tidiness, because the VAO twin memoises that it already synced that binding.
// (and free) unless ResolveRestartSubstitution asks for it. The swap lives for the
// object's lifetime, so it covers every pass of a viewport-routed draw, and the previous
// GL_ELEMENT_ARRAY_BUFFER name is restored on destruction - which matters beyond tidiness,
// because the VAO twin memoises that it already synced that binding.
//
// The copy may be WIDER than the source (see IndexType): when the source already contains
// the type's all-ones value as an ordinary vertex index, that value cannot double as the
// restart sentinel, and widening is the only way to keep both meanings. Callers must
// therefore take the index type from this object, not from their own argument.
class ScopedRestartIndexSubstitution {
public:
// count/indices describe the draw's index range when the CPU knows it. Pass
// count == 0 for an indirect draw, whose count lives in GPU memory: the whole bound
// element array buffer is rewritten instead, so every element keeps its position and
// a GPU-resident firstIndex still addresses the index it named.
// a GPU-resident firstIndex - an ELEMENT index, so it survives widening too - still
// addresses the index it named.
ScopedRestartIndexSubstitution(GLenum indexType, GLsizei count, const void* indices);
~ScopedRestartIndexSubstitution();
ScopedRestartIndexSubstitution(const ScopedRestartIndexSubstitution&) = delete;
@@ -136,9 +172,19 @@ namespace MobileGL::MG_Backend::DirectGLES {
// The element-array offset (or client pointer) the draw must use. Identical to what
// was passed in unless a substitution was made.
const void* Indices() const { return m_indices; }
// The index type the draw must be issued with. Identical to the constructor's unless
// the copy had to be widened to keep an all-ones vertex index distinguishable from the
// restart sentinel.
GLenum IndexType() const { return m_indexType; }
private:
// Declared before m_capOverride so it is initialised first (members initialise in
// declaration order): the whole decision is made once, and both the cap override and the
// constructor body read the same answer.
RestartSubstitutionKind m_kind = RestartSubstitutionKind::None;
ScopedSuppressedPrimitiveRestart m_capOverride;
const void* m_indices = nullptr;
GLenum m_indexType = 0;
Uint m_previousBinding = 0;
Bool m_substituted = false;
Bool m_valid = true;
MobileGL/MG_Backend/DirectGLES/MultiDraw.cpp
+28 -13
View File
@@ -29,20 +29,21 @@ namespace MobileGL::MG_Backend::DirectGLES::MultiDrawImpl {
}
}
// The index value this batch restarts on, in the SOURCE index type's width. Normally
// the all-ones value of that type, which is what GL_PRIMITIVE_RESTART_FIXED_INDEX and
// GLES both restart on; with desktop GL_PRIMITIVE_RESTART it is instead whatever
// glPrimitiveRestartIndex named. The rebased tier turns whichever it is into
// 0xFFFFFFFF in its widened stream, which is what the driver restarts on.
// The index value this batch restarts on, compared at 32 bits against the zero-extended
// source index. Normally the all-ones value of the source type, which is what
// GL_PRIMITIVE_RESTART_FIXED_INDEX and GLES both restart on; with desktop
// GL_PRIMITIVE_RESTART it is instead whatever glPrimitiveRestartIndex named. The rebased
// tier turns whichever it is into 0xFFFFFFFF in its widened stream, which is what the
// driver restarts on.
//
// No truncation, deliberately, and the same rule ResolveRestartSubstitution applies: a
// restart index the source type cannot hold simply matches nothing, so returning it
// verbatim is already "this batch restarts nowhere".
Uint32 RestartSentinelFor(GLenum type) {
if (NeedsArbitraryRestartSubstitution(type)) {
if (ResolveRestartSubstitution(type) != RestartSubstitutionKind::None) {
return MG_State::pGLContext->GetPrimitiveRestartIndex();
}
switch (type) {
case GL_UNSIGNED_BYTE: return 0xFFu;
case GL_UNSIGNED_SHORT: return 0xFFFFu;
default: return 0xFFFFFFFFu;
}
return MG_Util::FixedRestartIndexForGLType(type);
}
Bool RestartActive() {
@@ -502,6 +503,16 @@ namespace MobileGL::MG_Backend::DirectGLES::MultiDrawImpl {
const Bool restartActive = RestartActive();
const Uint32 restartSentinel = RestartSentinelFor(type);
// Widening to GL_UNSIGNED_INT gives a UBYTE/USHORT source a sentinel it can never
// spell, so those batches are lossless. A UINT source that already uses 0xFFFFFFFF as
// a real vertex index while restarting on a different one is the one shape 32 bits
// cannot express - the same corner the single-draw substitution reports.
if (restartActive && indexSize == 4 && restartSentinel != 0xFFFFFFFFu) {
MGLOG_E_ONCE("GL_PRIMITIVE_RESTART with restart index %u over GL_UNSIGNED_INT multi-draw indices: "
"any index that is already 0xFFFFFFFF will restart too, because the rewritten stream "
"has no wider sentinel to move to.",
restartSentinel);
}
g_indexStaging.resize(total);
SizeT cursor = 0;
for (GLsizei i = 0; i < drawcount; ++i) {
@@ -868,8 +879,12 @@ void main() {
if (drawcount <= 0 || !count || !indices) return;
// Read before any GL work, because it decides the tier below: a desktop restart index
// the driver does not know about can only be honoured by the tier that rewrites the
// index stream (see ResolveTierForBatch).
const Bool arbitraryRestart = NeedsArbitraryRestartSubstitution(type);
// index stream (see ResolveTierForBatch). A restart index this index type cannot hold
// needs no rewrite at all - nothing can match it - but it does need the driver's own
// fixed-index restart held off for the batch, which is what the scope below does.
const RestartSubstitutionKind restartKind = ResolveRestartSubstitution(type);
const Bool arbitraryRestart = restartKind == RestartSubstitutionKind::RewriteIndices;
const ScopedSuppressedPrimitiveRestart restartCapOverride(restartKind);
const Bool hasIndexBuffer = BoundIndexBuffer() != nullptr;
MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp
+71 -16
View File
@@ -3985,14 +3985,13 @@ void main() {
const RenderStateParameters& rsp = MG_State::pGLContext->GetRenderStateParameters();
if (rsp.PrimitiveRestartEnabled && !rsp.PrimitiveRestartFixedIndexEnabled) {
const Uint32 restartIndex = rsp.PrimitiveRestartIndex;
Uint32 fixedMax = 0;
switch (vkIndexType) {
case VK_INDEX_TYPE_UINT8: fixedMax = 0xFFu; break;
case VK_INDEX_TYPE_UINT16: fixedMax = 0xFFFFu; break;
case VK_INDEX_TYPE_UINT32: fixedMax = 0xFFFFFFFFu; break;
default: break;
}
substituteRestart = restartIndex != fixedMax;
const Uint32 fixedMax = MG_Util::FixedRestartIndexForGLType(pIndexBufferView->indexType);
// STRICTLY less, and never truncated. Equal needs no rewrite (the driver already
// restarts there); GREATER means the index type cannot hold the application's restart
// index, so GL 4.6 core 10.3.6 says nothing matches it and the draw restarts nowhere -
// which is exactly what ResolvePrimitiveRestartEnable told the pipeline, so rewriting
// here would put restarts into a stream the pipeline was built not to restart on.
substituteRestart = restartIndex < fixedMax;
substituteRestartIndex = restartIndex;
}
@@ -4794,13 +4793,42 @@ void main() {
return program.HasLinkedShaderStage(ShaderStage::Geometry);
}
// GL primitive restart is defined on the INDEX STREAM (GL 4.6 core 10.3.6): it splits
// primitives when a fetched index matches PRIMITIVE_RESTART_INDEX. Two consequences the
// capability bits alone cannot express, both resolved here because only the caller knows them:
//
// - A non-indexed draw has no index stream, so restart is a no-op for it. Leaving the
// pipeline's primitiveRestartEnable on for a glDrawArrays is what made the list-topology
// guard below refuse those draws, so an application that enables GL_PRIMITIVE_RESTART once
// at init lost every glDrawArrays on a device without the extension.
// - The comparison is against the full 32-bit restart index with the fetched index
// zero-extended, so a restart index the type cannot hold (0x100FF against UNSIGNED_BYTE
// data) matches no index and that draw restarts NOWHERE. UploadAndBindIndexBuffer makes the
// same call for the rewrite, and the two must agree or the pipeline says "restart" over
// index data nothing rewrote.
Bool VulkanRenderer::ResolvePrimitiveRestartEnable(Flags<DrawSetupAspect> aspects,
const IndexBufferView* pIndexBufferView) const {
if (!(aspects & DrawSetupAspect::IndexBuffer) || pIndexBufferView == nullptr) {
return false;
}
const RenderStateParameters& rsp = MG_State::pGLContext->GetRenderStateParameters();
if (rsp.PrimitiveRestartFixedIndexEnabled) {
return true;
}
if (!rsp.PrimitiveRestartEnabled) {
return false;
}
return rsp.PrimitiveRestartIndex <= MG_Util::FixedRestartIndexForGLType(pIndexBufferView->indexType);
}
VkPipeline VulkanRenderer::GetOrCreatePipeline(
GLenum mode,
const MG_State::GLState::ProgramObject& program,
const ProgramFactory::VkProgramObject& programObj,
ProgramFactory::CompileOptionFlags transformFlags,
const MG_State::GLState::VertexArrayObject& vao,
const RenderPassEntry& renderPassEntry) {
const RenderPassEntry& renderPassEntry,
Bool primitiveRestartEnable) {
Bool invertClockwise = transformFlags & ProgramFactory::CompileOptionBit::PositionYFlip;
if (programObj.stages.empty()) {
MGLOG_D("GetOrCreatePipeline skipped: program has no shader stages");
@@ -4842,6 +4870,7 @@ void main() {
entry.programHash == programObj.hash && entry.vertexInputHash == vertexLayoutHash &&
entry.renderPassHash == renderPassHash &&
entry.pipelineStateHash == pipelineStateHash &&
entry.primitiveRestartEnable == primitiveRestartEnable &&
entry.transformFlags == transformFlags) {
return entry.pipeline;
}
@@ -5040,9 +5069,22 @@ void main() {
: VK_POLYGON_MODE_FILL;
const VkPrimitiveTopology vkTopology = MG_Util::ConvertPrimitiveModeToVkEnum(mode);
const Bool primitiveRestartEnabled =
MG_State::pGLContext->IsCapabilityEnabled(CapabilityInput::PrimitiveRestart) ||
MG_State::pGLContext->IsCapabilityEnabled(CapabilityInput::PrimitiveRestartFixedIndex);
// Resolved by the caller (ResolvePrimitiveRestartEnable), which knows whether the draw is
// indexed and with what index type; the capability bits alone answer neither.
Bool primitiveRestartEnabled = primitiveRestartEnable;
// GL applies restart to PATCHES only when PRIMITIVE_RESTART_FOR_PATCHES_SUPPORTED is true
// (GL 4.6 core 10.3.6). MobileGL supports no such thing - neither backend has a way to
// restart a patch stream - and GL_FALSE is a legal answer to that query, so a patch draw
// simply never restarts here. Doing this BEFORE the feature guard below is what keeps a
// perfectly ordinary GL_PATCHES draw from being refused on a device that lacks
// VK_EXT_primitive_topology_list_restart. (When GL_PRIMITIVE_RESTART_FOR_PATCHES_SUPPORTED
// is eventually added to glGetIntegerv it has to report GL_FALSE to stay consistent with
// this.)
if (vkTopology == VK_PRIMITIVE_TOPOLOGY_PATCH_LIST) {
primitiveRestartEnabled = false;
}
// Primitive restart on a *list* topology requires the primitiveTopologyListRestart feature;
// strip/fan restart works without it. There is no fallback - silently dropping the restarts
// would weld the primitives on either side of each one together - so the draw is declined
@@ -5053,11 +5095,16 @@ void main() {
// already name); an application that merely enabled a legal desktop feature died instead of
// getting a draw that rendered nothing. VK_NULL_HANDLE is this function's established
// "skip this draw" answer, used by the no-stages case above.
//
// Reached only when this draw's index stream really does restart. Testing the raw
// capability bits here instead - which is what it did - refused every NON-INDEXED
// list-topology draw as well, so an application that enables GL_PRIMITIVE_RESTART once at
// init and then calls glDrawArrays(GL_TRIANGLES, ...) rendered nothing at all.
const auto isListTopology = [](VkPrimitiveTopology t) {
return t == VK_PRIMITIVE_TOPOLOGY_POINT_LIST || t == VK_PRIMITIVE_TOPOLOGY_LINE_LIST ||
t == VK_PRIMITIVE_TOPOLOGY_TRIANGLE_LIST ||
t == VK_PRIMITIVE_TOPOLOGY_LINE_LIST_WITH_ADJACENCY ||
t == VK_PRIMITIVE_TOPOLOGY_TRIANGLE_LIST_WITH_ADJACENCY || t == VK_PRIMITIVE_TOPOLOGY_PATCH_LIST;
t == VK_PRIMITIVE_TOPOLOGY_TRIANGLE_LIST_WITH_ADJACENCY;
};
if (primitiveRestartEnabled && !m_primitiveTopologyListRestartFeatureEnabled && isListTopology(vkTopology)) {
MGLOG_E_ONCE("Draw skipped: primitive restart on a list topology (0x%x) requires the "
@@ -5425,6 +5472,7 @@ void main() {
entry.vertexInputHash = vertexLayoutHash;
entry.renderPassHash = renderPassHash;
entry.pipelineStateHash = pipelineStateHash;
entry.primitiveRestartEnable = primitiveRestartEnable;
entry.transformFlags = transformFlags;
entry.pipeline = pipeline;
m_pipelineMemoNext = (m_pipelineMemoNext + 1) % kPipelineMemoSize;
@@ -5809,7 +5857,11 @@ void main() {
return false;
}
SetupDrawSnapshot& snap = *snapPtr;
if (snap.aspects != aspects.GetRaw() || snap.mode != mode) {
// Resolved once for the whole function: it guards the snapshot, keys the pipeline memo
// probe below, and is handed to GetOrCreatePipeline on a miss - all three must agree.
const Bool drawPrimitiveRestartEnable = ResolvePrimitiveRestartEnable(aspects, pIndexBufferView);
if (snap.aspects != aspects.GetRaw() || snap.mode != mode ||
snap.primitiveRestartEnable != drawPrimitiveRestartEnable) {
return false;
}
if (m_clearManager->HasAnyPendingClears()) {
@@ -6079,6 +6131,7 @@ void main() {
entry.programHash == programObj.hash && entry.vertexInputHash == vaoLayoutHash &&
entry.renderPassHash == snap.renderPassHash &&
entry.pipelineStateHash == m_pipelineStateHash &&
entry.primitiveRestartEnable == drawPrimitiveRestartEnable &&
entry.transformFlags == memoTransformFlags) {
pipeline = entry.pipeline;
break;
@@ -6096,7 +6149,7 @@ void main() {
}
pipeline = GetOrCreatePipeline(mode, program, programObj,
ProgramFactory::CompileOptionFlags(snap.resolvedTransformFlags),
vao, renderPassEntry);
vao, renderPassEntry, drawPrimitiveRestartEnable);
if (pipeline == VK_NULL_HANDLE) {
return false;
}
@@ -6533,7 +6586,8 @@ void main() {
}
}
auto pipeline = GetOrCreatePipeline(mode, program, programObj, transformFlags, vao, *renderPassEntry);
auto pipeline = GetOrCreatePipeline(mode, program, programObj, transformFlags, vao, *renderPassEntry,
ResolvePrimitiveRestartEnable(aspects, pIndexBufferView));
// GetOrCreatePipeline documents a VK_NULL_HANDLE return (empty stages, or a driver that
// rejected vkCreateGraphicsPipelines). Binding it dereferences null inside the driver -
// 9 of the 15 CTS process deaths were exactly this vkCmdBindPipeline. A draw that has no
@@ -6596,6 +6650,7 @@ void main() {
if (nowActiveRenderPass != nullptr && !programObj.hasStorageImages) {
snap.valid = true;
snap.aspects = aspects.GetRaw();
snap.primitiveRestartEnable = ResolvePrimitiveRestartEnable(aspects, pIndexBufferView);
snap.mode = mode;
snap.programLifetimeId = program.GetLifetimeId();
snap.programVersion = program.GetBackendStateVersion();
MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h
+23 -1
View File
@@ -733,6 +733,12 @@ namespace MobileGL::MG_Backend::DirectVulkan {
// values the memo already holds.
Uint64 pipelineStateHash = 0;
ProgramFactory::CompileOptionFlags transformFlags = {};
// Baked into the pipeline (PipelineFactory::ComputeHash mixes it), and NOT derivable
// from anything else in this key: it depends on whether the draw is indexed and on the
// index type, neither of which the mode/program/state hashes carry. Without it an
// indexed and a non-indexed draw over the same program and state collide on one entry
// and the second one gets the first one's restart setting.
Bool primitiveRestartEnable = false;
VkPipeline pipeline = VK_NULL_HANDLE;
};
static constexpr Uint32 kPipelineMemoSize = 8;
@@ -865,6 +871,12 @@ namespace MobileGL::MG_Backend::DirectVulkan {
Uint64 bindGeneration = 0;
Uint32 baseTransformFlags = 0;
Uint32 resolvedTransformFlags = 0;
// What ResolvePrimitiveRestartEnable answered for the draw this snapshot was taken
// from, i.e. what its pipeline's primitiveRestartEnable was built with. `aspects`
// already separates indexed from non-indexed draws, but not one index TYPE from
// another, and a restart index that fits GL_UNSIGNED_INT but not GL_UNSIGNED_SHORT
// makes those two draws want different pipelines.
Bool primitiveRestartEnable = false;
Uint64 renderPassHash = 0;
Uint32 imageIndex = 0;
Uint64 textureEraseEpoch = 0;
@@ -1168,13 +1180,23 @@ namespace MobileGL::MG_Backend::DirectVulkan {
void CreateSwapchain();
void CreateCommandPool();
// Whether THIS draw's primitive stream restarts, and therefore what
// VkPipelineInputAssemblyStateCreateInfo::primitiveRestartEnable must be. Resolved by the
// caller because it needs two facts a pipeline cannot see: whether the draw is indexed at
// all (GL primitive restart acts on the index stream, so it is a no-op for glDrawArrays),
// and the index TYPE (an application restart index that does not fit the type matches no
// index, so that draw restarts nowhere - see UploadAndBindIndexBuffer).
Bool ResolvePrimitiveRestartEnable(Flags<DrawSetupAspect> aspects,
const IndexBufferView* pIndexBufferView) const;
VkPipeline GetOrCreatePipeline(
GLenum mode,
const MG_State::GLState::ProgramObject& program,
const ProgramFactory::VkProgramObject& programObj,
ProgramFactory::CompileOptionFlags transformFlags,
const MG_State::GLState::VertexArrayObject& vao,
const RenderPassEntry& renderPassEntry);
const RenderPassEntry& renderPassEntry,
Bool primitiveRestartEnable);
VkPipeline GetOrCreateComputePipeline(const ProgramFactory::VkProgramObject& programObj);
void DestroyComputePipelines();
// Takes the frame rather than a command buffer: a first-time storage-usage upgrade has to
MobileGL/MG_IntegrationTest/Scenarios/PrimitiveRestartScenario.cpp
+103
View File
@@ -297,5 +297,108 @@ void main()
<< "the second draw restarted on an index that is not in its data";
}
// A NON-indexed draw has no index stream, so GL primitive restart cannot affect it - and a
// list topology is the shape DirectVulkan has to refuse when the device lacks
// VK_EXT_primitive_topology_list_restart. Deriving the pipeline's primitiveRestartEnable
// from the capability bits alone conflated the two: an application that enables
// GL_PRIMITIVE_RESTART once at init and then draws its UI with glDrawArrays(GL_TRIANGLES)
// had every one of those draws silently dropped on such a device.
TEST_F(PrimitiveRestartScenario, ANonIndexedListTopologyDrawIsUnaffectedByTheCap) {
if (!Ready()) GTEST_SKIP();
glEnable(GL_PRIMITIVE_RESTART);
glPrimitiveRestartIndex(kRestartIndex);
ASSERT_EQ(glGetError(), static_cast<GLenum>(GL_NO_ERROR));
glClearColor(0.0f, 0.0f, 0.0f, 1.0f);
glClear(GL_COLOR_BUFFER_BIT);
// Vertices 0,1,2 are the left triangle; GL_TRIANGLES is a list topology.
glDrawArrays(GL_TRIANGLES, 0, 3);
EXPECT_EQ(glGetError(), static_cast<GLenum>(GL_NO_ERROR));
std::vector<Pixel> pixels(static_cast<std::size_t>(kSurface) * kSurface);
glReadPixels(0, 0, kSurface, kSurface, GL_RGBA, GL_UNSIGNED_BYTE, pixels.data());
EXPECT_TRUE(IsGreen(At(pixels, kLeftX, kLeftY)))
<< "primitive restart has no meaning for glDrawArrays, so the draw must render "
"normally whatever the device supports";
DrainErrors();
}
// GL 4.6 core 10.3.6 compares the fetched index, zero-extended, against the full 32-bit
// PRIMITIVE_RESTART_INDEX. A restart index the index type cannot hold therefore matches
// nothing and the draw restarts NOWHERE - it does not restart on the truncated value, and
// it does not restart on the type's all-ones value either, which is what the driver's own
// fixed-index restart would have done if it had been left enabled.
TEST_F(PrimitiveRestartScenario, ARestartIndexTooLargeForTheIndexTypeRestartsNowhere) {
if (!Ready()) GTEST_SKIP();
// 16-bit indices with a restart index of 0x10007: the low half (7) IS a real index in
// the data, so a truncating comparison would split the strip exactly where a correct
// one leaves it whole.
const GLushort shortIndices[] = {0, 1, 2, static_cast<GLushort>(kRestartIndex), 3, 4, 5};
glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, m_ebo);
glBufferData(GL_ELEMENT_ARRAY_BUFFER, sizeof(shortIndices), shortIndices, GL_STATIC_DRAW);
glEnable(GL_PRIMITIVE_RESTART);
glPrimitiveRestartIndex(0x10000u + kRestartIndex);
ASSERT_EQ(glGetError(), static_cast<GLenum>(GL_NO_ERROR));
glClearColor(0.0f, 0.0f, 0.0f, 1.0f);
glClear(GL_COLOR_BUFFER_BIT);
glDrawElements(GL_TRIANGLE_STRIP, static_cast<GLsizei>(std::size(shortIndices)), GL_UNSIGNED_SHORT,
nullptr);
EXPECT_EQ(glGetError(), static_cast<GLenum>(GL_NO_ERROR));
std::vector<Pixel> pixels(static_cast<std::size_t>(kSurface) * kSurface);
glReadPixels(0, 0, kSurface, kSurface, GL_RGBA, GL_UNSIGNED_BYTE, pixels.data());
EXPECT_TRUE(IsGreen(At(pixels, kLeftX, kLeftY)));
EXPECT_TRUE(IsGreen(At(pixels, kGapX, kGapY)))
<< "no 16-bit index can equal 0x10007, so nothing restarts and the strip is "
"continuous - truncating the restart index to 7 would split it here";
glBufferData(GL_ELEMENT_ARRAY_BUFFER, sizeof(kIndices), kIndices, GL_STATIC_DRAW);
DrainErrors();
}
// The all-ones value of an index type is an ordinary vertex index whenever the array uses
// the type's full range, which is exactly why an application picks an arbitrary restart
// index in the first place. Substituting the sentinel in place would either steal that
// vertex or spuriously restart on it, so the copy widens instead - and the draw has to be
// issued with the widened type, which is the part that is easy to forget.
TEST_F(PrimitiveRestartScenario, AnAllOnesVertexIndexSurvivesTheSubstitution) {
if (!Ready()) GTEST_SKIP();
// The buffer carries the 16-bit all-ones value as an ordinary element. It sits past
// the seven indices this draw reads, because the vertex array has only eight entries
// and fetching index 65535 would be out of range - what is under test is that its
// mere PRESENCE forces the widened copy, and that the draw still finds its own
// indices at the right offsets in a copy whose element width has changed underneath
// it. Narrowly substituting in place instead would rewrite this element to 0xFFFE.
const GLushort shortIndices[] = {0, 1, 2, static_cast<GLushort>(kRestartIndex), 3, 4, 5, 0xFFFFu};
glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, m_ebo);
glBufferData(GL_ELEMENT_ARRAY_BUFFER, sizeof(shortIndices), shortIndices, GL_STATIC_DRAW);
glEnable(GL_PRIMITIVE_RESTART);
glPrimitiveRestartIndex(kRestartIndex);
ASSERT_EQ(glGetError(), static_cast<GLenum>(GL_NO_ERROR));
glClearColor(0.0f, 0.0f, 0.0f, 1.0f);
glClear(GL_COLOR_BUFFER_BIT);
// Only the first seven indices are drawn, so the 0xFFFF element is never fetched - what
// is under test is that its PRESENCE does not break the substitution or the offsets.
glDrawElements(GL_TRIANGLE_STRIP, 7, GL_UNSIGNED_SHORT, nullptr);
EXPECT_EQ(glGetError(), static_cast<GLenum>(GL_NO_ERROR));
std::vector<Pixel> pixels(static_cast<std::size_t>(kSurface) * kSurface);
glReadPixels(0, 0, kSurface, kSurface, GL_RGBA, GL_UNSIGNED_BYTE, pixels.data());
EXPECT_TRUE(IsGreen(At(pixels, kLeftX, kLeftY))) << "the first strip half did not render";
EXPECT_TRUE(IsGreen(At(pixels, kRightX, kRightY))) << "the second strip half did not render";
EXPECT_FALSE(IsGreen(At(pixels, kGapX, kGapY)))
<< "the restart still has to happen once the copy has been widened";
glBufferData(GL_ELEMENT_ARRAY_BUFFER, sizeof(kIndices), kIndices, GL_STATIC_DRAW);
DrainErrors();
}
} // namespace
} // namespace MGITest
MobileGL/MG_Util/Metrics/BufferMetrics.cpp
+9
View File
@@ -10,6 +10,15 @@
namespace MobileGL {
namespace MG_Util {
Uint32 FixedRestartIndexForGLType(GLenum indexType) {
switch (indexType) {
case GL_UNSIGNED_BYTE: return 0xFFu;
case GL_UNSIGNED_SHORT: return 0xFFFFu;
case GL_UNSIGNED_INT: return 0xFFFFFFFFu;
default: return 0;
}
}
SizeT GetGLTypeSize(GLenum type) {
switch (type) {
// Scalars
MobileGL/MG_Util/Metrics/BufferMetrics.h
+14
View File
@@ -12,5 +12,19 @@
namespace MobileGL {
namespace MG_Util {
SizeT GetGLTypeSize(GLenum type);
// The largest value an index of this type can hold, which is also the value
// GL_PRIMITIVE_RESTART_FIXED_INDEX (and its GLES/Vulkan equivalents) restart on. Zero for a
// type that cannot index at all.
//
// Shared rather than re-derived per backend on purpose: three places have to agree about
// what an index of this type can be - whether a rewrite is needed at all, what the rewrite
// compares against, and whether the driver should be told to restart. GL 4.6 core 10.3.6
// compares the FETCHED index, zero-extended, against the full 32-bit
// PRIMITIVE_RESTART_INDEX, so a restart index greater than this value matches no index and
// the draw restarts nowhere. Truncating it to the type's width instead - which one of these
// three places used to do - turns a legal vertex index into a restart.
Uint32 FixedRestartIndexForGLType(GLenum indexType);
} // namespace MG_Util
} // namespace MobileGL