From 5b6dec2d81d9ffbe87df0ebb9bfc7e9929af7747 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Tue, 11 Aug 2026 06:30:30 -0400 Subject: [PATCH] [Revert] (MG_Backend/DirectVulkan): back out the gl_FragCoord default-framebuffer origin fix - correct, but it costs DirectVulkan a large order-dependent slowdown that is not yet root-caused --- .../DirectVulkan/Renderer/ProgramFactory.cpp | 252 +----------------- .../DirectVulkan/Renderer/ProgramFactory.h | 21 -- .../DirectVulkan/Renderer/VulkanRenderer.cpp | 17 -- MobileGL/MG_IntegrationTest/CMakeLists.txt | 1 - .../Scenarios/FragCoordOriginScenario.cpp | 139 ---------- 5 files changed, 2 insertions(+), 428 deletions(-) delete mode 100644 MobileGL/MG_IntegrationTest/Scenarios/FragCoordOriginScenario.cpp diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.cpp index 85f54fc8..c121f133 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.cpp @@ -937,189 +937,6 @@ namespace MobileGL::MG_Backend::DirectVulkan { ProgramFactory::CompileOptionFlags m_transformFlags; }; - // gl_FragCoord back into GL's window space, for default-framebuffer draws only. - // - // Vulkan's gl_FragCoord.y is the framebuffer ROW being written - not a value the - // viewport rect can move independently of placement. The default framebuffer's image is - // stored display-side-up and the vertex stage compensates by negating gl_Position.y, so - // for every default-FBO draw the framebuffer row of a fragment is exactly - // `height - y_GL` (the viewport terms cancel: yf_VK = H - yf_GL for any viewport rect). - // A shader that reads gl_FragCoord therefore sees a flipped Y, and once the viewport - // rect started being converted to the stored orientation it also sees a Y that is - // OUTSIDE the range GL promises - a 32-pixel-tall viewport at GL y=0 reports 224..255 on - // a 256-tall surface. GL CTS shader_image_load_store writes imageStore(image, - // ivec2(gl_FragCoord.xy)) into an image exactly the size of that viewport, so every - // store fell outside the image and the test read back zeroes. - // - // The rewrite redirects every read of the builtin to a Private copy initialised once at - // entry, which is exact for all access forms (whole-vector loads, `.y` access chains, - // OpCopyMemory) and leaves the builtin itself - and its decorations - untouched. - class GlFragCoordYFlipPass final : public spvtools::opt::Pass { - public: - const char* name() const override { return "mobilegl-fragcoord-y-flip"; } - explicit GlFragCoordYFlipPass(Uint32 framebufferHeight) : m_framebufferHeight(framebufferHeight) {} - - Status Process() override { - using namespace spvtools::opt; - if (m_framebufferHeight == 0) return Status::SuccessWithoutChange; - - Instruction* entryPoint = nullptr; - for (auto& candidate : get_module()->entry_points()) { - if (candidate.NumInOperands() >= 2 && - static_cast(candidate.GetSingleWordInOperand(0)) == - spv::ExecutionModel::Fragment) { - entryPoint = &candidate; - break; - } - } - if (!entryPoint) return Status::SuccessWithoutChange; - - const Uint32 builtinVarId = FindFragCoordVariable(); - if (builtinVarId == 0) return Status::SuccessWithoutChange; - - Instruction* builtinVar = context()->get_def_use_mgr()->GetDef(builtinVarId); - if (!builtinVar || builtinVar->opcode() != spv::Op::OpVariable) return Status::SuccessWithoutChange; - - // The builtin is `Input vec4`; take the vector and component types from its own - // pointer type rather than assuming float32x4, so a module that spells it - // differently declines instead of miscompiling. - Instruction* inputPtrType = context()->get_def_use_mgr()->GetDef(builtinVar->type_id()); - if (!inputPtrType || inputPtrType->opcode() != spv::Op::OpTypePointer) { - return Status::SuccessWithoutChange; - } - const Uint32 vectorTypeId = inputPtrType->GetSingleWordInOperand(1); - Instruction* vectorType = context()->get_def_use_mgr()->GetDef(vectorTypeId); - if (!vectorType || vectorType->opcode() != spv::Op::OpTypeVector || - vectorType->GetSingleWordInOperand(1) != 4) { - return Status::SuccessWithoutChange; - } - const Uint32 floatTypeId = vectorType->GetSingleWordInOperand(0); - auto* floatType = context()->get_type_mgr()->GetType(floatTypeId); - if (!floatType || !floatType->AsFloat() || floatType->AsFloat()->width() != 32) { - return Status::SuccessWithoutChange; - } - - const auto heightBits = std::bit_cast(static_cast(m_framebufferHeight)); - const auto* heightConst = context()->get_constant_mgr()->GetConstant(floatType, {heightBits}); - auto* heightInst = context()->get_constant_mgr()->GetDefiningInstruction(heightConst); - if (!heightInst) return Status::SuccessWithoutChange; - - auto* function = context()->GetFunction(entryPoint->GetSingleWordInOperand(1)); - if (!function || function->begin() == function->end()) return Status::SuccessWithoutChange; - - const Uint32 privatePtrTypeId = - context()->get_type_mgr()->FindPointerToType(vectorTypeId, spv::StorageClass::Private); - if (privatePtrTypeId == 0) return Status::SuccessWithoutChange; - - const Uint32 copyVarId = context()->TakeNextId(); - if (copyVarId == 0) return Status::SuccessWithoutChange; - auto copyVar = std::make_unique( - context(), spv::Op::OpVariable, privatePtrTypeId, copyVarId, - std::initializer_list{ - {SPV_OPERAND_TYPE_STORAGE_CLASS, {static_cast(spv::StorageClass::Private)}}}); - context()->AddGlobalValue(std::move(copyVar)); - - // Redirect the reads BEFORE emitting the initialiser, so the initialiser's own - // load of the builtin is not rewritten into a load of the (still empty) copy. - if (!RedirectReads(builtinVarId, copyVarId)) return Status::SuccessWithoutChange; - - auto& entryBlock = *function->begin(); - auto insertPoint = entryBlock.begin(); - while (insertPoint != entryBlock.end() && insertPoint->opcode() == spv::Op::OpVariable) { - ++insertPoint; - } - if (insertPoint == entryBlock.end()) return Status::SuccessWithoutChange; - - InstructionBuilder builder(context(), &*insertPoint, - IRContext::kAnalysisDefUse | IRContext::kAnalysisInstrToBlockMapping); - auto* raw = builder.AddLoad(vectorTypeId, builtinVarId); - if (!raw) return Status::SuccessWithoutChange; - auto* x = builder.AddCompositeExtract(floatTypeId, raw->result_id(), {0}); - auto* y = builder.AddCompositeExtract(floatTypeId, raw->result_id(), {1}); - auto* z = builder.AddCompositeExtract(floatTypeId, raw->result_id(), {2}); - auto* w = builder.AddCompositeExtract(floatTypeId, raw->result_id(), {3}); - if (!x || !y || !z || !w) return Status::SuccessWithoutChange; - auto* flippedY = - builder.AddBinaryOp(floatTypeId, spv::Op::OpFSub, heightInst->result_id(), y->result_id()); - if (!flippedY) return Status::SuccessWithoutChange; - auto* corrected = builder.AddCompositeConstruct( - vectorTypeId, {x->result_id(), flippedY->result_id(), z->result_id(), w->result_id()}); - if (!corrected) return Status::SuccessWithoutChange; - if (!builder.AddStore(copyVarId, corrected->result_id())) return Status::SuccessWithoutChange; - - // SPIR-V 1.4 widened the entry-point interface to every global the entry point - // statically uses, Private included; earlier versions accept Input/Output only, - // so listing it there would be invalid. - if (get_module()->version() >= 0x00010400u) { - entryPoint->AddOperand({SPV_OPERAND_TYPE_ID, {copyVarId}}); - context()->AnalyzeUses(entryPoint); - } - - context()->InvalidateAnalysesExceptFor(spvtools::opt::IRContext::kAnalysisDefUse | - spvtools::opt::IRContext::kAnalysisInstrToBlockMapping); - return Status::SuccessWithChange; - } - - private: - Uint32 FindFragCoordVariable() const { - for (const auto& annotation : get_module()->annotations()) { - if (annotation.opcode() != spv::Op::OpDecorate) continue; - if (annotation.NumInOperands() < 3) continue; - if (static_cast(annotation.GetSingleWordInOperand(1)) != - spv::Decoration::BuiltIn) { - continue; - } - if (static_cast(annotation.GetSingleWordInOperand(2)) != spv::BuiltIn::FragCoord) { - continue; - } - return annotation.GetSingleWordInOperand(0); - } - return 0; - } - - // Every instruction that reads through the builtin's POINTER gets the copy instead. - // Decorations, names and the entry-point interface keep naming the builtin. - Bool RedirectReads(Uint32 builtinVarId, Uint32 copyVarId) { - using namespace spvtools::opt; - Bool ok = true; - Vector users; - context()->get_def_use_mgr()->ForEachUser(builtinVarId, [&](Instruction* user) { - switch (user->opcode()) { - case spv::Op::OpLoad: - case spv::Op::OpAccessChain: - case spv::Op::OpInBoundsAccessChain: - case spv::Op::OpPtrAccessChain: - case spv::Op::OpInBoundsPtrAccessChain: - case spv::Op::OpCopyMemory: - case spv::Op::OpCopyMemorySized: - users.push_back(user); - break; - case spv::Op::OpStore: - // gl_FragCoord is read-only; a store through it means this is not the - // module we think it is. - ok = false; - break; - default: - break; - } - }); - if (!ok) return false; - for (Instruction* user : users) { - for (Uint32 i = 0; i < user->NumInOperands(); ++i) { - auto& operand = user->GetInOperand(i); - if (operand.type == SPV_OPERAND_TYPE_ID && !operand.words.empty() && - operand.words[0] == builtinVarId) { - operand.words[0] = copyVarId; - } - } - context()->AnalyzeUses(user); - } - return true; - } - - Uint32 m_framebufferHeight = 0; - }; - // Decorates the module's captured varyings for VK_EXT_transform_feedback: // user outputs get XfbBuffer/XfbStride/Offset directly; a captured // gl_Position (a gl_PerVertex member) is mirrored into a dedicated output @@ -1484,35 +1301,6 @@ namespace MobileGL::MG_Backend::DirectVulkan { return spvtools::Optimizer::PassToken(MakeUnique(transformFlags)); } - Bool TransformSpirvForFragCoordYFlip(const Vector& input, Vector& output, - Uint32 framebufferHeight) { - if (input.empty()) { - output.clear(); - return true; - } - if (framebufferHeight == 0) { - output = input; - return true; - } - - spvtools::Optimizer optimizer(SPV_ENV_VULKAN_1_3); - spvtools::OptimizerOptions options; - options.set_run_validator(false); // see TransformSpirvForExplicitLod0Sampling - optimizer.SetMessageConsumer([](spv_message_level_t, const char*, const spv_position_t&, - const char* message) { - MGLOG_E("Vulkan: fragcoord y-flip pass: %s", message != nullptr ? message : ""); - }); - optimizer.RegisterPass( - spvtools::Optimizer::PassToken(MakeUnique(framebufferHeight))); - - const Bool success = optimizer.Run(input.data(), input.size(), &output, options); - if (!success) { - MGLOG_E("Vulkan: failed to run the gl_FragCoord y-flip pass; keeping the original module"); - output = input; - } - return success; - } - Bool TransformSpirvForXfbCapture(const Vector& input, Vector& output, const MG_State::GLState::ProgramObject& program) { if (input.empty()) { @@ -1984,12 +1772,6 @@ namespace MobileGL::MG_Backend::DirectVulkan { XXHASH_VERIFY(XXH64_update(m_hashState, spv.data(), spv.size() * sizeof(Uint))); } XXHASH_VERIFY(XXH64_update(m_hashState, &flags, sizeof(CompileOptionFlags))); - // Only FragCoordYFlip variants bake the height in, so mixing it unconditionally would - // re-key every program in the cache on a resize for no reason. - if (flags & CompileOptionBit::FragCoordYFlip) { - XXHASH_VERIFY(XXH64_update(m_hashState, &m_defaultFramebufferHeight, - sizeof(m_defaultFramebufferHeight))); - } // Include UBO block bindings in hash so different binding configurations produce different entries const Uint32 blockCount = static_cast(program.GetActiveUniformBlocksCount()); @@ -2598,36 +2380,14 @@ namespace MobileGL::MG_Backend::DirectVulkan { } } - void ProgramFactory::SetDefaultFramebufferHeight(Uint32 height) { - if (m_defaultFramebufferHeight == height) { - return; - } - m_defaultFramebufferHeight = height; - // Both memos key on (program, flags) alone, so neither can tell the two heights apart: - // drop the lookup memo, and bump the structure epoch so every caller holding a - // VkProgramObject* re-runs GetOrCreateProgram and lands on the new hash. The cached - // entries themselves stay - they are keyed by a hash that now includes the old height, - // so they can only be reached again if that height comes back, and the frame-boundary - // sweep retires them otherwise. - m_lastLookup = {}; - ++m_cacheStructureEpoch; - } - const ProgramFactory::VkProgramObject& ProgramFactory::GetOrCreateProgram( const MG_State::GLState::ProgramObject& program, CompileOptionFlags flags) { // Hashing the full SPIR-V of every stage is far too expensive to repeat per draw; // reuse the program's memoized hash while its backend state version is unchanged. - // The memo keys on the flags word, which ComputeHash is no longer a pure function of: - // a FragCoordYFlip variant also depends on the baked default-framebuffer height, so - // that height rides in the free high half of the key. Flags occupy the low bits, and a - // height cannot exceed the 16 bits a swapchain extent fits in. - const Uint memoKey = (flags & CompileOptionBit::FragCoordYFlip) - ? (flags.GetRaw() | (m_defaultFramebufferHeight << 16)) - : flags.GetRaw(); HashType hash = 0; - if (!program.GetBackendHashMemo(memoKey, hash)) { + if (!program.GetBackendHashMemo(flags.GetRaw(), hash)) { hash = ComputeHash(program, flags); - program.SetBackendHashMemo(memoKey, hash); + program.SetBackendHashMemo(flags.GetRaw(), hash); } auto it = m_cache.find(hash); if (it != m_cache.end()) { @@ -2680,14 +2440,6 @@ namespace MobileGL::MG_Backend::DirectVulkan { } } - if ((flags & ProgramFactory::CompileOptionBit::FragCoordYFlip) && shaders[i] && - shaders[i]->GetShaderStage() == ShaderStage::Fragment) { - Vector fragCoordSpirv; - if (TransformSpirvForFragCoordYFlip(moduleSpirvs[i], fragCoordSpirv, m_defaultFramebufferHeight)) { - moduleSpirvs[i] = Move(fragCoordSpirv); - } - } - // Vulkan's SPIR-V environment has no rectangle image dimension, so a // GL_TEXTURE_RECTANGLE lookup has to become the 2D one the texture is really // stored as - which addresses [0,1] where the application addressed texels. diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.h index c34fda0b..942dd698 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.h @@ -53,13 +53,6 @@ namespace MobileGL::MG_Backend::DirectVulkan { // recorded while GL transform feedback is active, so plain draws keep the // undecorated variant. XfbCapture = 1 << 6, - // Rewrites the fragment stage's gl_FragCoord reads to GL's bottom-left window - // origin. Vulkan's gl_FragCoord.y IS the framebuffer row being written, and the - // default framebuffer's image is stored in display (top-left) order, so a shader - // that reads gl_FragCoord there sees `height - y_GL`. Set together with - // PositionYFlip (the two are the same fact about the same draws) except under a - // quarter turn, which this renderer does not convert rectangles for either. - FragCoordYFlip = 1 << 7, }; using CompileOptionFlags = Flags; using HashType = Uint64; @@ -274,17 +267,6 @@ namespace MobileGL::MG_Backend::DirectVulkan { const VkProgramObject& GetOrCreateProgram( const MG_State::GLState::ProgramObject& program, CompileOptionFlags flags); - // The default framebuffer's current image height, baked as a literal into every - // FragCoordYFlip variant (there is no push-constant or specialization channel here, and - // adding one for a value that changes only on swapchain recreation would cost the draw - // path more than a recompile costs a resize). It is therefore part of those variants' - // identity: ComputeHash mixes it in when the bit is set, so a height change re-keys them - // and leaves every other program's hash untouched. Setting a NEW height also bumps the - // cache-structure epoch, because a caller holding a memoised VkProgramObject* would - // otherwise keep using a module compiled against the old height. - void SetDefaultFramebufferHeight(Uint32 height); - Uint32 GetDefaultFramebufferHeight() const { return m_defaultFramebufferHeight; } - // Bumped whenever m_cache's STRUCTURE changes (any insert or erase): the cache is // an open-addressing map holding entries by value, so both moves existing entries. // A caller that memoised a VkProgramObject* may keep dereferencing it only while @@ -342,9 +324,6 @@ namespace MobileGL::MG_Backend::DirectVulkan { // True only when the logical device enabled both // shaderStorageImageReadWithoutFormat and shaderStorageImageWriteWithoutFormat. Bool m_unformattedFloatStorageImagesEnabled = false; - // See SetDefaultFramebufferHeight. 0 means "not known yet"; the FragCoordYFlip bit is - // never set before the swapchain exists, so no variant can be compiled against it. - Uint32 m_defaultFramebufferHeight = 0; mutable ProgramLookupCache m_lastLookup; // Monotonic frame-boundary counter (bumped in OnFrameBoundary) for cache aging. Uint64 m_frameCounter = 0; diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp index 6445fc8d..d0c0a74e 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp @@ -2779,14 +2779,6 @@ void main() { MG_State::pGLContext->GetFramebufferBindingSlot(FramebufferTarget::Draw).GetBoundObject(); if (currentDrawFBO != nullptr && currentDrawFBO->IsDefaultFramebuffer()) { flags |= ProgramFactory::CompileOptionBit::PositionYFlip; - // gl_FragCoord follows the same rule the default-framebuffer RECTANGLES follow - // (GetDefaultFramebufferRectMapping): flipped for identity/180, left alone under a - // quarter turn, which this renderer converts nothing for. Keeping the two in step - // is the whole point - a fragment's window Y and the viewport that placed it must - // agree on which end of the image they count from. - if (!IsQuarterTurnPreTransform(preTransform)) { - flags |= ProgramFactory::CompileOptionBit::FragCoordYFlip; - } switch (preTransform) { case VK_SURFACE_TRANSFORM_ROTATE_90_BIT_KHR: flags |= ProgramFactory::CompileOptionBit::SurfaceRotate90; @@ -2941,9 +2933,6 @@ void main() { m_shaderDrawParametersFeatureEnabled, m_unformattedFloatStorageImagesEnabled); MOBILEGL_ASSERT(m_programFactory != nullptr, "ProgramFactory creation failed."); - // The swapchain already exists at this point (Initialize creates it first), so seed the - // height the factory could not be told about from CreateSwapchain. - m_programFactory->SetDefaultFramebufferHeight(m_swapchainObject.GetExtent().height); // Aging evictions (render passes and program entries) must purge the dependent // pipeline / compute-pipeline / descriptor-set caches in the same step; both // sweeps only run from the frame-boundary seams, long after initialization. @@ -12032,12 +12021,6 @@ void main() { static_cast(m_physicalDevice.queueFamilies.graphicsFamily), static_cast(m_physicalDevice.queueFamilies.presentFamily), m_config.MaxFramesInFlight, desiredExtent); - // The FragCoordYFlip variants bake this height in; it is the only input to a shader - // module that lives outside the GL program, so the factory has to learn it here (and on - // every recreation, which is the only way it can change). - if (m_programFactory) { - m_programFactory->SetDefaultFramebufferHeight(m_swapchainObject.GetExtent().height); - } } void VulkanRenderer::CreateCommandPool() { diff --git a/MobileGL/MG_IntegrationTest/CMakeLists.txt b/MobileGL/MG_IntegrationTest/CMakeLists.txt index 77f82712..b2ca128f 100644 --- a/MobileGL/MG_IntegrationTest/CMakeLists.txt +++ b/MobileGL/MG_IntegrationTest/CMakeLists.txt @@ -56,7 +56,6 @@ add_executable(MobileGLIntegrationTest Scenarios/PipelineFailureScenario.cpp Scenarios/AdvertisedLimitsScenario.cpp Scenarios/PixelStoreSweepScenario.cpp - Scenarios/FragCoordOriginScenario.cpp ) target_include_directories(MobileGLIntegrationTest PRIVATE diff --git a/MobileGL/MG_IntegrationTest/Scenarios/FragCoordOriginScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/FragCoordOriginScenario.cpp deleted file mode 100644 index 9ae737c4..00000000 --- a/MobileGL/MG_IntegrationTest/Scenarios/FragCoordOriginScenario.cpp +++ /dev/null @@ -1,139 +0,0 @@ -// MobileGL - MobileGL/MG_IntegrationTest/Scenarios/FragCoordOriginScenario.cpp -// Copyright (c) 2025-2026 MobileGL-Dev -// Licensed under the GNU Lesser General Public License v3.0: -// https://www.gnu.org/licenses/gpl-3.0.txt -// https://www.gnu.org/licenses/lgpl-3.0.txt -// SPDX-License-Identifier: LGPL-3.0-only -// End of Source File Header -// -// Scenario - gl_FragCoord ON THE DEFAULT FRAMEBUFFER CARRIES GL'S WINDOW ORIGIN. -// -// GL measures gl_FragCoord.y from the BOTTOM of the window. Vulkan's gl_FragCoord.y is the -// framebuffer ROW being written, and DirectVulkan stores the default framebuffer display-side-up -// (compensating for vertices by negating gl_Position.y), so a fragment's reported Y there was -// `height - y_GL` - flipped, and for a viewport that does not span the full height, outside the -// range GL promises entirely. GL CTS -// `KHR-GL42.shader_image_load_store.basic-{allTargets-atomic,glsl-earlyFragTests,glsl-misc}` -// caught it: each sets a small viewport at GL y=0 and does -// `imageStore(image, ivec2(gl_FragCoord.xy), ...)` into an image exactly that size, so on a -// 256-tall surface every store addressed rows 224..255 of a 32-row image and was dropped. -// -// The shader here paints each row with its own GL window Y, which is the whole claim in one -// value: row j of the readback must be j, for a full-height viewport and for a half-height one -// (the case where a flip and an offset can no longer hide each other). DirectGLES is the -// built-in control - a native GL driver gets this right by construction, so a failure there -// would mean the test, not the backend. - -#include -#include -#include - -#include "../Harness/HeadlessGL.h" -#include "../Harness/ScenarioFixture.h" - -#ifdef GLAPI -#undef GLAPI -#endif -#define GL_GLEXT_PROTOTYPES -#include -#include -#undef GL_GLEXT_PROTOTYPES - -namespace MGITest { - namespace { - - constexpr const char* kVS = R"(#version 330 core -in vec2 aPos; -void main() { gl_Position = vec4(aPos, 0.0, 1.0); } -)"; - - // floor(gl_FragCoord.y) is the fragment's window row; 1/255 steps survive an RGBA8 - // round trip exactly, so the readback byte IS the row the shader believes it is on. - constexpr const char* kFS = R"(#version 330 core -out vec4 o_color; -void main() { o_color = vec4(floor(gl_FragCoord.y) / 255.0, 0.0, 0.0, 1.0); } -)"; - - class FragCoordOriginScenario : public ScenarioTest {}; - - // A quad covering the whole viewport, drawn with attribute 0 = aPos. - void DrawFullViewportQuad(unsigned int program) { - static const float kQuad[] = {-1.0f, -1.0f, 1.0f, -1.0f, -1.0f, 1.0f, 1.0f, 1.0f}; - GLuint vao = 0, vbo = 0; - glGenVertexArrays(1, &vao); - glBindVertexArray(vao); - glGenBuffers(1, &vbo); - glBindBuffer(GL_ARRAY_BUFFER, vbo); - glBufferData(GL_ARRAY_BUFFER, sizeof(kQuad), kQuad, GL_STATIC_DRAW); - glEnableVertexAttribArray(0); - glVertexAttribPointer(0, 2, GL_FLOAT, GL_FALSE, 2 * sizeof(float), nullptr); - glUseProgram(program); - glDrawArrays(GL_TRIANGLE_STRIP, 0, 4); - glBindVertexArray(0); - glDeleteBuffers(1, &vbo); - glDeleteVertexArrays(1, &vao); - } - - // Paints `viewportHeight` rows starting at GL y=0 and returns the red byte of each row. - std::vector RowsPaintedWithTheirOwnWindowY(unsigned int program, int width, int viewportHeight) { - BindDefaultFramebuffer(); - glViewport(0, 0, width, viewportHeight); - glDisable(GL_SCISSOR_TEST); - glDisable(GL_DEPTH_TEST); - ClearTo(0.0f, 0.0f, 1.0f, 1.0f); - DrawFullViewportQuad(program); - - const Image image = ReadPixelsRect(0, 0, width, viewportHeight); - std::vector rows; - rows.reserve(static_cast(viewportHeight)); - for (int y = 0; y < viewportHeight; ++y) { - rows.push_back(image.At(width / 2, y).r); - } - return rows; - } - - ::testing::AssertionResult RowsAreTheirOwnIndex(const std::vector& rows, const char* when) { - for (std::size_t y = 0; y < rows.size(); ++y) { - if (rows[y] != static_cast(y)) { - return ::testing::AssertionFailure() - << when << ": GL window row " << y << " reported gl_FragCoord.y = " << rows[y] - << " (expected " << y << "). Rows 0.." << (rows.size() - 1) << " read back as [" - << rows.front() << " .. " << rows.back() << "]."; - } - } - return ::testing::AssertionSuccess(); - } - - } // namespace - - TEST_F(FragCoordOriginScenario, DefaultFramebufferFragCoordCountsFromTheBottom) { - if (!Ready()) return; - HeadlessGL& gl = Gl(); - // 1/255 steps only stay distinguishable while the row index fits in a byte. - const int width = gl.Width(); - const int fullHeight = std::min(gl.Height(), 256); - ASSERT_GE(fullHeight, 8) << "the harness surface is too small to tell rows apart"; - - std::string error; - const unsigned int program = CompileProgram(kVS, kFS, &error); - ASSERT_NE(program, 0u) << error; - - // Full height first: this one passed even before the fix (a flip alone maps the row set - // onto itself), so it is the control that the shader and the readback agree at all. - EXPECT_TRUE(RowsAreTheirOwnIndex(RowsPaintedWithTheirOwnWindowY(program, width, fullHeight), - "full-height viewport")); - - // Half height at GL y=0: the case the CTS failures were made of. A backend that reports - // the stored row here answers `height - y` for every row - off the bottom of the range, - // not merely reversed within it. - const int halfHeight = fullHeight / 2; - EXPECT_TRUE(RowsAreTheirOwnIndex(RowsPaintedWithTheirOwnWindowY(program, width, halfHeight), - "half-height viewport at GL y=0")); - - glUseProgram(0); - glDeleteProgram(program); - glViewport(0, 0, gl.Width(), gl.Height()); - EXPECT_EQ(FirstGLError(), 0u); - } - -} // namespace MGITest