diff --git a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp index d0415ea8..4cadf37c 100644 --- a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp +++ b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp @@ -1786,9 +1786,11 @@ namespace MobileGL::MG_Backend::DirectGLES { memo.pendingMask = pending; memo.valid = true; } - if (memo.pendingMask == 0) return; + // Plus the enabled arrays the walk had to disable (Managers.h, GetDisabledEnabledAttribMask). + const Uint32 pendingMask = memo.pendingMask | (vaoTwin->GetDisabledEnabledAttribMask() & memo.activeMask); + if (pendingMask == 0) return; - for (Uint32 remaining = memo.pendingMask; remaining != 0; remaining &= remaining - 1) { + for (Uint32 remaining = pendingMask; remaining != 0; remaining &= remaining - 1) { const Uint32 location = static_cast(std::countr_zero(remaining)); const auto& currentValue = MG_Pipe::gPipeInputs->GetCurrentVertexAttribute(location); @@ -1922,9 +1924,11 @@ namespace MobileGL::MG_Backend::DirectGLES { memo.pendingMask = pending; memo.valid = true; } - if (memo.pendingMask == 0) return; + // Plus the enabled arrays the walk had to disable (Managers.h, GetDisabledEnabledAttribMask). + const Uint32 pendingMask = memo.pendingMask | (vaoTwin->GetDisabledEnabledAttribMask() & memo.activeMask); + if (pendingMask == 0) return; - for (Uint32 remaining = memo.pendingMask; remaining != 0; remaining &= remaining - 1) { + for (Uint32 remaining = pendingMask; remaining != 0; remaining &= remaining - 1) { const Uint32 location = static_cast(std::countr_zero(remaining)); // BOUNDS-CHECKED, exactly as ProgramObject::GetAttribType is not: the frontend's diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index 93e5e096..df9c4137 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -3528,7 +3528,7 @@ namespace MobileGL::MG_Backend::DirectGLES { // AND PUBLISH IT, because the readers that have NO frontend object read // resource->hostBytes and nothing else: the readback flush (Ops_H_Readback), the // kill-switch map arm of Ops_H_FlushRange, and the fp64 narrowing - // (SyncFloat64AttributeAsFloat32ByHandle, whose legacy counterpart reads + // (SyncConvertedAttributeByHandle, whose legacy counterpart reads // bufferObject->MappedData()). A twin is created LAZILY - D-A2's row makes // Ops_H_Create a no-op - so the very first resource_respecify of a buffer finds // FindBufferResourceForHandle == nullptr and its base is dropped on the floor; with @@ -4727,6 +4727,38 @@ namespace MobileGL::MG_Backend::DirectGLES { } } } + + // GL_BGRA as a vertex SIZE is desktop-only - ES rejects it outright - and it means + // nothing more than "the element's first and third components are stored swapped". + // So the element is put back in RGBA order and fetched as an ordinary size-4 array of + // the same type: for GL_UNSIGNED_BYTE the first and third BYTES swap, for the two + // 2_10_10_10_REV types the 10-bit fields in bits 0-9 and 20-29 (the core spec's packed + // component layout: with BGRA, x comes from bits 20-29 and z from bits 0-9). The 2-bit + // w field and the middle field stay where they are. Elements are copied word-wise, so + // no source alignment is assumed; the output is tightly packed, one word per element. + void SwizzleBgraElementToRgba(Uint8* element, DataType type) { + if (type == DataType::Uint8) { + std::swap(element[0], element[2]); + return; + } + Uint32 word = 0; + Memcpy(&word, element, sizeof(word)); + constexpr Uint32 kField = 0x3FFu; + const Uint32 low = word & kField; + const Uint32 high = (word >> 20) & kField; + word = (word & ~(kField | (kField << 20))) | high | (low << 20); + Memcpy(element, &word, sizeof(word)); + } + + void SwizzleBgraStreamToRgba(const Uint8* sourceBase, SizeT sourceStride, SizeT elementCount, + DataType type, Vector& outData) { + outData.resize(elementCount * 4); + for (SizeT element = 0; element < elementCount; ++element) { + Uint8* destination = outData.data() + element * 4; + Memcpy(destination, sourceBase + element * sourceStride, 4); + SwizzleBgraElementToRgba(destination, type); + } + } } // namespace BackendVertexArrayObject::BackendVertexArrayObject() { @@ -4924,7 +4956,7 @@ namespace MobileGL::MG_Backend::DirectGLES { return false; } if (!attrib.IsInteger) { - const GLint glSize = attrib.IsBgra ? static_cast(GL_BGRA) : static_cast(attrib.Size); + const GLint glSize = static_cast(attrib.Size); g_GLESFuncs.glVertexAttribFormat(attribIndex, glSize, MG_Util::ConvertDataTypeToGLEnum(static_cast(attrib.Type)), attrib.Normalized ? GL_TRUE : GL_FALSE, 0); @@ -4976,7 +5008,7 @@ namespace MobileGL::MG_Backend::DirectGLES { // The same function, driven by the applier's records (D-G4). Every branch of the walk // survives - the enable/disable block, the fp64 narrowing with its Adreno disable, the - // zero-stride binding-API path, the attribute-buffer bind, the BGRA refusal probe, the + // zero-stride binding-API path, the attribute-buffer bind, the BGRA swizzle, the // pointer/IPointer at the shifted fetch offset and the divisor - and the gate is // re-keyed onto three server-owned MGGen counters plus the CSO's {slot, gen}: // @@ -4988,7 +5020,7 @@ namespace MobileGL::MG_Backend::DirectGLES { // still the only thing that // catches a driver-id re-mint no // counter on either side moves - // m_hasConvertedFloat64Attribute <- UNCHANGED: a narrowed stream is + // m_hasConvertedAttribute <- UNCHANGED: a narrowed stream is // derived from buffer CONTENT, // which no serial covers // m_syncedFetchBaseInstance <- no longer in the gate as a @@ -5039,7 +5071,7 @@ namespace MobileGL::MG_Backend::DirectGLES { const Uint32 fetchBaseInstance = BackendUsesNativeBaseInstance() ? 0u : st.VertexFetchBaseInstance; const Bool baseInstanceDirty = m_syncedFetchBaseInstance != fetchBaseInstance; - const Bool emitAttributes = attributesDirty || baseInstanceDirty || m_hasConvertedFloat64Attribute; + const Bool emitAttributes = attributesDirty || baseInstanceDirty || m_hasConvertedAttribute; if (!emitAttributes && !indexBufferDirty) { return; } @@ -5052,9 +5084,11 @@ namespace MobileGL::MG_Backend::DirectGLES { // through other state, and the client-memory upload rewrites the VAO behind this // gate, so any of them sends the next sync through the whole walk again. const Bool buffersOnly = emitAttributes && !elementsDirty && m_lastWalkWasPlain && - !m_hasConvertedFloat64Attribute; + !m_hasConvertedAttribute; Bool walkWasPlain = true; - m_hasConvertedFloat64Attribute = false; + m_hasConvertedAttribute = false; + // Rebuilt by every walk; a buffers-only one follows a plain walk, which set none. + if (emitAttributes && !buffersOnly) m_disabledEnabledAttribMask = 0; Bind(); if (m_hostPointerBufferGeneration != currentBufferIdGeneration) { @@ -5094,13 +5128,20 @@ namespace MobileGL::MG_Backend::DirectGLES { // the next draw (KHR-GL43.vertex_attrib_binding.basic-input-case4). IsLong is // carried separately from Type == Float64 on the wire precisely so this test // can still tell the two apart. - if (attrib.IsLong || attrib.Type == static_cast(DataType::Float64)) { + // + // A GL_BGRA-sized array takes the same stream, swizzled instead of narrowed, and + // the same disable when it cannot be built: GL_BGRA as a vertex SIZE is + // desktop-only, ES rejects it, and the rejected call leaves the array ENABLED + // with no pointer - the shape the Adreno driver dereferences null on inside the + // next draw (KHR-GL43.vertex_attrib_binding.basic-input-case5). + const Bool bgraArray = attrib.IsBgra != 0 && !attrib.IsInteger; + if (attrib.IsLong || attrib.Type == static_cast(DataType::Float64) || bgraArray) { walkWasPlain = false; - if (attrib.Enabled && attrib.Type == static_cast(DataType::Float64) && - binding != nullptr && - SyncFloat64AttributeAsFloat32ByHandle(attribIndex, attrib, *binding, fetchBaseInstance)) { - m_hasConvertedFloat64Attribute = true; - // The narrowing pointed the attribute at its own stream (and may have + const Bool convertible = bgraArray || attrib.Type == static_cast(DataType::Float64); + if (attrib.Enabled && convertible && binding != nullptr && + SyncConvertedAttributeByHandle(attribIndex, attrib, *binding, fetchBaseInstance)) { + m_hasConvertedAttribute = true; + // The conversion pointed the attribute at its own stream (and may have // used the binding API), so the pointer and divisor shadow are stale. // An earlier walk that could not build the stream disabled this array. m_hostPointer[attribIndex].valid = false; @@ -5110,9 +5151,12 @@ namespace MobileGL::MG_Backend::DirectGLES { continue; } if (attrib.Enabled) { - MGLOG_W_ONCE("DirectGLES: vertex attribute %u is a 64-bit (GL_DOUBLE) array whose source " - "stream could not be narrowed to float32 - disabling the array", - attribIndex); + MGLOG_W_ONCE("DirectGLES: vertex attribute %u is a %s array whose source stream could not " + "be converted - disabling the array", + attribIndex, bgraArray ? "GL_BGRA" : "64-bit (GL_DOUBLE)"); + // The draw now reads the attribute's current generic value, which the + // application had no reason to set for an enabled array. + m_disabledEnabledAttribMask |= 1u << attribIndex; } HostSetEnabled(attribIndex, false); continue; @@ -5155,30 +5199,15 @@ namespace MobileGL::MG_Backend::DirectGLES { continue; } - // GL_BGRA as a vertex SIZE is desktop-only and ES rejects it, which leaves the - // array ENABLED with no pointer - and the Adreno driver then dereferences null - // inside the next draw (KHR-GL43.vertex_attrib_binding.basic-input-case5). So - // the refusal is observed and the array disabled. Deliberately ONLY this - // format: the per-draw sync must not grow a glGetError round trip for the - // formats real applications use. - const Bool formatMayBeRefused = attrib.IsBgra != 0; - if (formatMayBeRefused) walkWasPlain = false; - if (formatMayBeRefused) { - while (g_GLESFuncs.glGetError() != GL_NO_ERROR) { - } // start from a clean slate so the check below is about THIS call - } - const SizeT fetchOffset = static_cast(attrib.Offset) + BaseInstanceByteShiftWire(attrib.Stride, divisor, fetchBaseInstance); const GLenum glType = MG_Util::ConvertDataTypeToGLEnum(static_cast(attrib.Type)); - const GLint glSize = (!attrib.IsInteger && attrib.IsBgra) ? static_cast(GL_BGRA) - : static_cast(attrib.Size); + const GLint glSize = static_cast(attrib.Size); // The pointer the driver VAO already holds for this attribute - the buffer it - // captured included - needs no glBindBuffer + pointer call. A format that may be - // refused always goes to the driver, since its glGetError is the point. + // captured included - needs no glBindBuffer + pointer call. auto& shadowPointer = m_hostPointer[attribIndex]; - const Bool pointerHeld = VaoShadowOn() && !formatMayBeRefused && shadowPointer.valid && + const Bool pointerHeld = VaoShadowOn() && shadowPointer.valid && shadowPointer.bufferId == attribBuffer->id && shadowPointer.size == glSize && shadowPointer.type == static_cast(glType) && shadowPointer.stride == static_cast(attrib.Stride) && @@ -5200,17 +5229,6 @@ namespace MobileGL::MG_Backend::DirectGLES { attrib.IsInteger != 0, true}; } - if (formatMayBeRefused && g_GLESFuncs.glGetError() != GL_NO_ERROR) { - MGLOG_W_ONCE("DirectGLES: the driver refused the vertex format of attribute %u " - "(size=%d bgra=%d type=%s) - disabling the array so the draw cannot " - "fetch through a pointer the driver never accepted", - attribIndex, static_cast(attrib.Size), attrib.IsBgra ? 1 : 0, - MG_Util::ConvertGLEnumToString(glType).c_str()); - shadowPointer.valid = false; - HostSetEnabled(attribIndex, false); - continue; - } - HostSetDivisor(attribIndex, divisor); } @@ -5395,8 +5413,20 @@ namespace MobileGL::MG_Backend::DirectGLES { } } + // A GL_BGRA array is swizzled into RGBA order on the way, as on the buffer path + // (SyncConvertedAttributeByHandle): ES has no BGRA vertex size to declare. + const Bool bgra = attrib.IsBgra && !attrib.IsInteger; + Vector swizzled; + if (bgra) { + swizzled.assign(clientData, clientData + uploadSize); + for (SizeT element = 0; element < static_cast(first + count); ++element) { + SwizzleBgraElementToRgba(swizzled.data() + element * stride, attrib.Type); + } + } + BufferImpl::BindBufferId(GL_ARRAY_BUFFER, bufferId); - g_GLESFuncs.glBufferData(GL_ARRAY_BUFFER, static_cast(uploadSize), clientData, + g_GLESFuncs.glBufferData(GL_ARRAY_BUFFER, static_cast(uploadSize), + bgra ? static_cast(swizzled.data()) : clientData, GL_STREAM_DRAW); if (MG_Util::PipeStats::Enabled()) { MG_Util::PipeStats::AddBytes(MG_Util::PipeStats::ByteClass::StageVertexClient, @@ -5404,7 +5434,7 @@ namespace MobileGL::MG_Backend::DirectGLES { } if (!attrib.IsInteger) { - const GLint glSize = attrib.IsBgra ? static_cast(GL_BGRA) : attrib.Size; + const GLint glSize = bgra ? 4 : attrib.Size; g_GLESFuncs.glVertexAttribPointer( attribIndex, glSize, MG_Util::ConvertDataTypeToGLEnum(attrib.Type), attrib.Normalized ? GL_TRUE : GL_FALSE, static_cast(stride), @@ -5426,7 +5456,7 @@ namespace MobileGL::MG_Backend::DirectGLES { } - Bool BackendVertexArrayObject::SyncFloat64AttributeAsFloat32ByHandle( + Bool BackendVertexArrayObject::SyncConvertedAttributeByHandle( Uint attribIndex, const MGPVertexAttribWire& attrib, const MG_Pipe::MGPVertexBuffer& binding, Uint32 fetchBaseInstance) { #ifdef TRACY_ENABLE @@ -5435,6 +5465,16 @@ namespace MobileGL::MG_Backend::DirectGLES { if (attribIndex >= m_convertedAttributeBufferIds.size() || attrib.Size < 1 || attrib.Size > 4) { return false; } + // Which of the two conversions: a BGRA array is a 4-byte element whatever its type, + // and only the three types GL allows with it reach here (the frontend refuses the + // rest at the format call). + const DataType sourceType = static_cast(attrib.Type); + const Bool bgra = attrib.IsBgra != 0; + if (bgra && sourceType != DataType::Uint8 && sourceType != DataType::Int2101010Rev && + sourceType != DataType::Uint2101010Rev) { + return false; + } + if (!bgra && sourceType != DataType::Float64) return false; if (MG_Pipe::MGPipeHandleIsNull(binding.Res)) { // A client-memory 64-bit array is narrowed on the draw path instead, which is // the only place its fetch range is known. @@ -5481,7 +5521,8 @@ namespace MobileGL::MG_Backend::DirectGLES { const Bool sourceGpuWritten = resource->serverGpuWritten; if (sourceGpuWritten) { - BufferImpl::SplitHostBytesForCpuRead(*resource, binding.Res, "fp64_vertex_narrowing"); + BufferImpl::SplitHostBytesForCpuRead(*resource, binding.Res, + bgra ? "bgra_vertex_swizzle" : "fp64_vertex_narrowing"); } const Uint8* const sourceBase = (resource->persistentMapped && resource->persistentPtr != nullptr) @@ -5492,8 +5533,11 @@ namespace MobileGL::MG_Backend::DirectGLES { return false; } + // A BGRA element is always one 32-bit word, and so is its RGBA replacement; a 64-bit + // element narrows to the float32 vector of the same component count. const SizeT componentCount = static_cast(attrib.Size); - const SizeT sourceElementSize = componentCount * sizeof(Double); + const SizeT sourceElementSize = bgra ? 4 : componentCount * sizeof(Double); + const SizeT convertedElementSize = bgra ? 4 : componentCount * sizeof(Float); const SizeT available = sourceSize - static_cast(attrib.Offset); if (available < sourceElementSize) { return false; @@ -5517,60 +5561,82 @@ namespace MobileGL::MG_Backend::DirectGLES { // frontend lifetime id, and the freshness input is the applier's server-owned // Serial instead of the frontend change serial. A PERSISTENTLY MAPPED source is // still never trusted - it is written through the pointer with no call at all, so - // no serial on either side can prove the converted copy is current. + // no serial on either side can prove the converted copy is current. The conversion + // itself is part of the key: one attribute index can carry both shapes over its life, + // over the same bytes. const Bool memoHit = stream.valid && convertedBufferId != 0 && !resource->persistentMapped && !sourceGpuWritten && stream.sourceHandle == binding.Res && stream.sourceChangeSerial == sourceSerial && stream.sourceOffset == static_cast(attrib.Offset) && - stream.sourceStride == sourceStride && - stream.componentCount == componentCount && stream.elementCount == elementCount; + stream.sourceStride == sourceStride && stream.bgra == bgra && + stream.sourceType == attrib.Type && stream.componentCount == componentCount && + stream.elementCount == elementCount; if (!memoHit) { if (convertedBufferId == 0) { g_GLESFuncs.glGenBuffers(1, &convertedBufferId); if (convertedBufferId == 0) { - MGLOG_E_ONCE("Failed to create the float32 scratch buffer for the 64-bit vertex array at " + MGLOG_E_ONCE("Failed to create the scratch buffer for the converted vertex array at " "attribute %u.", attribIndex); return false; } } - Vector converted; - NarrowDoubleStreamToFloat32(sourceBase + attrib.Offset, sourceStride, componentCount, elementCount, - converted); + Vector narrowed; + Vector swizzled; + const void* convertedData = nullptr; + SizeT convertedBytes = 0; + if (bgra) { + SwizzleBgraStreamToRgba(sourceBase + attrib.Offset, sourceStride, elementCount, sourceType, + swizzled); + convertedData = swizzled.data(); + convertedBytes = swizzled.size(); + } else { + NarrowDoubleStreamToFloat32(sourceBase + attrib.Offset, sourceStride, componentCount, + elementCount, narrowed); + convertedData = narrowed.data(); + convertedBytes = narrowed.size() * sizeof(Float); + } BufferImpl::BindBufferId(GL_ARRAY_BUFFER, convertedBufferId); - g_GLESFuncs.glBufferData(GL_ARRAY_BUFFER, static_cast(converted.size() * sizeof(Float)), - converted.data(), GL_STREAM_DRAW); + g_GLESFuncs.glBufferData(GL_ARRAY_BUFFER, static_cast(convertedBytes), convertedData, + GL_STREAM_DRAW); if (MG_Util::PipeStats::Enabled()) { MG_Util::PipeStats::AddBytes(MG_Util::PipeStats::ByteClass::StageVertexClient, - static_cast(converted.size() * sizeof(Float))); + static_cast(convertedBytes)); } stream.valid = true; stream.sourceHandle = binding.Res; stream.sourceChangeSerial = sourceSerial; stream.sourceOffset = static_cast(attrib.Offset); stream.sourceStride = sourceStride; + stream.bgra = bgra; + stream.sourceType = attrib.Type; stream.componentCount = componentCount; stream.elementCount = elementCount; - MGLOG_D("DirectGLES: narrowed the 64-bit vertex array at attribute %u to %zu float32 element(s).", - attribIndex, elementCount); + MGLOG_D("DirectGLES: converted the %s vertex array at attribute %u, %zu element(s).", + bgra ? "BGRA" : "64-bit", attribIndex, elementCount); } - const SizeT convertedElementSize = componentCount * sizeof(Float); + // What the converted stream is to the driver. The swizzled BGRA stream keeps its + // source type and `normalized` (GL accepts BGRA only normalized, and a packed type's + // signedness is the type's own). The narrowed stream is float32, and `normalized` is + // deliberately GL_FALSE there rather than attrib.Normalized: GL ignores it for + // floating-point array types, and honouring it would scale the fetched values. + const GLint convertedSize = bgra ? 4 : static_cast(attrib.Size); + const GLenum convertedType = bgra ? MG_Util::ConvertDataTypeToGLEnum(sourceType) : GL_FLOAT; + const GLboolean convertedNormalized = (bgra && attrib.Normalized) ? GL_TRUE : GL_FALSE; if (neverAdvances) { // Only the binding-point API can say "stride 0". if (!HasVertexBindingApi()) { return false; } - g_GLESFuncs.glVertexAttribFormat(attribIndex, static_cast(attrib.Size), GL_FLOAT, GL_FALSE, 0); + g_GLESFuncs.glVertexAttribFormat(attribIndex, convertedSize, convertedType, convertedNormalized, 0); g_GLESFuncs.glVertexAttribBinding(attribIndex, attribIndex); g_GLESFuncs.glBindVertexBuffer(attribIndex, convertedBufferId, 0, 0); return true; } BufferImpl::BindBufferId(GL_ARRAY_BUFFER, convertedBufferId); - // `normalized` is deliberately GL_FALSE rather than attrib.Normalized: GL ignores it - // for floating-point array types, and honouring it would scale the fetched values. - g_GLESFuncs.glVertexAttribPointer(attribIndex, static_cast(attrib.Size), GL_FLOAT, GL_FALSE, + g_GLESFuncs.glVertexAttribPointer(attribIndex, convertedSize, convertedType, convertedNormalized, static_cast(convertedElementSize), (const void*)(firstElement * convertedElementSize)); return true; diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.h b/MobileGL/MG_Backend/DirectGLES/Managers.h index 11262529..7dc54cba 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.h +++ b/MobileGL/MG_Backend/DirectGLES/Managers.h @@ -1359,6 +1359,12 @@ namespace MobileGL::MG_Backend::DirectGLES { Uint64 elementsSerial = 0; }; PendingAttribValueMask& GetPendingAttribValueMaskMemo() { return m_pendingAttribValueMask; } + // Attributes the configuration ENABLES but the last walk had to disable in the driver + // VAO (a GL_DOUBLE or GL_BGRA array whose converted stream could not be built). Those + // read the current generic value too, so the value sync pushes it for them as well - + // without that they read the driver's untouched (0,0,0,1) instead of what the + // application last set. + Uint32 GetDisabledEnabledAttribMask() const { return m_disabledEnabledAttribMask; } private: // Narrows one enabled GL_DOUBLE array into a tightly packed float32 stream held in @@ -1369,6 +1375,12 @@ namespace MobileGL::MG_Backend::DirectGLES { // dropping it. Returns false when the stream cannot be built, in which case the // caller must DISABLE the array - leaving a 64-bit array enabled with no pointer is // what the Adreno driver turns into a SIGSEGV at the next draw. + // + // The same stream carries the second format ES has no spelling for: a GL_BGRA-sized + // array (GL_UNSIGNED_BYTE or either 2_10_10_10_REV type) is swizzled into RGBA order + // element by element and declared as an ordinary size-4 array of its own type + // (KHR-GL43.vertex_attrib_binding.basic-input-case5). Passing GL_BGRA through is + // GL_INVALID_VALUE on ES, which left the array enabled with no pointer. // The handle arm of the whole vertex-elements half. Everything it needs arrives in // the applier's records - the bound CSO's two views, the vertex-buffer set, the @@ -1378,14 +1390,13 @@ namespace MobileGL::MG_Backend::DirectGLES { // as of P5e (vi) - see the note there. // Same narrowing, same memo, same Adreno disable; the source bytes are the shadow // base the resource call carried and the memo key is the buffer's {slot, gen}. - Bool SyncFloat64AttributeAsFloat32ByHandle(Uint attribIndex, const MGPVertexAttribWire& attrib, - const MG_Pipe::MGPVertexBuffer& binding, - Uint32 fetchBaseInstance); + Bool SyncConvertedAttributeByHandle(Uint attribIndex, const MGPVertexAttribWire& attrib, + const MG_Pipe::MGPVertexBuffer& binding, Uint32 fetchBaseInstance); - // What the converted float32 stream in m_convertedAttributeBufferIds[i] was built - // from. A hit skips the CPU conversion and the re-upload; the buffer's change serial - // is part of the key, so a glBufferSubData into the source invalidates it. - struct ConvertedFloat64Stream { + // What the converted stream in m_convertedAttributeBufferIds[i] was built from. A + // hit skips the CPU conversion and the re-upload; the buffer's change serial is part + // of the key, so a glBufferSubData into the source invalidates it. + struct ConvertedAttributeStream { Bool valid = false; // What replaces it: the source buffer's {slot, gen}. It is the SAME identity // the rest of the backend now keys on, it cannot be reproduced by a recycled @@ -1399,6 +1410,9 @@ namespace MobileGL::MG_Backend::DirectGLES { Uint64 sourceChangeSerial = 0; SizeT sourceOffset = 0; SizeT sourceStride = 0; + // The conversion: a BGRA swizzle of `sourceType` elements, or the fp64 narrowing. + Bool bgra = false; + Uint32 sourceType = 0; SizeT componentCount = 0; SizeT elementCount = 0; }; @@ -1407,16 +1421,17 @@ namespace MobileGL::MG_Backend::DirectGLES { PendingAttribValueMask m_pendingAttribValueMask; Uint m_backendVAOId = 0; Array m_clientAttributeBufferIds; - // Scratch stores for the buffer-backed GL_DOUBLE narrowing. Deliberately separate + // Scratch stores for the buffer-backed GL_DOUBLE narrowing and BGRA swizzle. Deliberately separate // from m_clientAttributeBufferIds: that one holds the per-draw upload of a // CLIENT-MEMORY array, and an attribute index can carry both shapes over its life. Array m_convertedAttributeBufferIds; - Array + Array m_convertedAttributeStreams; // True while at least one attribute of this VAO is fed by a converted stream. Such a // stream is derived from buffer CONTENT, which no VAO version covers, so the config // version early-out in SyncToBackend must not be trusted while it is set. - Bool m_hasConvertedFloat64Attribute = false; + Bool m_hasConvertedAttribute = false; + Uint32 m_disabledEnabledAttribMask = 0; Bool m_isInitialized = false; // ---- what replaces them on the handle arm (D-G4) --------------------------- // The bound vertex-elements CSO this twin last emitted, and the applier's @@ -1463,7 +1478,7 @@ namespace MobileGL::MG_Backend::DirectGLES { // enable/disable/divisor re-sets. The shadow lets the walk emit only differences. It // is valid from creation (a new VAO is all-disabled with divisor 0). Anything that // writes the VAO behind the walk (client-memory uploads, the fp64 narrowing, the - // binding-API zero stride, a refused BGRA format) forgets the affected part. Pointer + // binding-API zero stride, the BGRA swizzle) forgets the affected part. Pointer // entries name driver buffer ids, so they also go when g_bufferBackendIdGeneration // moves: an id can be re-minted. struct HostAttribPointer { diff --git a/MobileGL/MG_IntegrationTest/Scenarios/GpuWrittenDrawInputScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/GpuWrittenDrawInputScenario.cpp index ec1484af..ccef9682 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/GpuWrittenDrawInputScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/GpuWrittenDrawInputScenario.cpp @@ -750,7 +750,7 @@ void main() { EXPECT_TRUE(IsGreen(Half(image, 1))) << "command 1 did not draw (" << Half(image, 1) << ")"; } - // ---- fp64 vertex narrowing (Managers.cpp SyncFloat64AttributeAsFloat32ByHandle) ---- + // ---- fp64 vertex narrowing (Managers.cpp SyncConvertedAttributeByHandle) ---- // // A GL_DOUBLE vertex array is narrowed to float on the CPU from the source buffer's bytes, // and the narrowed copy is memoised on the source's serial - which a GPU write never moves. diff --git a/MobileGL/MG_IntegrationTest/Scenarios/VertexAttribBindingScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/VertexAttribBindingScenario.cpp index 04b3faf6..ddd400b1 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/VertexAttribBindingScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/VertexAttribBindingScenario.cpp @@ -755,4 +755,64 @@ void main() { glDeleteBuffers(1, &vbo); } + // GL_BGRA as the SIZE of an array: the first and third components come from swapped places, + // for GL_UNSIGNED_BYTE and for both packed 2_10_10_10_REV types (KHR-GL43.vertex_attrib_binding. + // basic-input-case5). ES has no BGRA vertex size, so a backend that forwards it gets the array + // refused and reads the current value (0,0,0,0 here) instead. Attribute 1 is the plain-RGBA + // control over the same packed word attribute 2 reads as BGRA. + TEST_F(VertexAttribBindingScenario, BgraArraysAreFetchedWithRedAndBlueSwapped) { + if (!Ready()) GTEST_SKIP(); + ResetCurrentAttribs(); + + constexpr int kStride = 12; + unsigned char vertices[2 * kStride] = {}; + const auto put = [&](int vertex, int offset, const void* data, std::size_t size) { + std::memcpy(vertices + vertex * kStride + offset, data, size); + }; + { + const unsigned char bytes0[4] = {0, 0xff / 4, 0xff / 2, 0xff}; + const unsigned int packedUint0 = 0u | (1023u << 10) | (511u << 20) | (1u << 30); + const int packedInt0 = 0 | (511 << 10) | (255 << 20) | (0 << 30); + put(0, 0, bytes0, 4); + put(0, 4, &packedUint0, 4); + put(0, 8, &packedInt0, 4); + const unsigned char bytes1[4] = {0xff, 0xff / 2, 0xff / 4, 0}; + const unsigned int packedUint1 = 0u | (1023u << 10) | (511u << 20) | (2u << 30); + const unsigned int packedInt1 = (static_cast(-511) & 0x3ffu) | (511u << 10) | + (255u << 20) | (3u << 30); + put(1, 0, bytes1, 4); + put(1, 4, &packedUint1, 4); + put(1, 8, &packedInt1, 4); + } + GLuint vbo = 0; + glGenBuffers(1, &vbo); + glBindBuffer(GL_ARRAY_BUFFER, vbo); + glBufferData(GL_ARRAY_BUFFER, sizeof(vertices), vertices, GL_STATIC_DRAW); + glBindBuffer(GL_ARRAY_BUFFER, 0); + + glBindVertexBuffer(0, vbo, 0, kStride); + glVertexAttribFormat(0, GL_BGRA, GL_UNSIGNED_BYTE, GL_TRUE, 0); + glVertexAttribFormat(1, 4, GL_UNSIGNED_INT_2_10_10_10_REV, GL_TRUE, 4); + glVertexAttribFormat(2, GL_BGRA, GL_UNSIGNED_INT_2_10_10_10_REV, GL_TRUE, 4); + glVertexAttribFormat(3, GL_BGRA, GL_INT_2_10_10_10_REV, GL_TRUE, 8); + for (GLuint i = 0; i < 4; ++i) { + glVertexAttribBinding(i, 0); + glEnableVertexAttribArray(i); + } + ASSERT_EQ(glGetError(), static_cast(GL_NO_ERROR)); + + const std::vector data = CapturePoints(m_program, m_xfbo, 2, 1); + EXPECT_TRUE(Vec4Is(data, 0, 0, 0.5f, 0.25f, 0.0f, 1.0f)); + EXPECT_TRUE(Vec4Is(data, 0, 1, 0.0f, 1.0f, 0.5f, 1.0f / 3.0f)); + EXPECT_TRUE(Vec4Is(data, 0, 2, 0.5f, 1.0f, 0.0f, 1.0f / 3.0f)); + EXPECT_TRUE(Vec4Is(data, 0, 3, 0.5f, 1.0f, 0.0f, 0.0f)); + EXPECT_TRUE(Vec4Is(data, 1, 0, 0.25f, 0.5f, 1.0f, 0.0f)); + EXPECT_TRUE(Vec4Is(data, 1, 1, 0.0f, 1.0f, 0.5f, 2.0f / 3.0f)); + EXPECT_TRUE(Vec4Is(data, 1, 2, 0.5f, 1.0f, 0.0f, 2.0f / 3.0f)); + EXPECT_TRUE(Vec4Is(data, 1, 3, 0.5f, 1.0f, -1.0f, -1.0f)); + + for (GLuint i = 0; i < 4; ++i) glDisableVertexAttribArray(i); + glDeleteBuffers(1, &vbo); + } + } // namespace MGITest