From 7aa91e802467dc76b01bcda02d41ead149511e2d Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Thu, 20 Aug 2026 23:54:49 -0400 Subject: [PATCH] [Fix, Test] (DirectGLES, DirectVulkan): narrow GL_DOUBLE vertex arrays to float32 instead of dropping them --- MobileGL/MG_Backend/DirectGLES/Managers.cpp | 257 ++++++++++++++++-- MobileGL/MG_Backend/DirectGLES/Managers.h | 34 +++ .../Renderer/VertexInputStateFactory.cpp | 33 ++- .../Renderer/VertexInputStateFactory.h | 3 + .../DirectVulkan/Renderer/VulkanRenderer.cpp | 38 +++ .../GLImpl/VertexArray/GL_VertexArray.cpp | 26 +- .../Scenarios/VertexAttribBindingScenario.cpp | 82 ++++++ MobileGL/MG_Util/SelfTest/DriverPost.cpp | 25 +- 8 files changed, 436 insertions(+), 62 deletions(-) diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index e66ce27d..c91a8fba 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -1576,6 +1576,25 @@ namespace MobileGL::MG_Backend::DirectGLES { const SizeT componentSize = GetDataTypeSize(type); return componentSize == 0 ? 0 : componentSize * static_cast(size); } + + // Deinterleaves elementCount elements of componentCount doubles into a tightly packed + // float32 stream - the fetch half of the fp64 demotion the shader side already does + // unconditionally (DemoteFloat64Pass). GL byte strides and offsets are arbitrary, so + // no component carries an 8-byte alignment guarantee and each is copied out before it + // is narrowed. + void NarrowDoubleStreamToFloat32(const Uint8* sourceBase, SizeT sourceStride, SizeT componentCount, + SizeT elementCount, Vector& outData) { + outData.resize(elementCount * componentCount); + for (SizeT element = 0; element < elementCount; ++element) { + const Uint8* sourceElement = sourceBase + element * sourceStride; + Float* destinationElement = outData.data() + element * componentCount; + for (SizeT component = 0; component < componentCount; ++component) { + Double value = 0.0; + Memcpy(&value, sourceElement + component * sizeof(Double), sizeof(Double)); + destinationElement[component] = static_cast(value); + } + } + } } // namespace BackendVertexArrayObject::BackendVertexArrayObject() { @@ -1583,6 +1602,7 @@ namespace MobileGL::MG_Backend::DirectGLES { ZoneScopedC(TRACY_ZONECOLOR_BACKEND); #endif m_clientAttributeBufferIds.fill(0); + m_convertedAttributeBufferIds.fill(0); g_GLESFuncs.glGenVertexArrays(1, &m_backendVAOId); if (m_backendVAOId == 0) { MGLOG_E_ONCE("Failed to generate vertex array object."); @@ -1608,6 +1628,13 @@ namespace MobileGL::MG_Backend::DirectGLES { bufferId = 0; } } + for (auto& bufferId : m_convertedAttributeBufferIds) { + if (bufferId != 0) { + BufferImpl::NoteBufferIdDeleted(bufferId); + g_GLESFuncs.glDeleteBuffers(1, &bufferId); + bufferId = 0; + } + } } namespace { @@ -1776,10 +1803,15 @@ namespace MobileGL::MG_Backend::DirectGLES { // that never calls a *BaseInstance entry point never pays for this compare. const Uint32 fetchBaseInstance = g_pendingFetchBaseInstance; const Bool baseInstanceDirty = m_syncedFetchBaseInstance != fetchBaseInstance; - const Bool emitAttributes = attributesDirty || baseInstanceDirty; + // A narrowed GL_DOUBLE stream is derived from the source buffer's CONTENT, and no + // VAO version moves when an app writes into a buffer, so the version gate cannot + // prove such a stream is still current. Re-walk while one is live; the walk itself + // re-checks the buffer's change serial and only re-converts on a real move. + const Bool emitAttributes = attributesDirty || baseInstanceDirty || m_hasConvertedFloat64Attribute; if (!emitAttributes && !indexBufferDirty) { return; } + m_hasConvertedFloat64Attribute = false; Bind(); @@ -1805,35 +1837,51 @@ namespace MobileGL::MG_Backend::DirectGLES { m_syncedAttributeVersions[attribIndex].FormatVersion; Bool needsSyncBuffer = bufferIdsRemitted || allAttributeVersions[attribIndex].BufferVersion != m_syncedAttributeVersions[attribIndex].BufferVersion; - if (!needsSyncFormat && !needsSyncBuffer && !needsSyncBaseInstance) continue; - - // This is where a 64-bit array actually stops. glVertexAttribLFormat is a legal call - // in a GL 4.3 context and the frontend RECORDS its format (the state queries have to - // answer), so IsLong does arrive here - what this backend cannot do is FEED it: - // SupportsFloat64VertexAttributes is false because ES has no GL_DOUBLE vertex format - // and ESSL has no fp64 type, and passing GL_DOUBLE to glVertexAttribPointer would - // only raise GL_INVALID_ENUM on the real driver. Disabling rather than merely - // skipping matters: becoming long bumps FormatVersion, not SwitchVersion, so the - // enable/disable block above will not run again and an already-enabled array would - // stay enabled with no pointer and no ARRAY_BUFFER binding - which ES 3.1+ makes an - // INVALID_OPERATION at draw. + // This is where a 64-bit array is narrowed. glVertexAttribLFormat is a legal call in + // a GL 4.3 context and the frontend RECORDS its format (the state queries have to + // answer), so IsLong does arrive here - what this backend cannot do is feed it at + // full precision: ES has no GL_DOUBLE vertex FORMAT and ESSL has no fp64 type, and + // passing GL_DOUBLE to glVertexAttribPointer would only raise GL_INVALID_ENUM on the + // real driver. But the FORMAT is the only 64-bit thing here: the source bytes are + // ordinary IEEE-754 doubles, and MobileGL already narrows every fp64 value in every + // shader (DemoteFloat64Pass) and every glUniform*d the same way, so the array is + // deinterleaved into a float32 stream and fetched as GL_FLOAT rather than dropped. + // glVertexAttribFormat(GL_DOUBLE) asks for exactly that conversion anyway; the L form + // asks for more precision than any backend here can give, and gets the same stream. // - // IsLong is not the only way a 64-bit array gets here: glVertexAttribFormat - // with GL_DOUBLE asks for doubles in memory CONVERTED to float, so it is not - // long, is not declined by the frontend, and still has no ES vertex format. - // Leaving that one enabled did not merely raise INVALID_ENUM - the Adreno - // driver dereferenced null inside the next draw and took the process with it - // (SIGSEGV in libGLESv2_adreno, KHR-GL43.vertex_attrib_binding.basic-input-case4), - // because the array stayed enabled with no pointer the failed call could set. - // The type test therefore covers the storage, not the spelling. + // The conversion is deliberately NOT behind the version gate below: it is derived + // from buffer CONTENT, which no VAO version covers. Its own memo (keyed on the + // source buffer's change serial) is what keeps the repeat cost down. + // + // When no stream can be built the array is DISABLED rather than left alone. That + // matters: becoming long bumps FormatVersion, not SwitchVersion, so the + // enable/disable block above will not run again and an already-enabled array would + // stay enabled with no pointer and no ARRAY_BUFFER binding - which did not merely + // raise INVALID_ENUM but had the Adreno driver dereference null inside the next draw + // and take the process with it (SIGSEGV in libGLESv2_adreno, + // KHR-GL43.vertex_attrib_binding.basic-input-case4). if (attrib.IsLong || attrib.Type == DataType::Float64) { - MGLOG_W_ONCE("DirectGLES: vertex attribute %u is a 64-bit (GL_DOUBLE) array, which this " - "backend cannot feed - disabling the array", - attribIndex); + if (attrib.Enabled && attrib.Type == DataType::Float64 && + SyncFloat64AttributeAsFloat32(attribIndex, attrib, fetchBaseInstance)) { + m_hasConvertedFloat64Attribute = true; + // Explicit, not redundant: an earlier walk that could not build the stream + // disabled this array, and becoming feedable again bumps no SwitchVersion, + // so the enable/disable block above would never turn it back on. + g_GLESFuncs.glEnableVertexAttribArray(attribIndex); + g_GLESFuncs.glVertexAttribDivisor(attribIndex, attrib.Divisor); + 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); + } g_GLESFuncs.glDisableVertexAttribArray(attribIndex); continue; } + if (!needsSyncFormat && !needsSyncBuffer && !needsSyncBaseInstance) continue; + // A resolved stride of zero is the binding model's "never advance" (see // VertexAttribute::Stride) and glVertexAttribPointer cannot say it - a zero // stride argument there means "tightly packed" instead, i.e. exactly the @@ -1956,11 +2004,50 @@ namespace MobileGL::MG_Backend::DirectGLES { continue; } - // Same reason as SyncToBackend, including why the test is on the storage rather - // than on IsLong: there is no ES vertex format for a 64-bit array, and this path - // only ever reaches glVertexAttribPointer/IPointer. + // Same narrowing as SyncToBackend (the long note lives there), with the draw's own + // fetch range standing in for the buffer extent a client array does not have. The + // upload starts at element 0 so `first` keeps indexing the stream, exactly as the + // unconverted upload below does. The test is on the storage rather than on IsLong + // because glVertexAttribFormat(GL_DOUBLE) is 64-bit data without being long. if (attrib.IsLong || attrib.Type == DataType::Float64) { - g_GLESFuncs.glDisableVertexAttribArray(attribIndex); + const auto* sourceBase = reinterpret_cast(attrib.Offset); + if (attrib.Type != DataType::Float64 || sourceBase == nullptr || attrib.Size < 1 || + attrib.Size > 4) { + g_GLESFuncs.glDisableVertexAttribArray(attribIndex); + continue; + } + + auto& bufferId = m_clientAttributeBufferIds[attribIndex]; + if (bufferId == 0) { + g_GLESFuncs.glGenBuffers(1, &bufferId); + if (bufferId == 0) { + MGLOG_E_ONCE("Failed to create client-side vertex attribute upload buffer."); + g_GLESFuncs.glDisableVertexAttribArray(attribIndex); + continue; + } + } + + const SizeT componentCount = static_cast(attrib.Size); + const SizeT sourceElementSize = componentCount * sizeof(Double); + // A client array only ever reaches here through a pointer call, whose stride 0 + // the frontend already resolved to the element size, so this is a guard rather + // than a case (VertexAttribute::Stride). + const SizeT sourceStride = + attrib.Stride > 0 ? static_cast(attrib.Stride) : sourceElementSize; + const SizeT elementCount = static_cast(first) + static_cast(count); + Vector converted; + NarrowDoubleStreamToFloat32(sourceBase, sourceStride, componentCount, elementCount, converted); + + BufferImpl::BindBufferId(GL_ARRAY_BUFFER, bufferId); + g_GLESFuncs.glBufferData(GL_ARRAY_BUFFER, + static_cast(converted.size() * sizeof(Float)), + converted.data(), GL_STREAM_DRAW); + // GL ignores `normalized` for floating-point array types, so it is not + // forwarded here either. + g_GLESFuncs.glVertexAttribPointer(attribIndex, attrib.Size, GL_FLOAT, GL_FALSE, + static_cast(componentCount * sizeof(Float)), + nullptr); + g_GLESFuncs.glEnableVertexAttribArray(attribIndex); continue; } @@ -2000,6 +2087,120 @@ namespace MobileGL::MG_Backend::DirectGLES { } + Bool BackendVertexArrayObject::SyncFloat64AttributeAsFloat32( + Uint attribIndex, const MG_State::GLState::VertexAttribute& attrib, Uint32 fetchBaseInstance) { +#ifdef TRACY_ENABLE + ZoneScopedC(TRACY_ZONECOLOR_BACKEND); +#endif + if (attribIndex >= m_convertedAttributeBufferIds.size() || attrib.Size < 1 || attrib.Size > 4) { + return false; + } + const auto& bufferObject = attrib.Buffer; + if (!bufferObject) { + // A client-memory 64-bit array is narrowed on the draw path instead, which is the + // only place its fetch range is known (SyncClientSideAttributesForDrawArrays). + return false; + } + + // The frontend shadow is what the conversion reads, so a shader write that has not + // been pulled back yet has to land first. A no-op unless one is outstanding. + bufferObject->SyncGpuWrites(); + const Uint8* const sourceBase = bufferObject->MappedData(); + const SizeT sourceSize = bufferObject->GetSize(); + if (sourceBase == nullptr || attrib.Offset >= sourceSize) { + return false; + } + + const SizeT componentCount = static_cast(attrib.Size); + const SizeT sourceElementSize = componentCount * sizeof(Double); + const SizeT available = sourceSize - attrib.Offset; + if (available < sourceElementSize) { + return false; + } + + // A resolved stride of zero is the binding model's "never advance" (see + // VertexAttribute::Stride): exactly one element exists and every vertex reads it, so + // exactly one is converted. Otherwise the array's extent is the source buffer's own - + // SyncToBackend has no draw range, and a whole-array conversion is affordable because + // it is memoised on the buffer's change serial and 64-bit arrays are vanishingly rare. + const Bool neverAdvances = attrib.Stride <= 0; + const SizeT sourceStride = neverAdvances ? sourceElementSize : static_cast(attrib.Stride); + const SizeT elementCount = neverAdvances ? 1 : ((available - sourceElementSize) / sourceStride) + 1; + + // baseInstance shifts the ELEMENT index of a divisor'd array, and one element of the + // converted stream is componentCount floats. A zero stride never advances, so no + // shift can move it. A shift past the array's own extent has no source data at all. + const SizeT firstElement = (fetchBaseInstance != 0 && attrib.Divisor != 0 && !neverAdvances) + ? static_cast(fetchBaseInstance) + : 0; + if (firstElement >= elementCount) { + return false; + } + + auto& stream = m_convertedAttributeStreams[attribIndex]; + Uint& convertedBufferId = m_convertedAttributeBufferIds[attribIndex]; + const Uint64 sourceLifetimeId = bufferObject->GetLifetimeId(); + const Uint64 sourceChangeSerial = bufferObject->GetChangeSerial(); + // A persistent map is written through the pointer, with no API call to bump the change + // serial (see BufferObject::SyncPersistentMappedRange), so its serial cannot prove the + // converted copy is still current and the memo is never trusted for one. + const Bool memoHit = + stream.valid && convertedBufferId != 0 && !bufferObject->IsBackendPersistentMapped() && + stream.sourceLifetimeId == sourceLifetimeId && stream.sourceChangeSerial == sourceChangeSerial && + stream.sourceOffset == attrib.Offset && stream.sourceStride == sourceStride && + 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 " + "attribute %u.", + attribIndex); + return false; + } + } + Vector converted; + NarrowDoubleStreamToFloat32(sourceBase + attrib.Offset, sourceStride, componentCount, elementCount, + converted); + BufferImpl::BindBufferId(GL_ARRAY_BUFFER, convertedBufferId); + g_GLESFuncs.glBufferData(GL_ARRAY_BUFFER, + static_cast(converted.size() * sizeof(Float)), + converted.data(), GL_STREAM_DRAW); + stream.valid = true; + stream.sourceLifetimeId = sourceLifetimeId; + stream.sourceChangeSerial = sourceChangeSerial; + stream.sourceOffset = attrib.Offset; + stream.sourceStride = sourceStride; + 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); + } + + const SizeT convertedElementSize = componentCount * sizeof(Float); + if (neverAdvances) { + // Only the binding-point API can say "stride 0": glVertexAttribPointer's zero + // means "tightly packed" instead, i.e. the opposite, and would walk the driver + // straight off the end of the single converted element. + if (!HasVertexBindingApi()) { + return false; + } + g_GLESFuncs.glVertexAttribFormat(attribIndex, attrib.Size, GL_FLOAT, GL_FALSE, 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 + // (KHR-GL43.vertex_attrib_binding.basic-input-case5 passes GL_TRUE and expects 10/20). + g_GLESFuncs.glVertexAttribPointer(attribIndex, attrib.Size, GL_FLOAT, GL_FALSE, + static_cast(convertedElementSize), + (const void*)(firstElement * convertedElementSize)); + return true; + } + StateBackendObjectRegistry g_backendVertexArrayObjects; } // namespace VertexArrayImpl diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.h b/MobileGL/MG_Backend/DirectGLES/Managers.h index 0f9b92b5..b6d70317 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.h +++ b/MobileGL/MG_Backend/DirectGLES/Managers.h @@ -509,10 +509,44 @@ namespace MobileGL::MG_Backend::DirectGLES { PendingAttribValueMask& GetPendingAttribValueMaskMemo() { return m_pendingAttribValueMask; } private: + // Narrows one enabled GL_DOUBLE array into a tightly packed float32 stream held in + // this VAO's own scratch buffer and declares the attribute against it. ES has no + // 64-bit vertex format, but the source bytes are ordinary IEEE-754 doubles and every + // fp64 value in every shader is already narrowed to 32 bits (DemoteFloat64Pass), so + // narrowing the ARRAY is the coherent completion of that decision rather than + // 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. + Bool SyncFloat64AttributeAsFloat32(Uint attribIndex, const MG_State::GLState::VertexAttribute& attrib, + 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 { + Bool valid = false; + Uint64 sourceLifetimeId = 0; + Uint64 sourceChangeSerial = 0; + SizeT sourceOffset = 0; + SizeT sourceStride = 0; + SizeT componentCount = 0; + SizeT elementCount = 0; + }; + ResolvedDrawBuffers m_resolvedDrawBuffers; PendingAttribValueMask m_pendingAttribValueMask; Uint m_backendVAOId = 0; Array m_clientAttributeBufferIds; + // Scratch stores for the buffer-backed GL_DOUBLE narrowing. 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 + 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_isInitialized = false; Uint16 m_syncedIndexBufferVersion = 0; // Identity of the buffer the version above was stamped against. Raw and never diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.cpp index c69a994a..d6dc2d35 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.cpp @@ -108,8 +108,23 @@ namespace MobileGL::MG_Backend::DirectVulkan { continue; } - const VkFormat sourceVkFormat = + VkFormat sourceVkFormat = ToVkVertexFormat(attr.Type, attr.Size, attr.Normalized, attr.IsInteger, attr.IsBgra, attr.IsLong); + VertexStreamConversion conversion = VertexStreamConversion::None; + if (sourceVkFormat == VK_FORMAT_UNDEFINED && attr.Type == DataType::Float64) { + // No native 64-bit fetch here (see ToVkVertexFormat's Float64 case), but the + // source bytes are ordinary IEEE-754 doubles and DemoteFloat64Pass has already + // narrowed every dvec input to a vec, so the array is narrowed to match rather + // than dropped. Mirrors what DirectGLES does for the same state. + const VkFormat narrowedFormat = ToFloat32VertexFormat(attr.Size); + if (narrowedFormat != VK_FORMAT_UNDEFINED && SupportsVertexBufferFormat(narrowedFormat)) { + sourceVkFormat = narrowedFormat; + conversion = VertexStreamConversion::Float64ToFloat32; + MGLOG_W_ONCE("Vertex attribute location=%u is a 64-bit (GL_DOUBLE) array; fetching it at " + "float32 precision through format=%d (size=%d long=%s)", + location, static_cast(narrowedFormat), attr.Size, attr.IsLong ? "true" : "false"); + } + } if (sourceVkFormat == VK_FORMAT_UNDEFINED) { MGLOG_E_ONCE("Unsupported vertex attribute layout (location=%u, type=%s, size=%d): the array is " "enabled but cannot be mapped to a VkFormat", @@ -119,8 +134,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { } VkFormat vkFormat = sourceVkFormat; - VertexStreamConversion conversion = VertexStreamConversion::None; - if (!SupportsVertexBufferFormat(vkFormat)) { + if (conversion == VertexStreamConversion::None && !SupportsVertexBufferFormat(vkFormat)) { if (IsScaledIntegerVertexFormat(vkFormat)) { const VkFormat fallbackFormat = ToFloat32VertexFormat(attr.Size); if (fallbackFormat != VK_FORMAT_UNDEFINED && SupportsVertexBufferFormat(fallbackFormat)) { @@ -189,7 +203,8 @@ namespace MobileGL::MG_Backend::DirectVulkan { if (sourceStride != 0) { if (conversion == VertexStreamConversion::Repack) { stride = static_cast(attribByteSize); - } else if (conversion == VertexStreamConversion::ScaledIntegerToFloat32) { + } else if (conversion == VertexStreamConversion::ScaledIntegerToFloat32 || + conversion == VertexStreamConversion::Float64ToFloat32) { stride = static_cast(attr.Size * static_cast(sizeof(Float))); } } @@ -336,11 +351,11 @@ namespace MobileGL::MG_Backend::DirectVulkan { // no 64-bit vertex attribute support: DemoteFloat64Pass has already narrowed every // `dvec` input to a `vec` by then, so PackDoubleVertexInputsPass finds nothing to pack // and a UINT-formatted attribute would be fed to a float input - garbage with no - // diagnostic anywhere. Declining here drops the array instead (the caller skips - // UNDEFINED attributes and reports them through unsupportedAttribMask), which is what - // DirectGLES does for the same state. The frontend RECORDS the format either way, so - // this gate is the only thing standing between a legal glVertexAttribLFormat and a - // mismatched pipeline. + // diagnostic anywhere. Declining here hands the attribute to the caller's + // Float64ToFloat32 fallback instead, which narrows the source doubles to match the + // demoted `vec` input - the same thing DirectGLES does for the same state. The + // frontend RECORDS the format either way, so this gate is the only thing standing + // between a legal glVertexAttribLFormat and a mismatched pipeline. if (MG_Backend::pActiveBackendObject == nullptr || !MG_Backend::pActiveBackendObject->GetDynamicParameters().SupportsFloat64VertexAttributes) { return VK_FORMAT_UNDEFINED; diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.h index 7e13525e..b14231bb 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.h @@ -23,6 +23,9 @@ namespace MobileGL::MG_Backend::DirectVulkan { None = 0, Repack, ScaledIntegerToFloat32, + // GL_DOUBLE source data narrowed to a tightly packed float32 stream: the fetch half + // of the fp64 demotion the shader side already does unconditionally. + Float64ToFloat32, }; struct BackendVertexInputState { diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp index 2a1b0541..fd6862be 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp @@ -972,6 +972,36 @@ namespace MobileGL::MG_Backend::DirectVulkan { } } + // The fetch half of the fp64 demotion the shader side already does unconditionally + // (DemoteFloat64Pass): the source bytes are ordinary IEEE-754 doubles, so a GL_DOUBLE array is + // deinterleaved into a tightly packed float32 stream rather than dropped. `normalized` is not + // consulted - GL ignores it for floating-point array types. + static Bool ConvertFloat64VertexStreamToFloat32( + const MG_State::GLState::VertexAttribute& attribute, + const Uint8* sourceData, + SizeT sourceStride, + SizeT elementCount, + Vector& outData) { + if (sourceData == nullptr || attribute.Size < 1 || attribute.Size > 4 || sourceStride == 0) { + return false; + } + + const SizeT componentCount = static_cast(attribute.Size); + outData.resize(elementCount * componentCount); + for (SizeT element = 0; element < elementCount; ++element) { + const Uint8* sourceElement = sourceData + element * sourceStride; + Float* destinationElement = outData.data() + element * componentCount; + for (SizeT component = 0; component < componentCount; ++component) { + // GL byte strides and offsets are arbitrary, so no component carries an 8-byte + // alignment guarantee; copy it out before narrowing it. + Double value = 0.0; + Memcpy(&value, sourceElement + component * sizeof(Double), sizeof(Double)); + destinationElement[component] = static_cast(value); + } + } + return true; + } + static Bool RepackVertexStream(const Uint8* sourceData, SizeT sourceStride, SizeT elementSize, @@ -3595,6 +3625,14 @@ void main() { uploadData = m_vertexConversionScratch.data(); uploadSize = static_cast(m_vertexConversionScratch.size() * sizeof(Float)); break; + case VertexInputStateFactory::VertexStreamConversion::Float64ToFloat32: + if (!ConvertFloat64VertexStreamToFloat32(attribute, sourceData, sourceStride, elementCount, + m_vertexConversionScratch)) { + return false; + } + uploadData = m_vertexConversionScratch.data(); + uploadSize = static_cast(m_vertexConversionScratch.size() * sizeof(Float)); + break; case VertexInputStateFactory::VertexStreamConversion::None: return false; } diff --git a/MobileGL/MG_Impl/GLImpl/VertexArray/GL_VertexArray.cpp b/MobileGL/MG_Impl/GLImpl/VertexArray/GL_VertexArray.cpp index cfffb533..29b07372 100644 --- a/MobileGL/MG_Impl/GLImpl/VertexArray/GL_VertexArray.cpp +++ b/MobileGL/MG_Impl/GLImpl/VertexArray/GL_VertexArray.cpp @@ -514,15 +514,17 @@ namespace MobileGL::MG_Impl::GLImpl { // recorded DataType is always Float64 - what IsLong adds is that this is the *unconverted* form, // as opposed to VertexAttribFormat(GL_DOUBLE), which asks for a float conversion. // - // Whether the backend can FEED it is detected, not assumed: DirectVulkan needs shaderFloat64, - // and DirectGLES can never have it at all. What that costs is the ARRAY, not the call: GL 4.6 - // core 10.3.2 defines no error for a well-formed glVertexAttribLFormat, and a GL 4.3 context - // has 64-bit attributes in core, so declining the call would be non-conformant and would make - // the four pure state queries (VERTEX_ATTRIB_ARRAY_SIZE / _TYPE / _LONG / _RELATIVE_OFFSET) - // unanswerable (KHR-GL43.vertex_attrib_binding.basic-state1/3). The format is therefore - // RECORDED here and the enabled array is dropped at draw instead - loudly, once, naming the - // reason. The matching startup POST row is in MG_Util/SelfTest/DriverPost.cpp; the draw-side - // drop is DirectGLES/Managers.cpp and, on DirectVulkan, VertexInputStateFactory's Float64 case. + // Whether the backend can FEED it at full precision is detected, not assumed: DirectVulkan + // needs shaderFloat64, and DirectGLES can never have it at all. What that costs is PRECISION, + // not the call and no longer the array: GL 4.6 core 10.3.2 defines no error for a well-formed + // glVertexAttribLFormat, and a GL 4.3 context has 64-bit attributes in core, so declining the + // call would be non-conformant and would make the four pure state queries + // (VERTEX_ATTRIB_ARRAY_SIZE / _TYPE / _LONG / _RELATIVE_OFFSET) unanswerable + // (KHR-GL43.vertex_attrib_binding.basic-state1/3). The format is therefore RECORDED here and + // the array is NARROWED to float32 at draw, matching the fp64 demotion every shader already + // gets (DemoteFloat64Pass) - loudly, once, naming the cost. The matching startup POST row is in + // MG_Util/SelfTest/DriverPost.cpp; the draw-side narrowing is DirectGLES/Managers.cpp and, on + // DirectVulkan, VertexInputStateFactory's Float64 case. static void VertexAttribLFormatSeparate_State(const SharedPtr& vao, GLuint attribindex, GLint size, GLenum type, GLuint relativeoffset) { @@ -534,9 +536,9 @@ namespace MobileGL::MG_Impl::GLImpl { !MG_Backend::pActiveBackendObject->GetDynamicParameters().SupportsFloat64VertexAttributes) { MGLOG_W_ONCE("VertexAttribLFormat: attribute %u asked for a 64-bit (GL_DOUBLE) format, but this " "backend has no double-precision vertex attribute support - the format is recorded " - "and queryable, but the array will be DROPPED at draw and the attribute will read " - "its generic current value; see the \"64-bit vertex attributes\" / \"shaderFloat64\" " - "POST row for what that costs", + "and queryable, and the array is FETCHED AT FLOAT32 PRECISION at draw (the same " + "narrowing the shader's dvec inputs already get); see the \"64-bit vertex " + "attributes\" / \"shaderFloat64\" POST row for what that costs", attribindex); } diff --git a/MobileGL/MG_IntegrationTest/Scenarios/VertexAttribBindingScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/VertexAttribBindingScenario.cpp index ec8c5ada..04b3faf6 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/VertexAttribBindingScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/VertexAttribBindingScenario.cpp @@ -673,4 +673,86 @@ void main() { glDeleteProgram(program); } + // A GL_DOUBLE array is NARROWED to float32 and fetched, not dropped. No backend here has a + // 64-bit vertex format, but glVertexAttribFormat(GL_DOUBLE) is defined as "doubles in memory, + // converted to float" and the shader input is a plain vec4 either way, so nothing about fp64 + // is needed - only the fetch conversion (KHR-GL43.vertex_attrib_binding.basic-input-case4). + // Every value here is exact in float32, so the capture is an equality test. + TEST_F(VertexAttribBindingScenario, DoubleArrayIsFetchedAtFloat32Precision) { + if (!Ready()) GTEST_SKIP(); + ResetCurrentAttribs(); + + const double vertices[] = {100.0, 200.0, 300.0, 400.0}; + 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, 2 * static_cast(sizeof(double))); + glVertexAttribFormat(1, 2, GL_DOUBLE, GL_FALSE, 0); + glVertexAttribBinding(1, 0); + glEnableVertexAttribArray(1); + + const std::vector data = CapturePoints(m_program, m_xfbo, 2, 1); + EXPECT_TRUE(Vec4Is(data, 0, 1, 100.0f, 200.0f, 0.0f, 1.0f)); + EXPECT_TRUE(Vec4Is(data, 1, 1, 300.0f, 400.0f, 0.0f, 1.0f)); + + glDisableVertexAttribArray(1); + glDeleteBuffers(1, &vbo); + } + + // GL ignores `normalized` for floating-point array types, GL_DOUBLE included: the fetched + // values are the raw ones, not scaled into [0,1]. A conversion that forwarded the flag would + // return zeros here (KHR-GL43.vertex_attrib_binding.basic-input-case5). + TEST_F(VertexAttribBindingScenario, NormalizedIsIgnoredForDoubleArrays) { + if (!Ready()) GTEST_SKIP(); + ResetCurrentAttribs(); + + const double vertices[] = {0.0, 10.0, 20.0, 0.0}; + 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, 4 * static_cast(sizeof(double))); + glVertexAttribFormat(2, 4, GL_DOUBLE, GL_TRUE, 0); + glVertexAttribBinding(2, 0); + glEnableVertexAttribArray(2); + + const std::vector data = CapturePoints(m_program, m_xfbo, 1, 1); + EXPECT_TRUE(Vec4Is(data, 0, 2, 0.0f, 10.0f, 20.0f, 0.0f)); + + glDisableVertexAttribArray(2); + glDeleteBuffers(1, &vbo); + } + + // The LONG form asks for more precision than any backend here can give and gets the same + // float32 stream. IsLong must not gate the narrowing off + // (KHR-GL43.vertex_attrib_binding.advanced-bindingUpdate feeds its dvec3 this way). + TEST_F(VertexAttribBindingScenario, LongDoubleArrayIsFetchedAtFloat32Precision) { + if (!Ready()) GTEST_SKIP(); + ResetCurrentAttribs(); + + const double vertices[] = {1.0, 2.0, 3.0, 4.0, 5.0, 6.0}; + 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, 3 * static_cast(sizeof(double))); + glVertexAttribLFormat(3, 3, GL_DOUBLE, 0); + glVertexAttribBinding(3, 0); + glEnableVertexAttribArray(3); + + const std::vector data = CapturePoints(m_program, m_xfbo, 2, 1); + EXPECT_TRUE(Vec4Is(data, 0, 3, 1.0f, 2.0f, 3.0f, 1.0f)); + EXPECT_TRUE(Vec4Is(data, 1, 3, 4.0f, 5.0f, 6.0f, 1.0f)); + + glDisableVertexAttribArray(3); + glDeleteBuffers(1, &vbo); + } + } // namespace MGITest diff --git a/MobileGL/MG_Util/SelfTest/DriverPost.cpp b/MobileGL/MG_Util/SelfTest/DriverPost.cpp index ab24e9d9..26d31e48 100644 --- a/MobileGL/MG_Util/SelfTest/DriverPost.cpp +++ b/MobileGL/MG_Util/SelfTest/DriverPost.cpp @@ -519,13 +519,12 @@ namespace MobileGL::MG_Util::SelfTest { "narrowed members, so an application that hard-codes std140 offsets " "computed for doubles must query them instead")); builder.Warn("64-bit vertex attributes", - "not supported (ES has no GL_DOUBLE vertex format, and after the fp64 demotion " - "above there is no 64-bit shader input left to feed either); " - "glVertexAttribLFormat / glVertexArrayAttribLFormat succeed and their state is " - "queryable, but an ENABLED 64-bit array is DROPPED at draw and the attribute " - "reads its generic current value - feed the attribute with " - "glVertexAttribPointer(GL_FLOAT) instead, which a demoted dvec input reads " - "correctly"); + "narrowed to float32 (ES has no GL_DOUBLE vertex format, and after the fp64 " + "demotion above there is no 64-bit shader input left to feed either); " + "glVertexAttribLFormat / glVertexArrayAttribLFormat succeed, their state is " + "queryable, and an ENABLED 64-bit array IS fetched - the source doubles are " + "deinterleaved into a float32 stream at draw, so values outside float32's " + "range or precision are rounded rather than exact"); if (glesFuncs.glPatchParameteri != nullptr) { builder.Pass("Tessellation patch parameters", "glPatchParameteri present (GL_PATCH_VERTICES reaches the driver)"); @@ -2337,12 +2336,12 @@ namespace MobileGL::MG_Util::SelfTest { "for doubles must query them instead", features.shaderFloat64 == VK_TRUE ? "supported" : "unsupported"))); builder.Warn("64-bit vertex attributes", - "not supported; there is no 64-bit shader input left to feed after the fp64 demotion " - "above, and no VK_FORMAT_R64*_SFLOAT vertex fetch to feed it with on most devices " - "anyway. glVertexAttribLFormat succeeds and its state is queryable, but an ENABLED " - "64-bit array is DROPPED at pipeline build and the attribute reads its generic " - "current value - feed the attribute with glVertexAttribPointer(GL_FLOAT) instead, " - "which a demoted dvec input reads correctly"); + "narrowed to float32; there is no 64-bit shader input left to feed after the fp64 " + "demotion above, and no VK_FORMAT_R64*_SFLOAT vertex fetch to feed it with on most " + "devices anyway. glVertexAttribLFormat succeeds, its state is queryable, and an " + "ENABLED 64-bit array IS fetched - the source doubles are deinterleaved into a " + "float32 stream at draw, so values outside float32's range or precision are " + "rounded rather than exact"); Bool shaderDrawParameters = false; if (vkGetPhysicalDeviceFeatures2Fn != nullptr && properties.apiVersion >= VK_API_VERSION_1_1) {