From c2dbcbee190fc8615ce6ad9a191ceacc09853d36 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Mon, 12 Oct 2026 08:18:15 -0400 Subject: [PATCH] [Fix] (GL): report GL_SAMPLE_POSITION from the backend's sample locations and check the index against the draw framebuffer's GL_SAMPLES --- MobileGL/MG_Backend/BackendObject.h | 31 +++++++++++ .../DirectGLES/BackendObject_DirectGLES.cpp | 54 +++++++++++++++++++ .../BackendObject_DirectVulkan.cpp | 22 ++++++++ .../MG_Impl/GLImpl/Texture/GL_Texture.cpp | 24 ++++++--- MobileGL/MG_Pipe/PipeFields.def | 3 +- MobileGL/MG_Pipe/generated/PipeWireLayout.inc | 2 + MobileGL/MG_Test/Texture/TextureTest.cpp | 42 +++++++++++++++ .../MG_Util/BackendLoaders/Vulkan/Loader.cpp | 2 + .../MG_Util/BackendLoaders/Vulkan/Loader.h | 3 ++ 9 files changed, 174 insertions(+), 9 deletions(-) diff --git a/MobileGL/MG_Backend/BackendObject.h b/MobileGL/MG_Backend/BackendObject.h index 85a88130..712f7713 100644 --- a/MobileGL/MG_Backend/BackendObject.h +++ b/MobileGL/MG_Backend/BackendObject.h @@ -573,6 +573,14 @@ namespace MobileGL { Uint32 SubgroupSize = 0; Uint32 SubgroupSupportedStages = 0; Uint32 SubgroupSupportedFeatures = 0; + // GL_SAMPLE_POSITION (GL 4.6 core 14.3.1.1) as the device actually places the samples + // of a 2-, 4-, 8- and 16-sample pixel: (x, y) pairs in [0, 1], the N-sample row starting + // at pair N - 2. Bit log2(N) of SamplePositionCountMask says row N was filled in; a row + // the backend could not determine stays unset rather than being guessed. Read and + // written through Get/SetSamplePositions only. + static constexpr Int kMaxSamplePositionCount = 16; + Float SamplePositions[60] = {}; // 2 * (2 + 4 + 8 + 16) + Uint32 SamplePositionCountMask = 0; Bool SubgroupQuadOperationsInAllStages = false; GpuVendorKind GpuVendor = GpuVendorKind::Unknown; // GL_EXT_fragment_shading_rate and its two companions. Each is a statement about the @@ -606,6 +614,29 @@ namespace MobileGL { Int MaxFragmentShadingRateAttachmentTexelHeight = 0; Int MaxFragmentShadingRateAttachmentTexelAspectRatio = 0; Int MaxFragmentShadingRateAttachmentLayers = 0; + + + static constexpr Bool IsSamplePositionCount(Int samples) { + return samples >= 2 && samples <= kMaxSamplePositionCount && (samples & (samples - 1)) == 0; + } + static constexpr Uint32 SamplePositionCountBit(Int samples) { + return 1u << static_cast(std::countr_zero(static_cast(samples))); + } + // `xy` holds `samples` (x, y) pairs. + void SetSamplePositions(Int samples, const Float* xy) { + if (!IsSamplePositionCount(samples)) return; + for (Int i = 0; i < 2 * samples; ++i) SamplePositions[2 * (samples - 2) + i] = xy[i]; + SamplePositionCountMask |= SamplePositionCountBit(samples); + } + Bool GetSamplePosition(Int samples, Int index, Float* outXY) const { + if (!IsSamplePositionCount(samples) || index < 0 || index >= samples || + (SamplePositionCountMask & SamplePositionCountBit(samples)) == 0) { + return false; + } + outXY[0] = SamplePositions[2 * (samples - 2 + index)]; + outXY[1] = SamplePositions[2 * (samples - 2 + index) + 1]; + return true; + } }; enum class WindowBackend { diff --git a/MobileGL/MG_Backend/DirectGLES/BackendObject_DirectGLES.cpp b/MobileGL/MG_Backend/DirectGLES/BackendObject_DirectGLES.cpp index f057bd3e..04a596ae 100644 --- a/MobileGL/MG_Backend/DirectGLES/BackendObject_DirectGLES.cpp +++ b/MobileGL/MG_Backend/DirectGLES/BackendObject_DirectGLES.cpp @@ -453,6 +453,59 @@ namespace MobileGL::MG_Backend::DirectGLES { return supported; } + // GL_SAMPLE_POSITION is a property of the multisample framebuffer the driver builds, so it + // is asked once per power-of-two sample count, of a throwaway RGBA8 renderbuffer framebuffer, + // through the driver's own glGetMultisamplefv (ES 3.1 core). A count the driver rounds up + // reports the first positions of the pattern it really uses - the samples gl_SampleID + // 0..count-1 address in a shader. Counts that cannot be built are left unset. + void ProbeSamplePositions(const MG_External::GLESFunctionsTable& gl, + const MG_External::GLESCapabilities& capabilities, + DynamicBackendParameters& dynamicParameters) { + const auto& version = capabilities.GLESVersion; + if (version.Major < 3 || (version.Major == 3 && version.Minor < 1)) return; + if (!gl.glGetMultisamplefv || !gl.glGenFramebuffers || !gl.glBindFramebuffer || + !gl.glFramebufferRenderbuffer || !gl.glCheckFramebufferStatus || !gl.glDeleteFramebuffers || + !gl.glGenRenderbuffers || !gl.glBindRenderbuffer || !gl.glRenderbufferStorageMultisample || + !gl.glDeleteRenderbuffers || !gl.glGetIntegerv) { + return; + } + + GLint prevFramebuffer = 0, prevRenderbuffer = 0; + gl.glGetIntegerv(GL_FRAMEBUFFER_BINDING, &prevFramebuffer); + gl.glGetIntegerv(GL_RENDERBUFFER_BINDING, &prevRenderbuffer); + for (Int samples = 2; samples <= DynamicBackendParameters::kMaxSamplePositionCount && + samples <= capabilities.MaxSamples; + samples *= 2) { + GLuint framebuffer = 0, renderbuffer = 0; + gl.glGenFramebuffers(1, &framebuffer); + gl.glGenRenderbuffers(1, &renderbuffer); + gl.glBindRenderbuffer(GL_RENDERBUFFER, renderbuffer); + gl.glRenderbufferStorageMultisample(GL_RENDERBUFFER, samples, GL_RGBA8, 4, 4); + gl.glBindFramebuffer(GL_FRAMEBUFFER, framebuffer); + gl.glFramebufferRenderbuffer(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_RENDERBUFFER, renderbuffer); + GLint actualSamples = 0; + if (gl.glCheckFramebufferStatus(GL_FRAMEBUFFER) == GL_FRAMEBUFFER_COMPLETE) { + gl.glGetIntegerv(GL_SAMPLES, &actualSamples); + } + if (actualSamples >= samples) { + Float positions[2 * DynamicBackendParameters::kMaxSamplePositionCount] = {}; + for (Int index = 0; index < samples; ++index) { + gl.glGetMultisamplefv(GL_SAMPLE_POSITION, static_cast(index), &positions[2 * index]); + } + dynamicParameters.SetSamplePositions(samples, positions); + } + gl.glBindFramebuffer(GL_FRAMEBUFFER, static_cast(prevFramebuffer)); + gl.glBindRenderbuffer(GL_RENDERBUFFER, static_cast(prevRenderbuffer)); + gl.glDeleteFramebuffers(1, &framebuffer); + gl.glDeleteRenderbuffers(1, &renderbuffer); + } + // A count the driver refused leaves its error behind; it must not reach the application. + if (gl.glGetError) { + for (Int drained = 0; drained < 16 && gl.glGetError() != GL_NO_ERROR; ++drained) { + } + } + } + Bool ProbeFramebufferCompletenessForRenderbuffer(const MG_External::GLESFunctionsTable& gl, GLuint renderbuffer, TextureInternalFormat format) { GLuint framebuffer = 0; @@ -1868,6 +1921,7 @@ namespace MobileGL::MG_Backend::DirectGLES { clampStageImageUniforms(m_GLESCapabilities.MaxComputeImageUniforms); m_dynamicParameters.SupportsDistinctDepthStencilAttachments = ProbeDistinctDepthStencilAttachments(DirectGLES::g_GLESFuncs); + ProbeSamplePositions(DirectGLES::g_GLESFuncs, m_GLESCapabilities, m_dynamicParameters); // SyncAttachmentObject routes a layered upload target to glFramebufferTextureLayer with the // attachment's layer passed through, so this backend really does render to the layer it was // given - provided the driver resolved the entry point at all. diff --git a/MobileGL/MG_Backend/DirectVulkan/BackendObject_DirectVulkan.cpp b/MobileGL/MG_Backend/DirectVulkan/BackendObject_DirectVulkan.cpp index 65a00426..91a92174 100644 --- a/MobileGL/MG_Backend/DirectVulkan/BackendObject_DirectVulkan.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/BackendObject_DirectVulkan.cpp @@ -1201,6 +1201,28 @@ namespace MobileGL::MG_Backend::DirectVulkan { m_dynamicParameters.MaxIntegerSamples = m_vulkanCaps.MaxIntegerSamples; m_dynamicParameters.MaxSamples = m_vulkanCaps.MaxSamples; m_dynamicParameters.MaxSampleMaskWords = m_vulkanCaps.MaxSampleMaskWords; + // GL_SAMPLE_POSITION. A device reporting standardSampleLocations places the samples exactly + // at the Vulkan specification's table (Multisampling, "Standard sample locations"), which + // is also what gl_SamplePosition reads back in a shader. Vulkan's sample y grows with the + // framebuffer row, and a framebuffer object's row 0 is GL's y = 0, so the table needs no + // flip to be GL's answer. Without the flag the positions are unknown and stay unset. + if (m_vulkanCaps.StandardSampleLocations) { + static constexpr Float kStandard2[] = {0.75f, 0.75f, 0.25f, 0.25f}; + static constexpr Float kStandard4[] = {0.375f, 0.125f, 0.875f, 0.375f, 0.125f, 0.625f, 0.625f, 0.875f}; + static constexpr Float kStandard8[] = {0.5625f, 0.3125f, 0.4375f, 0.6875f, 0.8125f, 0.5625f, + 0.3125f, 0.1875f, 0.1875f, 0.8125f, 0.0625f, 0.4375f, + 0.6875f, 0.9375f, 0.9375f, 0.0625f}; + static constexpr Float kStandard16[] = { + 0.5625f, 0.5625f, 0.4375f, 0.3125f, 0.3125f, 0.625f, 0.75f, 0.4375f, 0.1875f, 0.375f, 0.625f, + 0.8125f, 0.8125f, 0.6875f, 0.6875f, 0.1875f, 0.375f, 0.875f, 0.5f, 0.0625f, 0.25f, 0.125f, + 0.125f, 0.75f, 0.0f, 0.5f, 0.9375f, 0.25f, 0.875f, 0.9375f, 0.0625f, 0.0f}; + const Float* const tables[] = {kStandard2, kStandard4, kStandard8, kStandard16}; + for (Int samples = 2, row = 0; samples <= decltype(m_dynamicParameters)::kMaxSamplePositionCount && + samples <= m_vulkanCaps.MaxSamples; + samples *= 2, ++row) { + m_dynamicParameters.SetSamplePositions(samples, tables[row]); + } + } const Int maxSupportedTextureUnits = static_cast(MG_State::GLState::TextureState::MAX_TEXTURE_IMAGE_UNITS); // GL_MAX_TEXTURE_IMAGE_UNITS is a *per-stage* sampler limit. Adreno/Qualcomm report a huge // maxPerStageDescriptorSampledImages (descriptor-indexing scale), so clamping it only to our diff --git a/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp b/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp index a0cd486b..297212e4 100644 --- a/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp +++ b/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp @@ -7208,19 +7208,27 @@ namespace MobileGL::MG_Impl::GLImpl { return; } - const Int maxSamples = MG_Backend::pActiveBackendObject != nullptr - ? std::max(MG_Backend::pActiveBackendObject->GetDynamicParameters().MaxSamples, 1) - : 1; - if (static_cast(index) >= maxSamples) { + // GL 4.6 core 14.3.1.1: a sample of the DRAW framebuffer, which has GL_SAMPLES of them - so + // a single-sample framebuffer has none to ask about. + const GLint samples = ResolveDrawFramebufferSampleCount(); + if (static_cast(index) >= samples) { MG_State::pGLContext->RecordError( ErrorCode::InvalidValue, - MakeUnique("MG_Impl/GLImpl", __func__, "Sample index is out of range.")); + MakeUnique("MG_Impl/GLImpl", __func__, + "Sample index is not below the draw framebuffer's GL_SAMPLES.")); return; } - // Keep sample positions deterministic even before the backend exposes vendor-specific patterns. - val[0] = 0.5f; - val[1] = 0.5f; + // The backend publishes where its device really places the samples of each count (a + // shader's gl_SamplePosition reads the same places). A count it could not determine keeps + // answering the pixel centre. + Float position[2] = {0.5f, 0.5f}; + if (MG_Backend::pActiveBackendObject != nullptr) { + MG_Backend::pActiveBackendObject->GetDynamicParameters().GetSamplePosition( + samples, static_cast(index), position); + } + val[0] = position[0]; + val[1] = position[1]; } void TexSubImage3D(GLenum target, GLint level, GLint xoffset, GLint yoffset, GLint zoffset, GLsizei width, diff --git a/MobileGL/MG_Pipe/PipeFields.def b/MobileGL/MG_Pipe/PipeFields.def index b5d74103..db129fb2 100644 --- a/MobileGL/MG_Pipe/PipeFields.def +++ b/MobileGL/MG_Pipe/PipeFields.def @@ -390,7 +390,8 @@ F(SupportsDistinctDepthStencilAttachments) F(PerLayerFramebufferAttachmentTargets) \ F(SupportsShaderFloat64) F(SupportsFloat64VertexAttributes) F(SupportsTessellationPointSize) \ F(SupportsGeometryPointSize) F(MaxShaderStorageBlockSize) F(SubgroupSize) \ - F(SubgroupSupportedStages) F(SubgroupSupportedFeatures) F(SubgroupQuadOperationsInAllStages) \ + F(SubgroupSupportedStages) F(SubgroupSupportedFeatures) F(SamplePositions) \ + F(SamplePositionCountMask) F(SubgroupQuadOperationsInAllStages) \ F(GpuVendor) F(SupportsFragmentShadingRate) F(SupportsPrimitiveFragmentShadingRate) \ F(SupportsAttachmentFragmentShadingRate) F(FragmentShadingRateNonTrivialCombiners) \ F(FragmentShadingRateWithShaderDepthStencilWrites) F(FragmentShadingRateWithSampleMask) \ diff --git a/MobileGL/MG_Pipe/generated/PipeWireLayout.inc b/MobileGL/MG_Pipe/generated/PipeWireLayout.inc index 28935029..df514012 100644 --- a/MobileGL/MG_Pipe/generated/PipeWireLayout.inc +++ b/MobileGL/MG_Pipe/generated/PipeWireLayout.inc @@ -664,6 +664,8 @@ inline constexpr WireLayoutMember kMGPipeWireLayoutMembers[] = { {"DynamicBackendParameters.SubgroupSize", offsetof(DynamicBackendParameters, SubgroupSize), sizeof(((DynamicBackendParameters*)nullptr)->SubgroupSize)}, {"DynamicBackendParameters.SubgroupSupportedStages", offsetof(DynamicBackendParameters, SubgroupSupportedStages), sizeof(((DynamicBackendParameters*)nullptr)->SubgroupSupportedStages)}, {"DynamicBackendParameters.SubgroupSupportedFeatures", offsetof(DynamicBackendParameters, SubgroupSupportedFeatures), sizeof(((DynamicBackendParameters*)nullptr)->SubgroupSupportedFeatures)}, + {"DynamicBackendParameters.SamplePositions", offsetof(DynamicBackendParameters, SamplePositions), sizeof(((DynamicBackendParameters*)nullptr)->SamplePositions)}, + {"DynamicBackendParameters.SamplePositionCountMask", offsetof(DynamicBackendParameters, SamplePositionCountMask), sizeof(((DynamicBackendParameters*)nullptr)->SamplePositionCountMask)}, {"DynamicBackendParameters.SubgroupQuadOperationsInAllStages", offsetof(DynamicBackendParameters, SubgroupQuadOperationsInAllStages), sizeof(((DynamicBackendParameters*)nullptr)->SubgroupQuadOperationsInAllStages)}, {"DynamicBackendParameters.GpuVendor", offsetof(DynamicBackendParameters, GpuVendor), sizeof(((DynamicBackendParameters*)nullptr)->GpuVendor)}, {"DynamicBackendParameters.SupportsFragmentShadingRate", offsetof(DynamicBackendParameters, SupportsFragmentShadingRate), sizeof(((DynamicBackendParameters*)nullptr)->SupportsFragmentShadingRate)}, diff --git a/MobileGL/MG_Test/Texture/TextureTest.cpp b/MobileGL/MG_Test/Texture/TextureTest.cpp index a8a1764d..da3c6294 100644 --- a/MobileGL/MG_Test/Texture/TextureTest.cpp +++ b/MobileGL/MG_Test/Texture/TextureTest.cpp @@ -414,6 +414,48 @@ TEST_F(TextureTest, ClearTexImageRejectsCompressedTextures) { EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); } +// GL 4.6 core 14.3.1.1 (KHR-GL4x sample_variables.position.fixed): GL_SAMPLE_POSITION answers where +// the backend places the samples of the draw framebuffer's sample count, and an index at or past +// that framebuffer's GL_SAMPLES is INVALID_VALUE - every index, for a single-sample framebuffer. +TEST_F(TextureTest, SamplePositionComesFromTheBackendForTheDrawFramebufferSampleCount) { + auto& params = FormatCapabilityBackend::MutableDynamicParameters(); + const auto savedParams = params; + params.MaxSamples = params.MaxColorTextureSamples = params.MaxDepthTextureSamples = 4; + params.MaxIntegerSamples = params.MaxFramebufferSamples = 4; + const Float fourSamples[] = {0.375f, 0.125f, 0.875f, 0.375f, 0.125f, 0.625f, 0.625f, 0.875f}; + params.SetSamplePositions(4, fourSamples); + ScopedBackendOverride override(MakeUnique()); + + GLuint texture = 0; + MG_Impl::GLImpl::CreateTextures(GL_TEXTURE_2D_MULTISAMPLE, 1, &texture); + MG_Impl::GLImpl::TextureStorage2DMultisample(texture, 4, GL_RGBA8, 8, 8, GL_TRUE); + GLuint framebuffer = 0; + MG_Impl::GLImpl::CreateFramebuffers(1, &framebuffer); + MG_Impl::GLImpl::NamedFramebufferTexture(framebuffer, GL_COLOR_ATTACHMENT0, texture, 0); + MG_Impl::GLImpl::BindFramebuffer(GL_DRAW_FRAMEBUFFER, framebuffer); + ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); + + for (GLuint index = 0; index < 4; ++index) { + GLfloat position[2] = {-1.0f, -1.0f}; + MG_Impl::GLImpl::GetMultisamplefv(GL_SAMPLE_POSITION, index, position); + EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); + EXPECT_EQ(position[0], fourSamples[2 * index]) << "sample " << index; + EXPECT_EQ(position[1], fourSamples[2 * index + 1]) << "sample " << index; + } + GLfloat position[2] = {}; + MG_Impl::GLImpl::GetMultisamplefv(GL_SAMPLE_POSITION, 4, position); + ExpectSingleGlError(GL_INVALID_VALUE); + + MG_Impl::GLImpl::BindFramebuffer(GL_DRAW_FRAMEBUFFER, 0); + MG_Impl::GLImpl::GetMultisamplefv(GL_SAMPLE_POSITION, 0, position); + ExpectSingleGlError(GL_INVALID_VALUE); + + MG_Impl::GLImpl::DeleteFramebuffers(1, &framebuffer); + MG_Impl::GLImpl::DeleteTextures(1, &texture); + params = savedParams; + DrainPendingGlErrors(); +} + // GL_MAX_TEXTURE_MAX_ANISOTROPY_EXT is float state that must answer every numeric query: GetFloatv // is authoritative and GetIntegerv would otherwise fall through to its INVALID_ENUM default. TEST_F(TextureTest, MaxTextureMaxAnisotropyIsAnsweredFromTheBackendLimit) { diff --git a/MobileGL/MG_Util/BackendLoaders/Vulkan/Loader.cpp b/MobileGL/MG_Util/BackendLoaders/Vulkan/Loader.cpp index ad01a739..afc42a12 100644 --- a/MobileGL/MG_Util/BackendLoaders/Vulkan/Loader.cpp +++ b/MobileGL/MG_Util/BackendLoaders/Vulkan/Loader.cpp @@ -187,6 +187,7 @@ namespace MobileGL::MG_Util::BackendLoader { // reproduced on this backend. One word is the spec minimum and therefore always legal. // It is also what PipelineCreatePayload::sampleMask is sized for. caps.MaxSampleMaskWords = std::min(SaturateToInt(p.limits.maxSampleMaskWords), 1); + caps.StandardSampleLocations = p.limits.standardSampleLocations == VK_TRUE; caps.MaxTextureImageUnits = SaturateToInt(p.limits.maxPerStageDescriptorSampledImages); caps.MaxVertexTextureImageUnits = SaturateToInt(p.limits.maxPerStageDescriptorSampledImages); caps.MaxComputeTextureImageUnits = SaturateToInt(p.limits.maxPerStageDescriptorSampledImages); @@ -329,6 +330,7 @@ namespace MobileGL::MG_Util::BackendLoader { // reproduced on this backend. One word is the spec minimum and therefore always legal. // It is also what PipelineCreatePayload::sampleMask is sized for. caps.MaxSampleMaskWords = std::min(SaturateToInt(properties.limits.maxSampleMaskWords), 1); + caps.StandardSampleLocations = properties.limits.standardSampleLocations == VK_TRUE; caps.MaxTextureImageUnits = SaturateToInt(properties.limits.maxPerStageDescriptorSampledImages); caps.MaxVertexTextureImageUnits = SaturateToInt(properties.limits.maxPerStageDescriptorSampledImages); caps.MaxComputeTextureImageUnits = SaturateToInt(properties.limits.maxPerStageDescriptorSampledImages); diff --git a/MobileGL/MG_Util/BackendLoaders/Vulkan/Loader.h b/MobileGL/MG_Util/BackendLoaders/Vulkan/Loader.h index 5a3cef61..dc7e88a6 100644 --- a/MobileGL/MG_Util/BackendLoaders/Vulkan/Loader.h +++ b/MobileGL/MG_Util/BackendLoaders/Vulkan/Loader.h @@ -47,6 +47,9 @@ namespace MobileGL { Int MaxIntegerSamples = 1; Int MaxSamples = 1; Int MaxSampleMaskWords = 1; + // VkPhysicalDeviceLimits::standardSampleLocations: the device places the samples of + // every supported count at the standard locations the Vulkan specification tabulates. + Bool StandardSampleLocations = false; Int MaxTextureImageUnits = 32; Int MaxVertexTextureImageUnits = 32; Int MaxComputeTextureImageUnits = 32;