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

[Fix] (Backend): honour desktop GL_PRIMITIVE_RESTART with an arbitrary index instead of throwing through the C GL ABI

Browse Source
This commit is contained in:
Swung0x48
2026-08-27 02:11:18 -04:00
parent 18fccdd796
commit 0f2fcbc469
6 changed files with 646 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files
MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp
+254 -30
View File
@@ -3524,6 +3524,12 @@ namespace MobileGL::MG_Backend::DirectGLES {
const SharedPtr<MG_State::GLState::BufferObject>& drawIndirectBuffer,
GLsizei drawcount, GLsizei stride, const char* label) {
(void)label;
// An indirect command's firstIndex/count live in GPU memory, so the substitution has
// 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.
const ScopedRestartIndexSubstitution restart(type, /*count=*/0, /*indices=*/nullptr);
if (!restart.DrawIsValid()) return;
const Bool useNative = drawIndirectBuffer != nullptr && SupportsNativeIndirectDraws();
if (useNative) {
// gl_BaseInstance must observe GPU-written command fields; expose the indirect
@@ -3829,29 +3835,230 @@ namespace MobileGL::MG_Backend::DirectGLES {
}
}
// GLES core supports only GL_PRIMITIVE_RESTART_FIXED_INDEX (fixed all-ones value). If the app
// enabled the arbitrary GL_PRIMITIVE_RESTART with a non-fixed index, hard-fail at this draw with
// the reason (a fallback would silently drop restarts and corrupt geometry).
void CheckPrimitiveRestartSupported(GLenum indexType) {
// ---------------------------------------------------------------------------
// Arbitrary-index primitive restart
//
// Desktop GL restarts on whatever index glPrimitiveRestartIndex named; GLES core only
// 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).
//
// 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
// already call out - so an application that merely asked for a legal desktop feature
// died rather than got an error.
// ---------------------------------------------------------------------------
namespace {
struct RestartScratchBuffer {
Uint id = 0;
SizeT capacity = 0;
};
RestartScratchBuffer g_restartIndices;
Vector<Uint8> g_restartStaging;
// Past this the rewrite would stage and re-upload hundreds of megabytes on EVERY
// draw (the copy is not memoised, exactly as on the Vulkan side). Decline instead of
// 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) {
switch (indexType) {
case GL_UNSIGNED_BYTE: return 1;
case GL_UNSIGNED_SHORT: return 2;
case GL_UNSIGNED_INT: return 4;
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;
}
}
// 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;
}
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);
}
}
};
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;
}
}
// Whole-buffer respecify through the manager-wide staging target, so binding it
// disturbs no VAO state. glBufferData orphans the previous store, so the upload
// never waits on a draw still reading the old contents out of the same name.
Bool UploadRestartScratch(SizeT bytes, const void* data) {
if (g_restartIndices.id == 0) {
GLuint id = 0;
g_GLESFuncs.glGenBuffers(1, &id);
if (id == 0) return false;
g_restartIndices.id = id;
g_restartIndices.capacity = 0;
}
BufferImpl::BindBufferId(BufferImpl::TempBufferTarget, g_restartIndices.id);
SizeT capacity = g_restartIndices.capacity == 0 ? bytes : g_restartIndices.capacity;
while (capacity < bytes) capacity *= 2;
g_GLESFuncs.glBufferData(BufferImpl::TempBufferTarget, static_cast<GLsizeiptr>(capacity), nullptr,
GL_STREAM_DRAW);
g_restartIndices.capacity = capacity;
if (data != nullptr && bytes != 0) {
g_GLESFuncs.glBufferSubData(BufferImpl::TempBufferTarget, 0, static_cast<GLsizeiptr>(bytes), data);
}
return true;
}
const SharedPtr<MG_State::GLState::BufferObject>& BoundElementArrayBuffer() {
static const SharedPtr<MG_State::GLState::BufferObject> none;
const auto& vao = MG_State::pGLContext->GetBoundVertexArray();
if (!vao) return none;
return vao->GetIndexBufferBindingSlot().GetBoundObject();
}
// The GL name PrepareForDraw left on GL_ELEMENT_ARRAY_BUFFER, i.e. what the
// substitution has to put back.
Uint BoundElementArrayBufferId() {
const auto& ibo = BoundElementArrayBuffer();
if (!ibo) return 0;
const auto* resource = BufferImpl::EnsureBufferResource(ibo);
return resource ? resource->id : 0;
}
} // namespace
Bool NeedsArbitraryRestartSubstitution(GLenum indexType) {
if (!MG_State::pGLContext->IsCapabilityEnabled(CapabilityInput::PrimitiveRestart) ||
MG_State::pGLContext->IsCapabilityEnabled(CapabilityInput::PrimitiveRestartFixedIndex)) {
return false;
}
const Uint32 fixedMax = FixedRestartIndexFor(indexType);
if (fixedMax == 0) return false;
return MG_State::pGLContext->GetPrimitiveRestartIndex() != fixedMax;
}
void OnRestartSubstitutionContextDestroyed() {
g_restartIndices = {};
g_restartStaging.clear();
g_restartStaging.shrink_to_fit();
}
ScopedRestartIndexSubstitution::ScopedRestartIndexSubstitution(GLenum indexType, GLsizei count,
const void* indices)
: m_indices(indices) {
if (!NeedsArbitraryRestartSubstitution(indexType)) {
return;
}
Uint32 fixedMax = 0;
switch (indexType) {
case GL_UNSIGNED_BYTE: fixedMax = 0xFFu; break;
case GL_UNSIGNED_SHORT: fixedMax = 0xFFFFu; break;
case GL_UNSIGNED_INT: fixedMax = 0xFFFFFFFFu; break;
default: return;
const SizeT indexSize = RestartIndexTypeSize(indexType);
const Uint32 applicationRestartIndex = MG_State::pGLContext->GetPrimitiveRestartIndex();
const auto& indexBuffer = BoundElementArrayBuffer();
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.
const SizeT sizeBytes = indexBuffer->GetSize();
if (sizeBytes == 0) {
return; // Nothing to restart on; let the driver see the draw unchanged.
}
if (sizeBytes > kMaxRestartRewriteBytes) {
MGLOG_E_ONCE("Draw skipped: GL_PRIMITIVE_RESTART with restart index %u needs the %zu-byte element "
"array buffer rewritten every draw, which is past the %zu-byte ceiling. Use "
"GL_PRIMITIVE_RESTART_FIXED_INDEX, or set glPrimitiveRestartIndex to the all-ones "
"value of the index type.",
applicationRestartIndex, sizeBytes, kMaxRestartRewriteBytes);
m_valid = false;
return;
}
// The shadow is the source of truth for CPU reads, but a persistent map or a
// shader write may have moved past it since the last sync.
indexBuffer->SyncPersistentMappedRange();
indexBuffer->SyncGpuWrites();
const Uint8* bytes = indexBuffer->MappedData();
if (bytes == 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);
} 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) {
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) {
MGLOG_E_ONCE("Draw skipped: GL_PRIMITIVE_RESTART index rewrite of %zu bytes is past the %zu-byte "
"ceiling.",
sizeBytes, kMaxRestartRewriteBytes);
m_valid = false;
return;
}
RewriteRestartIndices(indices, sizeBytes, indexType, applicationRestartIndex, g_restartStaging);
}
const Uint32 restartIndex = MG_State::pGLContext->GetPrimitiveRestartIndex();
if (restartIndex != fixedMax) {
THROW_EXCEPTION("GL_PRIMITIVE_RESTART with an arbitrary restart index (" + std::to_string(restartIndex) +
") is not supported by the GLES backend, which only restarts on the fixed index value (" +
std::to_string(fixedMax) +
") for this index type; use GL_PRIMITIVE_RESTART_FIXED_INDEX or set glPrimitiveRestartIndex "
"to that value.");
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.");
m_valid = false;
return;
}
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;
}
ScopedRestartIndexSubstitution::~ScopedRestartIndexSubstitution() {
if (!m_substituted) return;
BufferImpl::BindBufferId(GL_ELEMENT_ARRAY_BUFFER, m_previousBinding);
}
void DrawElements(GLenum mode, GLsizei count, GLenum type, const void* indices) {
@@ -3860,9 +4067,10 @@ namespace MobileGL::MG_Backend::DirectGLES {
#endif
DrawSyncFlags syncBit = DrawSyncBit::IndexBuffer;
PrepareForDraw(syncBit);
CheckPrimitiveRestartSupported(type);
const ScopedRestartIndexSubstitution restart(type, count, indices);
if (!restart.DrawIsValid()) return;
ForEachViewportRoutingPass([&] {
g_GLESFuncs.glDrawElements(mode, count, type, indices);
g_GLESFuncs.glDrawElements(mode, count, type, restart.Indices());
});
}
@@ -3890,10 +4098,11 @@ namespace MobileGL::MG_Backend::DirectGLES {
#endif
DrawSyncFlags syncBit = DrawSyncBit::IndexBuffer;
PrepareForDraw(syncBit);
CheckPrimitiveRestartSupported(type);
const ScopedRestartIndexSubstitution restart(type, count, indices);
if (!restart.DrawIsValid()) return;
SetCurrentBaseVertex(basevertex);
ForEachViewportRoutingPass([&] {
g_GLESFuncs.glDrawElementsBaseVertex(mode, count, type, indices, basevertex);
g_GLESFuncs.glDrawElementsBaseVertex(mode, count, type, restart.Indices(), basevertex);
});
SetCurrentBaseVertex(0);
}
@@ -4158,9 +4367,11 @@ namespace MobileGL::MG_Backend::DirectGLES {
const void* indices, GLint basevertex) {
DrawSyncFlags syncBit = DrawSyncBit::IndexBuffer;
PrepareForDraw(syncBit);
const ScopedRestartIndexSubstitution restart(type, count, indices);
if (!restart.DrawIsValid()) return;
SetCurrentBaseVertex(basevertex);
ForEachViewportRoutingPass([&] {
g_GLESFuncs.glDrawRangeElementsBaseVertex(mode, start, end, count, type, indices, basevertex);
g_GLESFuncs.glDrawRangeElementsBaseVertex(mode, start, end, count, type, restart.Indices(), basevertex);
});
SetCurrentBaseVertex(0);
}
@@ -4168,8 +4379,10 @@ namespace MobileGL::MG_Backend::DirectGLES {
void DrawRangeElements(GLenum mode, GLuint start, GLuint end, GLsizei count, GLenum type, const void* indices) {
DrawSyncFlags syncBit = DrawSyncBit::IndexBuffer;
PrepareForDraw(syncBit);
const ScopedRestartIndexSubstitution restart(type, count, indices);
if (!restart.DrawIsValid()) return;
ForEachViewportRoutingPass([&] {
g_GLESFuncs.glDrawRangeElements(mode, start, end, count, type, indices);
g_GLESFuncs.glDrawRangeElements(mode, start, end, count, type, restart.Indices());
});
}
@@ -4191,14 +4404,17 @@ namespace MobileGL::MG_Backend::DirectGLES {
DrawSyncFlags syncBit = DrawSyncBit::IndexBuffer | DrawSyncBit::Instancing;
const VertexArrayImpl::ScopedFetchBaseInstance fetchScope(EmulatedFetchBaseInstance(baseinstance));
PrepareForDraw(syncBit);
const ScopedRestartIndexSubstitution restart(type, count, indices);
if (!restart.DrawIsValid()) return;
SetCurrentBaseInstance(baseinstance);
SetCurrentBaseVertex(basevertex);
ForEachViewportRoutingPass([&] {
if (UseNativeBaseInstance()) {
g_GLESFuncs.glDrawElementsInstancedBaseVertexBaseInstanceEXT(mode, count, type, indices, instancecount,
basevertex, baseinstance);
g_GLESFuncs.glDrawElementsInstancedBaseVertexBaseInstanceEXT(mode, count, type, restart.Indices(),
instancecount, basevertex, baseinstance);
} else {
g_GLESFuncs.glDrawElementsInstancedBaseVertex(mode, count, type, indices, instancecount, basevertex);
g_GLESFuncs.glDrawElementsInstancedBaseVertex(mode, count, type, restart.Indices(), instancecount,
basevertex);
}
});
SetCurrentBaseVertex(0);
@@ -4209,9 +4425,12 @@ namespace MobileGL::MG_Backend::DirectGLES {
GLsizei instancecount, GLint basevertex) {
DrawSyncFlags syncBit = DrawSyncBit::IndexBuffer | DrawSyncBit::Instancing;
PrepareForDraw(syncBit);
const ScopedRestartIndexSubstitution restart(type, count, indices);
if (!restart.DrawIsValid()) return;
SetCurrentBaseVertex(basevertex);
ForEachViewportRoutingPass([&] {
g_GLESFuncs.glDrawElementsInstancedBaseVertex(mode, count, type, indices, instancecount, basevertex);
g_GLESFuncs.glDrawElementsInstancedBaseVertex(mode, count, type, restart.Indices(), instancecount,
basevertex);
});
SetCurrentBaseVertex(0);
}
@@ -4221,13 +4440,15 @@ namespace MobileGL::MG_Backend::DirectGLES {
DrawSyncFlags syncBit = DrawSyncBit::IndexBuffer | DrawSyncBit::Instancing;
const VertexArrayImpl::ScopedFetchBaseInstance fetchScope(EmulatedFetchBaseInstance(baseinstance));
PrepareForDraw(syncBit);
const ScopedRestartIndexSubstitution restart(type, count, indices);
if (!restart.DrawIsValid()) return;
SetCurrentBaseInstance(baseinstance);
ForEachViewportRoutingPass([&] {
if (UseNativeBaseInstance()) {
g_GLESFuncs.glDrawElementsInstancedBaseInstanceEXT(mode, count, type, indices, instancecount,
g_GLESFuncs.glDrawElementsInstancedBaseInstanceEXT(mode, count, type, restart.Indices(), instancecount,
baseinstance);
} else {
g_GLESFuncs.glDrawElementsInstanced(mode, count, type, indices, instancecount);
g_GLESFuncs.glDrawElementsInstanced(mode, count, type, restart.Indices(), instancecount);
}
});
SetCurrentBaseInstance(0);
@@ -4236,8 +4457,10 @@ namespace MobileGL::MG_Backend::DirectGLES {
void DrawElementsInstanced(GLenum mode, GLsizei count, GLenum type, const void* indices, GLsizei instancecount) {
DrawSyncFlags syncBit = DrawSyncBit::IndexBuffer | DrawSyncBit::Instancing;
PrepareForDraw(syncBit);
const ScopedRestartIndexSubstitution restart(type, count, indices);
if (!restart.DrawIsValid()) return;
ForEachViewportRoutingPass([&] {
g_GLESFuncs.glDrawElementsInstanced(mode, count, type, indices, instancecount);
g_GLESFuncs.glDrawElementsInstanced(mode, count, type, restart.Indices(), instancecount);
});
}
@@ -9996,6 +10219,7 @@ namespace MobileGL::MG_Backend::DirectGLES {
BufferImpl::OnBackendContextDestroyed();
XfbImpl::OnBackendContextDestroyed();
MultiDrawImpl::OnBackendContextDestroyed();
OnRestartSubstitutionContextDestroyed();
ScratchFBOImpl::OnBackendContextDestroyed();
ReleasePackedWordScratchTexture();
FramebufferImpl::InvalidateFramebufferBindingCache();
MobileGL/MG_Backend/DirectGLES/Managers.h
+46 -3
View File
@@ -102,9 +102,52 @@ 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);
// GLES core supports only GL_PRIMITIVE_RESTART_FIXED_INDEX. Throws when the app enabled
// the arbitrary GL_PRIMITIVE_RESTART with a non-fixed index for this index type.
void CheckPrimitiveRestartSupported(GLenum indexType);
// 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);
// 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.
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.
ScopedRestartIndexSubstitution(GLenum indexType, GLsizei count, const void* indices);
~ScopedRestartIndexSubstitution();
ScopedRestartIndexSubstitution(const ScopedRestartIndexSubstitution&) = delete;
ScopedRestartIndexSubstitution& operator=(const ScopedRestartIndexSubstitution&) = delete;
// False only when a substitution was needed and could not be made. The draw must
// then be skipped: issuing it would let the driver silently drop every restart and
// weld the primitives on either side together, which is worse than drawing nothing.
Bool DrawIsValid() const { return m_valid; }
// 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; }
private:
const void* m_indices = nullptr;
Uint m_previousBinding = 0;
Bool m_substituted = false;
Bool m_valid = true;
};
// Drops the scratch element array buffer the substitution above stages through. Like
// MultiDrawImpl's scratch names it is abandoned rather than deleted: the name belongs to
// the dead ES context, and deleting it would target whatever its successor handed out.
void OnRestartSubstitutionContextDestroyed();
// Feed the current program's gl_BaseInstance / gl_DrawID / gl_BaseVertex emulation
// uniforms. All are no-ops when the program does not read the corresponding builtin.
void SetCurrentBaseInstance(Uint32 baseInstance);
MobileGL/MG_Backend/DirectGLES/MultiDraw.cpp
+29 -10
View File
@@ -29,11 +29,15 @@ namespace MobileGL::MG_Backend::DirectGLES::MultiDrawImpl {
}
}
// The all-ones value of an index type, which is what GL restarts on once
// primitive restart is in play. CheckPrimitiveRestartSupported has already
// rejected the arbitrary-index form of GL_PRIMITIVE_RESTART, so an enabled
// restart always restarts here and nowhere else.
// 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.
Uint32 RestartSentinelFor(GLenum type) {
if (NeedsArbitraryRestartSubstitution(type)) {
return MG_State::pGLContext->GetPrimitiveRestartIndex();
}
switch (type) {
case GL_UNSIGNED_BYTE: return 0xFFu;
case GL_UNSIGNED_SHORT: return 0xFFFFu;
@@ -275,10 +279,20 @@ namespace MobileGL::MG_Backend::DirectGLES::MultiDrawImpl {
// its remaining feasibility checks inside its implementation, where the data it
// has to walk is already in hand.
GLESMultiDrawMode ResolveTierForBatch(Bool programReadsDrawID, Bool perSubDrawBaseVertex,
Bool hasIndexBuffer) {
Bool hasIndexBuffer, Bool arbitraryRestart) {
ResolveTierOnce();
GLESMultiDrawMode tier = g_resolvedTier;
// Desktop GL_PRIMITIVE_RESTART restarts on an application-chosen index; the driver
// only ever restarts on the all-ones value. Every tier but the rebased one hands
// the application's own index data to the driver, which would then see no restarts
// at all and weld the primitives together. The rebased tier is the one that
// REWRITES the stream, and RestartSentinelFor already tells it which value to
// translate, so it is the only tier this batch can take.
if (arbitraryRestart) {
return GLESMultiDrawMode::DrawElements;
}
// Batched tiers issue one driver entry for the whole batch, so the emulated
// gl_DrawID uniform can only hold one value across every sub-draw. A program
// that reads gl_DrawID gets an unrolled tier, which feeds each sub-draw its
@@ -852,8 +866,10 @@ void main() {
void DrawElementsBatch(GLenum mode, const GLsizei* count, GLenum type, const GLvoid* const* indices,
GLsizei drawcount, const GLint* basevertex) {
if (drawcount <= 0 || !count || !indices) return;
// State-independent and possibly throwing, so it runs before any GL work.
CheckPrimitiveRestartSupported(type);
// 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);
const Bool hasIndexBuffer = BoundIndexBuffer() != nullptr;
@@ -889,7 +905,8 @@ void main() {
// the tier choice and the per-sub-draw feeds use those, not the guess above.
const Bool feedDrawID = CurrentProgramReadsDrawID();
const Bool feedBaseVertex = basevertex != nullptr && CurrentProgramReadsBaseVertex();
const GLESMultiDrawMode tier = ResolveTierForBatch(feedDrawID, feedBaseVertex, hasIndexBuffer);
const GLESMultiDrawMode tier =
ResolveTierForBatch(feedDrawID, feedBaseVertex, hasIndexBuffer, arbitraryRestart);
Bool drawn = false;
switch (tier) {
@@ -921,8 +938,10 @@ void main() {
// Every tier above may decline a batch whose shape it cannot express. The two
// below are the floor: a base-vertex replay where the driver has one, and the
// rewritten index stream where it does not. Both are safe for any batch these
// entry points can receive.
if (!drawn) {
// entry points can receive - except that the base-vertex replay hands the
// application's own indices to the driver, which cannot restart on a desktop
// restart index, so that batch has only the rewriting floor.
if (!drawn && !arbitraryRestart) {
drawn = RunBaseVertexLoop(mode, count, type, indices, drawcount, basevertex, feedDrawID, feedBaseVertex);
}
if (!drawn) {
MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp
+15 -5
View File
@@ -5029,8 +5029,15 @@ void main() {
MG_State::pGLContext->IsCapabilityEnabled(CapabilityInput::PrimitiveRestart) ||
MG_State::pGLContext->IsCapabilityEnabled(CapabilityInput::PrimitiveRestartFixedIndex);
// Primitive restart on a *list* topology requires the primitiveTopologyListRestart feature;
// strip/fan restart works without it. Silently dropping restarts would corrupt geometry, so
// hard-fail here (at the draw) with the reason when the device lacks the 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
// here with the reason.
//
// Declined, not thrown. This used to THROW_EXCEPTION, which unwinds a C++ exception through
// the C GL ABI and takes the process down (the hazard GL_Texture.cpp and RenderState.cpp
// 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.
const auto isListTopology = [](VkPrimitiveTopology t) {
return t == VK_PRIMITIVE_TOPOLOGY_POINT_LIST || t == VK_PRIMITIVE_TOPOLOGY_LINE_LIST ||
t == VK_PRIMITIVE_TOPOLOGY_TRIANGLE_LIST ||
@@ -5038,9 +5045,12 @@ void main() {
t == VK_PRIMITIVE_TOPOLOGY_TRIANGLE_LIST_WITH_ADJACENCY || t == VK_PRIMITIVE_TOPOLOGY_PATCH_LIST;
};
if (primitiveRestartEnabled && !m_primitiveTopologyListRestartFeatureEnabled && isListTopology(vkTopology)) {
THROW_EXCEPTION("Primitive restart on a list topology requires the primitiveTopologyListRestart device "
"feature (VK_EXT_primitive_topology_list_restart), which this device does not support; use "
"a strip/fan topology or a device that supports it.");
MGLOG_E_ONCE("Draw skipped: primitive restart on a list topology (0x%x) requires the "
"primitiveTopologyListRestart device feature (VK_EXT_primitive_topology_list_restart), "
"which this device does not support; use a strip/fan topology, or disable primitive "
"restart for list-topology draws.",
mode);
return VK_NULL_HANDLE;
}
PipelineFactory::PipelineCreatePayload payload {
MobileGL/MG_IntegrationTest/CMakeLists.txt
+1
View File
@@ -63,6 +63,7 @@ add_executable(MobileGLIntegrationTest
Scenarios/PipelineFailureScenario.cpp
Scenarios/AdvertisedLimitsScenario.cpp
Scenarios/PixelStoreSweepScenario.cpp
Scenarios/PrimitiveRestartScenario.cpp
Scenarios/FragCoordOriginScenario.cpp
Scenarios/ClearThenReadPixelsScenario.cpp
Scenarios/DepthStencilReadbackScenario.cpp
MobileGL/MG_IntegrationTest/Scenarios/PrimitiveRestartScenario.cpp
+301
View File
@@ -0,0 +1,301 @@
// MobileGL - MobileGL/MG_IntegrationTest/Scenarios/PrimitiveRestartScenario.cpp
// Copyright (c) 2025-2026 MobileGL-Dev
// Licensed under the GNU Lesser General Public License v3.0:
// https://www.gnu.org/licenses/gpl-3.0.txt
// https://www.gnu.org/licenses/lgpl-3.0.txt
// SPDX-License-Identifier: LGPL-3.0-only
// End of Source File Header
//
// Scenario - DESKTOP GL_PRIMITIVE_RESTART WITH AN APPLICATION-CHOSEN INDEX.
//
// Desktop GL restarts on whatever glPrimitiveRestartIndex named; GLES and Vulkan both restart
// only on the all-ones value of the index type. DirectGLES used to THROW_EXCEPTION on the
// mismatch, and a throw out of a GL entry point unwinds a C++ exception through the C ABI and
// kills the process - which is how KHR-GL4x.geometry_shader.primitive_counter.*_rp took the whole
// conformance runner down, nine bodies at a time, losing every result in the chunk with it.
//
// So the first thing this asserts is simply that the process is still here. The second is that
// the restart actually happened: the substitution rewrites the index data so the driver restarts
// where the application asked, and the difference between "restart honoured" and "restart
// silently dropped" is a triangle strip that welds its two halves together across the gap.
//
// Needs a real context on purpose. The GPU-free suite cannot reach a backend at all, and this is
// entirely about what the backend does with the index buffer.
#include <cstddef>
#include <iterator>
#include <string>
#include <utility>
#include <vector>
#include "../Harness/HeadlessGL.h"
#include "../Harness/ScenarioFixture.h"
#ifdef GLAPI
#undef GLAPI
#endif
#define GL_GLEXT_PROTOTYPES
#include <GL/gl.h>
#include <GL/glcorearb.h>
#undef GL_GLEXT_PROTOTYPES
namespace MGITest {
namespace {
constexpr GLsizei kSurface = 64;
const char* const kVertexSource = R"(#version 420 core
layout(location = 0) in vec2 a_position;
void main()
{
gl_Position = vec4(a_position, 0.0, 1.0);
}
)";
const char* const kFragmentSource = R"(#version 420 core
out vec4 fragColor;
void main()
{
fragColor = vec4(0.0, 1.0, 0.0, 1.0);
}
)";
// Two triangles with a gap down the middle, plus two spare vertices parked at the origin.
//
// The spares exist so the restart index is a LEGAL vertex index: if the restart were
// dropped the driver would still fetch a real vertex rather than read out of bounds, so
// the negative case is defined behaviour and the test measures the restart rather than
// whatever robust-buffer-access does.
constexpr GLfloat kVertices[] = {
-0.9f, -0.9f, // 0 - left triangle
-0.1f, -0.9f, // 1
-0.9f, 0.9f, // 2
0.1f, -0.9f, // 3 - right triangle
0.9f, -0.9f, // 4
0.9f, 0.9f, // 5
0.0f, 0.0f, // 6 - spare
0.0f, 0.0f, // 7 - spare, and the application's restart index
};
constexpr GLuint kRestartIndex = 7;
// A triangle STRIP, restarted in the middle: honoured, it is exactly the two triangles
// above. Dropped, the strip welds vertices 2, 7 and 3 into extra triangles that spill
// across the gap - which is what the middle probe below catches.
constexpr GLuint kIndices[] = {0, 1, 2, kRestartIndex, 3, 4, 5};
struct Pixel {
GLubyte r = 0, g = 0, b = 0, a = 0;
};
class PrimitiveRestartScenario : public ScenarioTest {
protected:
void SetUp() override {
ScenarioTest::SetUp();
if (!Ready()) return;
glGenVertexArrays(1, &m_vao);
glBindVertexArray(m_vao);
glGenBuffers(1, &m_vbo);
glBindBuffer(GL_ARRAY_BUFFER, m_vbo);
glBufferData(GL_ARRAY_BUFFER, sizeof(kVertices), kVertices, GL_STATIC_DRAW);
glVertexAttribPointer(0, 2, GL_FLOAT, GL_FALSE, 2 * sizeof(GLfloat), nullptr);
glEnableVertexAttribArray(0);
glGenBuffers(1, &m_ebo);
glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, m_ebo);
glBufferData(GL_ELEMENT_ARRAY_BUFFER, sizeof(kIndices), kIndices, GL_STATIC_DRAW);
glGenTextures(1, &m_colorTexture);
glBindTexture(GL_TEXTURE_2D, m_colorTexture);
glTexStorage2D(GL_TEXTURE_2D, 1, GL_RGBA8, kSurface, kSurface);
glGenFramebuffers(1, &m_fbo);
glBindFramebuffer(GL_FRAMEBUFFER, m_fbo);
glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, m_colorTexture, 0);
ASSERT_EQ(glCheckFramebufferStatus(GL_FRAMEBUFFER),
static_cast<GLenum>(GL_FRAMEBUFFER_COMPLETE));
glViewport(0, 0, kSurface, kSurface);
m_program = BuildProgram();
ASSERT_NE(m_program, 0u) << "the flat-colour program did not build: " << m_buildLog;
glUseProgram(m_program);
DrainErrors();
}
void TearDown() override {
if (!Ready()) return;
glDisable(GL_PRIMITIVE_RESTART);
glDisable(GL_PRIMITIVE_RESTART_FIXED_INDEX);
glPrimitiveRestartIndex(0);
glUseProgram(0);
if (m_program != 0) glDeleteProgram(m_program);
glBindFramebuffer(GL_FRAMEBUFFER, 0);
if (m_fbo != 0) glDeleteFramebuffers(1, &m_fbo);
if (m_colorTexture != 0) glDeleteTextures(1, &m_colorTexture);
glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, 0);
glBindBuffer(GL_ARRAY_BUFFER, 0);
if (m_ebo != 0) glDeleteBuffers(1, &m_ebo);
if (m_vbo != 0) glDeleteBuffers(1, &m_vbo);
glBindVertexArray(0);
if (m_vao != 0) glDeleteVertexArrays(1, &m_vao);
DrainErrors();
}
static void DrainErrors() {
for (int i = 0; i < 16 && glGetError() != GL_NO_ERROR; ++i) {
}
}
GLuint BuildProgram() {
const GLuint vs = glCreateShader(GL_VERTEX_SHADER);
glShaderSource(vs, 1, &kVertexSource, nullptr);
glCompileShader(vs);
const GLuint fs = glCreateShader(GL_FRAGMENT_SHADER);
glShaderSource(fs, 1, &kFragmentSource, nullptr);
glCompileShader(fs);
const GLuint program = glCreateProgram();
glAttachShader(program, vs);
glAttachShader(program, fs);
glLinkProgram(program);
GLint linked = 0;
glGetProgramiv(program, GL_LINK_STATUS, &linked);
glDeleteShader(vs);
glDeleteShader(fs);
if (!linked) {
GLint length = 0;
glGetProgramiv(program, GL_INFO_LOG_LENGTH, &length);
std::vector<char> buffer(static_cast<std::size_t>(length) + 1, '\0');
glGetProgramInfoLog(program, length + 1, nullptr, buffer.data());
m_buildLog = buffer.data();
glDeleteProgram(program);
return 0;
}
return program;
}
// The whole surface, so a failure can report the three probes together rather than
// three separate readbacks that might disagree about which draw they saw.
std::vector<Pixel> DrawAndRead() {
glClearColor(0.0f, 0.0f, 0.0f, 1.0f);
glClear(GL_COLOR_BUFFER_BIT);
glDrawElements(GL_TRIANGLE_STRIP, static_cast<GLsizei>(std::size(kIndices)), GL_UNSIGNED_INT,
nullptr);
std::vector<Pixel> pixels(static_cast<std::size_t>(kSurface) * kSurface);
glReadPixels(0, 0, kSurface, kSurface, GL_RGBA, GL_UNSIGNED_BYTE, pixels.data());
return pixels;
}
static const Pixel& At(const std::vector<Pixel>& pixels, int x, int y) {
return pixels[static_cast<std::size_t>(y) * kSurface + x];
}
static bool IsGreen(const Pixel& p) { return p.g > 128 && p.r < 128; }
// NDC (-0.5, -0.5): well inside the left triangle whichever way the restart went.
static constexpr int kLeftX = 16, kLeftY = 16;
// NDC (0.6, -0.5): well inside the right triangle, and outside every welded one.
static constexpr int kRightX = 51, kRightY = 16;
// NDC (0.2, -0.5): in the gap between the two triangles, and INSIDE the triangle the
// strip welds out of vertices 7, 3 and 4 when the restart is dropped. This is the
// probe that distinguishes a working restart from a silently ignored one.
static constexpr int kGapX = 38, kGapY = 16;
GLuint m_vao = 0;
GLuint m_vbo = 0;
GLuint m_ebo = 0;
GLuint m_fbo = 0;
GLuint m_colorTexture = 0;
GLuint m_program = 0;
std::string m_buildLog;
};
// THE crash regression. Before the fix this call never returned: DirectGLES threw
// std::runtime_error out of glDrawElements and the process died on the spot. Reaching the
// assertion at all is most of the point.
TEST_F(PrimitiveRestartScenario, AnArbitraryRestartIndexDrawsInsteadOfKillingTheProcess) {
if (!Ready()) GTEST_SKIP();
glEnable(GL_PRIMITIVE_RESTART);
glPrimitiveRestartIndex(kRestartIndex);
ASSERT_EQ(glGetError(), static_cast<GLenum>(GL_NO_ERROR));
const std::vector<Pixel> pixels = DrawAndRead();
EXPECT_EQ(glGetError(), static_cast<GLenum>(GL_NO_ERROR))
<< "an arbitrary restart index is legal desktop GL and must raise no error";
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 gap between the two halves is covered, so the restart was dropped and the "
"strip welded across it";
}
// The other half of the state: an application that sets the restart index TO the fixed
// all-ones value needs no rewriting at all, and the cap must map straight onto the
// driver's own fixed-index restart. Same picture, different path through the backend.
TEST_F(PrimitiveRestartScenario, TheFixedIndexValueTakesTheForwardingPath) {
if (!Ready()) GTEST_SKIP();
// Index 0xFFFFFFFF is not a vertex this draw uses, so the strip is the same shape.
const GLuint fixedIndices[] = {0, 1, 2, 0xFFFFFFFFu, 3, 4, 5};
glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, m_ebo);
glBufferSubData(GL_ELEMENT_ARRAY_BUFFER, 0, sizeof(fixedIndices), fixedIndices);
glEnable(GL_PRIMITIVE_RESTART);
glPrimitiveRestartIndex(0xFFFFFFFFu);
ASSERT_EQ(glGetError(), static_cast<GLenum>(GL_NO_ERROR));
const std::vector<Pixel> pixels = DrawAndRead();
EXPECT_EQ(glGetError(), static_cast<GLenum>(GL_NO_ERROR));
EXPECT_TRUE(IsGreen(At(pixels, kLeftX, kLeftY)));
EXPECT_TRUE(IsGreen(At(pixels, kRightX, kRightY)));
EXPECT_FALSE(IsGreen(At(pixels, kGapX, kGapY)));
// Put the buffer back for whatever runs next in this fixture.
glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, m_ebo);
glBufferSubData(GL_ELEMENT_ARRAY_BUFFER, 0, sizeof(kIndices), kIndices);
DrainErrors();
}
// With the cap off, the same index data is just data - nothing restarts, and the strip
// welds across the gap. The negative control for the probe above: without it, a backend
// that lost the whole draw would pass the test by rendering nothing in the gap.
TEST_F(PrimitiveRestartScenario, WithoutTheCapTheStripWeldsAcrossTheGap) {
if (!Ready()) GTEST_SKIP();
glDisable(GL_PRIMITIVE_RESTART);
glPrimitiveRestartIndex(kRestartIndex);
ASSERT_EQ(glGetError(), static_cast<GLenum>(GL_NO_ERROR));
const std::vector<Pixel> pixels = DrawAndRead();
EXPECT_EQ(glGetError(), static_cast<GLenum>(GL_NO_ERROR));
EXPECT_TRUE(IsGreen(At(pixels, kLeftX, kLeftY))) << "the draw itself must still happen";
EXPECT_TRUE(IsGreen(At(pixels, kGapX, kGapY)))
<< "with restart disabled the strip is continuous, so the gap must be covered - if "
"it is not, the probe above proves nothing";
}
// A second draw with a DIFFERENT restart index has to be rewritten again. The substitution
// stages through one scratch buffer, so a cached or half-restored element-array binding
// would show up here as the second draw reusing the first one's data.
TEST_F(PrimitiveRestartScenario, ChangingTheRestartIndexBetweenDrawsIsHonoured) {
if (!Ready()) GTEST_SKIP();
glEnable(GL_PRIMITIVE_RESTART);
glPrimitiveRestartIndex(kRestartIndex);
const std::vector<Pixel> restarted = DrawAndRead();
ASSERT_EQ(glGetError(), static_cast<GLenum>(GL_NO_ERROR));
EXPECT_FALSE(IsGreen(At(restarted, kGapX, kGapY)));
// 6 is the other spare vertex, and it appears nowhere in the index data - so nothing
// restarts and the strip is continuous again, from the very same buffer.
glPrimitiveRestartIndex(6);
const std::vector<Pixel> notRestarted = DrawAndRead();
ASSERT_EQ(glGetError(), static_cast<GLenum>(GL_NO_ERROR));
EXPECT_TRUE(IsGreen(At(notRestarted, kLeftX, kLeftY)));
EXPECT_TRUE(IsGreen(At(notRestarted, kGapX, kGapY)))
<< "the second draw restarted on an index that is not in its data";
}
} // namespace
} // namespace MGITest