swung0x48/MobileGL
1
0
Fork 0
mirror of https://github.com/MobileGL-Dev/MobileGL synced 2026-09-11 05:38:31 +09:00
Code Issues Packages Projects Releases Wiki Activity

[Fix] (Getter): close the review findings - six-stage combined uniform blocks, bounded uniform-block bindings, honest vertex-stream count, per-format sample ceilings

Browse Source
This commit is contained in:
swung0x48
2026-08-27 03:54:16 -04:00
parent 6cc9faf772
commit 06744fde7f
15 changed files with 640 additions and 178 deletions
Download Patch File Download Diff File Expand all files Collapse all files
MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp
+6 -7
View File
@@ -1577,10 +1577,10 @@ namespace MobileGL::MG_Impl::GLImpl {
std::to_string(id) + " is not a transform feedback object name."));
return;
}
// GL 4.6 core 10.3.7 bounds `stream` by GL_MAX_VERTEX_STREAMS, which is 4. Only stream 0
// can ever have been written - nothing in the shader pipeline supports
// layout(stream = N) - so a higher stream captured zero vertices and the draw is a legal
// no-op rather than an error. The bound is read from the getter so the two cannot drift.
// GL 4.6 core 10.3.7 bounds `stream` by GL_MAX_VERTEX_STREAMS, which this implementation
// answers as 1 - so stream 0 is the only one that exists and anything else is
// INVALID_VALUE. Read from the getter rather than written as `stream != 0` so the two can
// never drift: if vertex-stream support ever lands, this bound moves with the limit.
GLint maxVertexStreams = 1;
GetIntegerv(GL_MAX_VERTEX_STREAMS, &maxVertexStreams);
if (stream >= static_cast<GLuint>(std::max(maxVertexStreams, 1))) {
@@ -1601,9 +1601,8 @@ namespace MobileGL::MG_Impl::GLImpl {
return;
}
// Only stream 0 ever records anything (see the stream bound above), so a higher stream
// replays nothing.
const Uint64 vertices = stream == 0 ? MG_State::pGLContext->GetTransformFeedbackRecordedVertices(id) : 0;
// `stream` is provably 0 here (the bound above is 1), so this is stream 0's record.
const Uint64 vertices = MG_State::pGLContext->GetTransformFeedbackRecordedVertices(id);
if (vertices == 0) return;
const auto count = static_cast<GLsizei>(vertices);
AccountTransformFeedbackPrimitives(mode, count);
MobileGL/MG_Impl/GLImpl/Framebuffer/GL_Framebuffer.cpp
+35 -2
View File
@@ -693,6 +693,24 @@ namespace MobileGL::MG_Impl::GLImpl {
// frontend would accept a count it had advertised globally - but on Adreno and Mali the
// integer path is genuinely one sample, and accepting four only moved the failure from an
// honest INVALID_OPERATION here to a silently under-allocated renderbuffer.
// The head of the per-format renderbuffer sample list the backend probed, or 0 when nothing
// was probed for it. Same shape as GetProbedMaxTextureSamples in GL_Texture.cpp, and reads
// the same cache glGetInternalformativ(GL_RENDERBUFFER, ..., GL_SAMPLES) answers from.
static Int GetProbedMaxRenderbufferSamples(TextureInternalFormat format) {
if (MG_Backend::pActiveBackendObject == nullptr) {
return 0;
}
const SizeT targetIndex = MG_Backend::GetRenderbufferFormatCapabilityTargetIndex();
const SizeT formatIndex = static_cast<SizeT>(format);
if (targetIndex >= MG_Backend::kFormatCapabilityTargetCount ||
formatIndex >= MG_Backend::kFormatCapabilityFormatCount) {
return 0;
}
const auto& sampleCounts =
MG_Backend::pActiveBackendObject->GetFormatCapabilities().SampleCounts[targetIndex][formatIndex];
return sampleCounts.empty() ? 0 : sampleCounts.front();
}
Int GetMaxRenderbufferSamplesForFormat_State(TextureInternalFormat format) {
if (MG_Backend::pActiveBackendObject == nullptr) {
return std::numeric_limits<Int>::max();
@@ -707,6 +725,19 @@ namespace MobileGL::MG_Impl::GLImpl {
&normalizedType);
const Bool isIntegerFormat = normalizedFormat == GL_RED_INTEGER || normalizedFormat == GL_RG_INTEGER ||
normalizedFormat == GL_RGB_INTEGER || normalizedFormat == GL_RGBA_INTEGER;
// The per-format probe first, for the same reason the texture path takes it first: GL 4.6
// core 9.2.4 words the error as "samples is greater than the maximum number of samples
// supported for internalformat (see GetInternalformativ)", and
// glGetInternalformativ(GL_RENDERBUFFER, ..., GL_SAMPLES) is answered from exactly this
// list. It was never consulted here - the TODO that deferred it was written before the
// query was backed and had gone stale - so a format whose multisample probes fail inside
// a category that allows four was accepted at four, quietly allocated at one by
// ClampSamplesToBackendSupport, and then reported as four by
// glGetRenderbufferParameteriv(GL_RENDERBUFFER_SAMPLES).
const Int probedMaxSamples = GetProbedMaxRenderbufferSamples(format);
if (probedMaxSamples > 0) {
return probedMaxSamples;
}
if (!isIntegerFormat) {
return GetMaxRenderbufferSamples_State();
}
@@ -743,8 +774,10 @@ namespace MobileGL::MG_Impl::GLImpl {
return false;
}
// TODO: Resolve the remaining per-internalformat renderbuffer sample limits once
// glGetInternalformativ is backed; integer formats are handled below.
// Per-internalformat, from the probe list glGetInternalformativ answers with, falling back
// to the format's category pname where nothing was probed. (This carried a TODO deferring
// the per-format resolution "once glGetInternalformativ is backed"; it has been backed for
// both renderbuffers and multisample textures since, so the deferral was collected.)
const Int maxSamples = GetMaxRenderbufferSamplesForFormat_State(format);
if (samples > maxSamples) {
// GL 4.6 core 9.2.4 makes asking for more samples than the format supports
MobileGL/MG_Impl/GLImpl/Getter/GL_Getter.cpp
+46 -21
View File
@@ -119,13 +119,24 @@ namespace MobileGL::MG_Impl::GLImpl {
constexpr GLint kFrontendMaxGeometryShaderInvocations = 32;
constexpr GLint kFrontendMaxTessControlUniformBlocks = 14;
constexpr GLint kFrontendMaxTessEvaluationUniformBlocks = 14;
// GL 4.6's minimum is 14 uniform blocks on each of the FIVE graphics stages (70), not
// three: the two tessellation stages were simply missing from this sum, so even a
// frontend with enough binding points advertised 42.
// The compute stage's share of the combined sum below. Compute's own per-stage answer is
// backend-derived (GL_MAX_COMPUTE_UNIFORM_BLOCKS reads dynamicParameters), so this is not
// what that query returns - it is the GL 4.3 core minimum, present here only so the
// combined total covers all SIX stages.
constexpr GLint kFrontendMaxComputeUniformBlocksShare = 14;
// GL 4.6 table 23.64 orders MAX_UNIFORM_BUFFER_BINDINGS >= MAX_COMBINED_UNIFORM_BLOCKS >=
// every per-stage count, and the sum has to run over SIX stages, not three and not five.
// Three (42) was the original bug. Five (70) replaced it and broke the middle term the
// other way: compute's per-stage count is backend-derived and clamps at the binding count,
// so a device reporting descriptor-indexing-scale uniform buffers (Adreno reports
// maxPerStageDescriptorUniformBuffers = 16777216) advertised 84 compute blocks against a
// combined 70. Six stages x 14 = 84, which is also exactly the binding-point count and the
// arithmetic the GL 4.5 minimum of 84 bindings is built from, so the ordering is now tight
// rather than accidental.
constexpr GLint kFrontendMaxCombinedUniformBlocks =
kFrontendMaxVertexUniformBlocks + kFrontendMaxTessControlUniformBlocks +
kFrontendMaxTessEvaluationUniformBlocks + kFrontendMaxGeometryUniformBlocks +
kFrontendMaxFragmentUniformBlocks;
kFrontendMaxFragmentUniformBlocks + kFrontendMaxComputeUniformBlocksShare;
constexpr GLint kFrontendMaxVaryingComponents =
static_cast<GLint>(MG_Util::ShaderTranspiler::MAX_VARYING_COMPONENTS);
constexpr GLint kFrontendMaxVaryingVectors =
@@ -135,9 +146,9 @@ namespace MobileGL::MG_Impl::GLImpl {
constexpr GLint kFrontendMaxTransformFeedbackInterleavedComponents = 64;
constexpr GLint kFrontendMaxTransformFeedbackSeparateAttribs = 4;
constexpr GLint kFrontendMaxTransformFeedbackSeparateComponents = 4;
// ARB_transform_feedback3's vertex-stream count; see the GL_MAX_VERTEX_STREAMS case for
// what streams 1..3 mean in an implementation that can only emit to stream 0.
constexpr GLint kFrontendMaxVertexStreams = 4;
// ARB_transform_feedback3's vertex-stream count. One is what this implementation can
// actually emit to; see the GL_MAX_VERTEX_STREAMS case for why it is not four.
constexpr GLint kFrontendMaxVertexStreams = 1;
constexpr GLint kFrontendMaxGeometryOutputVertices = 256;
constexpr GLint kFrontendMaxGeometryTotalOutputComponents = 1024;
// GL 4.5 core table 23.64 requires 84 indexed uniform binding points, and that is exactly
@@ -2366,8 +2377,12 @@ namespace MobileGL::MG_Impl::GLImpl {
*params = dynamicParameters.MaxComputeTextureImageUnits;
break;
case GL_MAX_COMBINED_COMPUTE_UNIFORM_COMPONENTS:
// The CLAMPED block count, i.e. exactly what GL_MAX_COMPUTE_UNIFORM_BLOCKS answers.
// GL 4.6 table 23.64 defines this as the components reachable through the blocks a
// stage may declare, so deriving it from the raw backend number described 256 blocks
// an application is only ever allowed 84 of.
*params = GetMaxCombinedUniformComponents(kFrontendMaxComputeUniformComponents,
dynamicParameters.MaxComputeUniformBlocks,
ClampUniformBlockCount(dynamicParameters.MaxComputeUniformBlocks),
dynamicParameters.MaxUniformBlockSize);
break;
case GL_MAX_COMPUTE_WORK_GROUP_INVOCATIONS:
@@ -2415,12 +2430,12 @@ namespace MobileGL::MG_Impl::GLImpl {
break;
case GL_MAX_COMBINED_FRAGMENT_UNIFORM_COMPONENTS:
*params = GetMaxCombinedUniformComponents(kFrontendMaxFragmentUniformComponents,
kFrontendMaxFragmentUniformBlocks,
ClampUniformBlockCount(kFrontendMaxFragmentUniformBlocks),
dynamicParameters.MaxUniformBlockSize);
break;
case GL_MAX_COMBINED_GEOMETRY_UNIFORM_COMPONENTS:
*params = GetMaxCombinedUniformComponents(kFrontendMaxGeometryUniformComponents,
kFrontendMaxGeometryUniformBlocks,
ClampUniformBlockCount(kFrontendMaxGeometryUniformBlocks),
dynamicParameters.MaxUniformBlockSize);
break;
case GL_MAX_GEOMETRY_OUTPUT_VERTICES:
@@ -2434,7 +2449,7 @@ namespace MobileGL::MG_Impl::GLImpl {
break;
case GL_MAX_COMBINED_VERTEX_UNIFORM_COMPONENTS:
*params = GetMaxCombinedUniformComponents(kFrontendMaxVertexUniformComponents,
kFrontendMaxVertexUniformBlocks,
ClampUniformBlockCount(kFrontendMaxVertexUniformBlocks),
dynamicParameters.MaxUniformBlockSize);
break;
case GL_MAX_CUBE_MAP_TEXTURE_SIZE:
@@ -2511,12 +2526,12 @@ namespace MobileGL::MG_Impl::GLImpl {
case GL_MAX_COMBINED_TESS_CONTROL_UNIFORM_COMPONENTS:
*params = GetMaxCombinedUniformComponents(
static_cast<GLint>(MG_Util::ShaderTranspiler::MAX_TESS_CONTROL_UNIFORM_COMPONENTS),
kFrontendMaxTessControlUniformBlocks, dynamicParameters.MaxUniformBlockSize);
ClampUniformBlockCount(kFrontendMaxTessControlUniformBlocks), dynamicParameters.MaxUniformBlockSize);
break;
case GL_MAX_COMBINED_TESS_EVALUATION_UNIFORM_COMPONENTS:
*params = GetMaxCombinedUniformComponents(
static_cast<GLint>(MG_Util::ShaderTranspiler::MAX_TESS_EVALUATION_UNIFORM_COMPONENTS),
kFrontendMaxTessEvaluationUniformBlocks, dynamicParameters.MaxUniformBlockSize);
ClampUniformBlockCount(kFrontendMaxTessEvaluationUniformBlocks), dynamicParameters.MaxUniformBlockSize);
break;
// ARB_cull_distance. Backend-derived exactly like GL_MAX_CLIP_DISTANCES beside it, and
// for a stronger reason: a cull distance discards the whole primitive, so advertising
@@ -2579,14 +2594,24 @@ namespace MobileGL::MG_Impl::GLImpl {
*params = kFrontendMaxTransformFeedbackSeparateAttribs;
break;
case GL_MAX_VERTEX_STREAMS:
// GL 4.5 core table 23.62 requires four. MobileGL can only ever EMIT to stream 0 -
// nothing in the shader pipeline supports layout(stream = N), EmitStreamVertex or a
// per-stream capture layout - but that is a statement about what a geometry shader
// may produce, not about which stream indices exist. Streams 1..3 exist and are
// permanently empty, and the two entry points that address a stream say so: an
// indexed primitive query on one answers zero (GL_Query's emptyVertexStream) and
// glDrawTransformFeedbackStream on one draws nothing. Answering 1 instead used to
// make both of them GL_INVALID_VALUE.
// ONE, which is under the GL 4.5 core table 23.62 minimum of four and is a known,
// deliberate non-conformance. It was briefly raised to 4 on the theory that streams
// 1..3 could exist and be permanently empty; measuring that decision refuted it.
// Raising the limit un-gates two CTS cases per package across KHR-GL40..GL46 -
// transform_feedback.draw_xfb_stream_test (which stops being skipped) and
// transform_feedback3.multiple_streams (which stops reporting NotSupported) - and
// both then fail, because nothing in the shader pipeline supports layout(stream = N),
// EmitStreamVertex or EndStreamPrimitive, and because the query state machine tracks
// one active query per TARGET rather than per (target, stream). That is 14 new
// failures against 2 gained limits passes, and a 4 nothing can back is the
// advertised-caps lie with the sign flipped.
//
// The real fix is the feature, not the number: per-stream capture needs
// layout(stream = N) through the transpiler plus per-(target, stream) query slots,
// which DirectVulkan could back with VK_EXT_transform_feedback's geometryStreams and
// DirectGLES cannot back at all (ES has no vertex streams). Until that lands, one is
// the honest count and every stream-addressing entry point bounds itself by THIS
// query, so raising it later moves them all together.
*params = kFrontendMaxVertexStreams;
break;
case GL_TRANSFORM_FEEDBACK_ACTIVE:
MobileGL/MG_Impl/GLImpl/Program/GL_Program.cpp
+25
View File
@@ -245,6 +245,30 @@ namespace MobileGL::MG_Impl::GLImpl {
return true;
}
// GL 4.6 core 7.6.3: INVALID_VALUE when uniformBlockBinding >= MAX_UNIFORM_BUFFER_BINDINGS.
// The storage-block twin below has always had this check; the uniform one never did, and the
// value it stores is used as a RAW SUBSCRIPT into the state layer's fixed indexed-binding
// array on every draw and dispatch (DirectGLES's per-program UBO rebind, DirectVulkan's
// descriptor resolve, whose only guard is a MOBILEGL_ASSERT that compiles away in release).
// An out-of-range binding therefore did not merely go unreported - it read past the array and
// dereferenced whatever SharedPtr it found there.
bool ValidateUniformBlockBinding(GLuint binding) {
// Exactly what glGetIntegerv(GL_MAX_UNIFORM_BUFFER_BINDINGS) advertises: the state
// layer's array width, which the getter clamps to as well.
const SizeT maxBindingCount = MG_State::pGLContext->GetBufferBindingPointCount(BufferTarget::Uniform);
if (binding < maxBindingCount) {
return true;
}
MG_State::pGLContext->RecordError(
ErrorCode::InvalidValue,
MakeUnique<GenericErrorInfo>(
"MG_Impl/GLImpl", __func__,
std::format("Uniform block binding {} is not less than GL_MAX_UNIFORM_BUFFER_BINDINGS ({}).", binding,
maxBindingCount)));
return false;
}
bool ValidateShaderStorageBlockBinding(GLuint binding) {
SizeT maxBindingCount = MG_State::pGLContext->GetBufferBindingPointCount(BufferTarget::ShaderStorage);
if (MG_Backend::pActiveBackendObject) {
@@ -1898,6 +1922,7 @@ namespace MobileGL::MG_Impl::GLImpl {
"Program object" + std::to_string(program) + " that has been linked."));
return;
}
if (!ValidateUniformBlockBinding(uniformBlockBinding)) return;
if (!programObject->IsActiveGlUniformBlock(uniformBlockIndex)) {
MG_State::pGLContext->RecordError(
ErrorCode::InvalidValue,
MobileGL/MG_Impl/GLImpl/Query/GL_Query.cpp
+10 -42
View File
@@ -40,12 +40,6 @@ namespace MobileGL::MG_Impl::GLImpl {
// stand in for the backend's.
Uint64 accountedCaptureDrawSnapshot = 0;
Uint64 geometryCaptureDrawSnapshot = 0;
// Set when glBeginQueryIndexed named a vertex stream above 0. MobileGL advertises
// GL_MAX_VERTEX_STREAMS = 4 because GL 4.5 requires it, and nothing in the shader
// pipeline can emit to a stream other than 0 - so the primitive count on any other
// stream is provably zero, and this makes the object report that instead of
// aliasing stream 0's backend counter.
Bool emptyVertexStream = false;
};
// Query calls may arrive from any thread (launchers migrate the context
@@ -122,7 +116,6 @@ namespace MobileGL::MG_Impl::GLImpl {
queryObject->ended = false;
queryObject->resultCached = false;
queryObject->cachedResult = 0;
queryObject->emptyVertexStream = false;
}
// Callers must hold g_queryObjectsMutex.
@@ -195,23 +188,6 @@ namespace MobileGL::MG_Impl::GLImpl {
return false;
}
// A span begun on a vertex stream this implementation can never emit to. The answer
// is zero, and it is available immediately - the backend query that ran alongside it
// counted stream 0 and must not be reported here.
if (queryObject->emptyVertexStream) {
switch (pname) {
case GL_QUERY_RESULT:
case GL_QUERY_RESULT_NO_WAIT:
outValue = 0;
return true;
case GL_QUERY_RESULT_AVAILABLE:
outValue = GL_TRUE;
return true;
default:
break;
}
}
switch (pname) {
case GL_QUERY_TARGET:
// The target a query was begun with (or created with, for glCreateQueries) - state
@@ -771,7 +747,16 @@ namespace MobileGL::MG_Impl::GLImpl {
// The indexed query entry points differ from the plain ones only in the vertex
// stream they address (GL 4.6 core 4.2.1): index must be below GL_MAX_VERTEX_STREAMS
// for the two transform feedback targets and zero for every other target.
// for the two transform feedback targets and zero for every other target. MobileGL
// implements ONE vertex stream, so both bounds are 1 and a valid call is always index 0 -
// which is what makes the three forwards below equivalent to the unindexed entry points.
//
// THAT EQUIVALENCE IS THE WHOLE JUSTIFICATION, and it is read out of the getter rather
// than assumed: the moment GL_MAX_VERTEX_STREAMS answers more than one, index 1..3 starts
// reaching EndQueryIndexed and GetQueryIndexediv, which resolve the active query from
// per-TARGET globals and would end - or report - a query begun on a different stream.
// Raising that limit therefore means giving each active query a stream index and
// comparing it here, not just changing the number.
Bool ValidateQueryStreamIndex(const char* function, GLenum target, GLuint index) {
const Bool perStreamTarget = IsPerVertexStreamQueryTarget(target);
GLint maxVertexStreams = 1;
@@ -787,28 +772,11 @@ namespace MobileGL::MG_Impl::GLImpl {
return false;
}
// Streams 1..GL_MAX_VERTEX_STREAMS-1 exist but nothing can emit to them, so a span begun
// on one counts zero primitives. Flagging the object is what keeps that answer honest:
// BeginQuery below still opens a real backend query (it is the only way to reuse the
// whole target/object state machine), and that query counts STREAM 0.
//
// Known simplification, spelled out rather than hidden: because the backend query is
// shared, only ONE query may be active per target here, while GL allows one per
// (target, stream) pair. A program running a stream-0 and a stream-2
// GL_PRIMITIVES_GENERATED query at the same time gets GL_INVALID_OPERATION on the
// second. Nothing can produce a non-zero stream-2 result to be worth more than that
// until the shader pipeline grows layout(stream = N).
void MarkQueryEmptyVertexStream(GLuint id) {
const std::lock_guard<std::mutex> lock(g_queryObjectsMutex);
auto* queryObject = FindQueryObjectLocked(id);
if (queryObject && queryObject->active) queryObject->emptyVertexStream = true;
}
} // namespace
void BeginQueryIndexed(GLenum target, GLuint index, GLuint id) {
if (!ValidateQueryStreamIndex(__FUNCTION__, target, index)) return;
BeginQuery(target, id);
if (index != 0 && IsPerVertexStreamQueryTarget(target)) MarkQueryEmptyVertexStream(id);
}
void EndQueryIndexed(GLenum target, GLuint index) {
MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp
+13 -4
View File
@@ -557,11 +557,20 @@ namespace MobileGL::MG_Impl::GLImpl {
: isIntegerFormat ? GetAdvertisedIntegerMaxSamples()
: GetAdvertisedColorTextureMaxSamples();
// glGetInternalformativ(GL_SAMPLES) is answered from this very list (GetInternalformativ
// below), and GL 4.6 core 8.8 makes that query the definition of the per-format
// maximum - validating against anything else is how the two answers drifted apart.
// glGetInternalformativ(GL_SAMPLES) is answered from this very list
// (GetInternalformativ below), and GL 4.6 core 8.8 makes that query the definition of
// the per-format maximum - so when the probe has an answer it IS the ceiling, and the
// category limit only stands in where nothing was probed.
//
// This used to be max(probed, category), which made the probe dead: the walk starts
// AT the category limit (BackendObject_DirectGLES's ProbeTextureSampleCounts) so its
// head can never exceed it, and max() therefore always collapsed to the category
// value. A format whose 4- and 2-sample probes fail inside a 4-sample category - a
// float colour format under EXT_color_buffer_float is the natural instance - was
// still accepted at 4, silently squeezed to 1 by ClampSamplesToBackendSupport, and
// then reported as 4 by GL_TEXTURE_SAMPLES while glGetInternalformativ said 1.
const Int probedMaxSamples = GetProbedMaxTextureSamples(textureTarget, textureInternalFormat);
return probedMaxSamples > 0 ? std::max(probedMaxSamples, categoryMaxSamples) : categoryMaxSamples;
return probedMaxSamples > 0 ? probedMaxSamples : categoryMaxSamples;
}
Bool ValidateTextureMultisampleStorage(TextureTarget textureTarget, GLsizei samples, GLsizei width,