mirror of
https://github.com/ARMSX2/ARMSX2.git
synced 2026-08-24 16:50:16 -07:00
GS/OpenGL: decide framebuffer fetch once, in a policy function
CheckFeatures decided m_features.framebuffer_fetch three times across roughly a hundred lines. The last of them -- the Mali profile block -- tested the raw GL_ARM_shader_framebuffer_fetch extension instead of the decision the earlier two had already made, and set the flag unconditionally back to true. So both the r44p1 driver guard and the user's DisableFramebufferFetch setting were undone a tenth of a millisecond after they ran, and there was no way to turn framebuffer fetch off on Mali GL from settings at all. The device log stated the contradiction in plain language -- "Mali r44p1: disabling framebuffer fetch" followed by "Active framebuffer fetch backend (Mali profile): ARM" -- which is why this is about where the decision lives, not about the condition itself. Move it to DecideGLFramebufferFetch in GSFramebufferFetchPolicy.h: one pure constexpr function, all inputs explicit, no GL types. CheckFeatures assigns m_features.framebuffer_fetch once from its result and nothing downstream writes that flag again. Two behavioural points fall out of separating them: - Turning fetch off no longer drags a Mali device to the PowerVR profile. The demotion is what it always was, a property of the extension set (a Mali profile that cannot reach the ARM shader path is on the wrong profile), but a driver blocklist or a user setting is a blend-path choice and must not swap in another vendor's tuning as a side effect. - With fetch off on Mali GLES, texture_barrier already resolves to false at the Auto branch above (ARB/NV texture barrier do not exist on GLES), so the non-fetch copy blend path the r44p1 comment intends is what actually runs. The block's own texture_barrier assignment was redundant in every reachable case and is now only a log line. The backend selection now mirrors tfx_fs.glsl exactly, including its `#elif HAS_ARM_SHADER_FRAMEBUFFER_FETCH` fallback for non-Mali profiles, and the reason fetch is off rides on the same line as the verdict. Verified on an M2 Max under Mesa (GL, EXT fetch): the setting-off arm now logs "backend (Generic profile): None (disabled in settings)" and the setting-on arm "backend (Generic profile): EXT/PLS". The Mali arm needs an Android build.
This commit is contained in:
Brian Degenhardt
@@ -584,6 +584,7 @@ set(pcsx2GSHeaders
|
||||
GS/Renderers/Common/GSDevice.h
|
||||
GS/Renderers/Common/GSDirtyRect.h
|
||||
GS/Renderers/Common/GSFastList.h
|
||||
GS/Renderers/Common/GSFramebufferFetchPolicy.h
|
||||
GS/Renderers/Common/GSFunctionMap.h
|
||||
GS/Renderers/Common/GSGPUProfile.h
|
||||
GS/Renderers/Common/GSGPUProfilePrivate.h
|
||||
|
||||
@@ -0,0 +1,101 @@
|
||||
// SPDX-FileCopyrightText: 2026 ARMSX2 Contributors
|
||||
// SPDX-License-Identifier: GPL-3.0+
|
||||
|
||||
#pragma once
|
||||
|
||||
// The OpenGL framebuffer-fetch decision, as one pure function.
|
||||
//
|
||||
// It used to be made imperatively in three places roughly a hundred lines apart in
|
||||
// GSDeviceOGL::CheckFeatures, and the last of them -- the Mali profile block -- tested the raw
|
||||
// GL_ARM_shader_framebuffer_fetch extension instead of the decision the first two had already
|
||||
// made. So on a Mali r44p1 device the driver guard turned fetch off and the profile block turned
|
||||
// it straight back on, 0.1 ms apart in the same log; DisableFramebufferFetch was eaten the same
|
||||
// way, leaving no way to turn fetch off on Mali GL from settings at all. The bug was not subtle
|
||||
// -- the log states the contradiction in plain language -- it survived because there was no
|
||||
// single place a reader or a test could look at to see what the decision was.
|
||||
//
|
||||
// Hence: one function, all inputs explicit, no GL types, constexpr so the cases below are pinned
|
||||
// at compile time and again by name in gs_framebuffer_fetch_policy_tests.cpp.
|
||||
|
||||
enum class GSFramebufferFetchBackend
|
||||
{
|
||||
None,
|
||||
ARM, // gl_LastFragColorARM (GL_ARM_shader_framebuffer_fetch)
|
||||
EXT, // `inout` colour output (GL_EXT_shader_framebuffer_fetch / pixel local storage)
|
||||
};
|
||||
|
||||
// Why fetch is off. Carried out of the policy so the caller can log the specific reason and raise
|
||||
// the OSD message for the one case that is a user setting rather than a hardware fact.
|
||||
enum class GSFramebufferFetchVeto
|
||||
{
|
||||
None,
|
||||
NoExtension, // neither ARM nor EXT fetch is advertised
|
||||
DriverBlocklist, // a driver build known to render it incorrectly
|
||||
UserSetting, // GSConfig.DisableFramebufferFetch
|
||||
};
|
||||
|
||||
struct GSFramebufferFetchDecision
|
||||
{
|
||||
bool enabled = false;
|
||||
GSFramebufferFetchBackend backend = GSFramebufferFetchBackend::None;
|
||||
GSFramebufferFetchVeto veto = GSFramebufferFetchVeto::NoExtension;
|
||||
|
||||
// A Mali profile that cannot reach the ARM shader path has to move to the PowerVR profile,
|
||||
// which shares the EXT/PLS arm with the catch-all default.
|
||||
bool demote_mali_to_powervr = false;
|
||||
};
|
||||
|
||||
// `driver_blocklisted` is the caller's driver-version test (currently Mali r44p1, which loses the
|
||||
// GL context under the in-tile blend path exactly as it loses the Vulkan device under
|
||||
// attachment-feedback-loop). `mali_profile` is the runtime GPU profile, which is what tfx_fs.glsl
|
||||
// keys its backend selection off -- not the extension set.
|
||||
constexpr GSFramebufferFetchDecision DecideGLFramebufferFetch(bool has_arm_fetch, bool has_ext_fetch,
|
||||
bool has_pls_fetch, bool driver_blocklisted, bool user_disabled, bool mali_profile)
|
||||
{
|
||||
GSFramebufferFetchDecision decision;
|
||||
|
||||
// Demotion is a property of the EXTENSIONS alone. A driver blocklist or the user's setting
|
||||
// turns fetch off, and turning fetch off is not a reason to move the device to a different
|
||||
// profile: the Mali profile still wants its own texture-preference and shader tuning, it
|
||||
// just takes the non-fetch (copy) blend path like any GPU without the extension. Demoting
|
||||
// there would silently swap in PowerVR's tuning as a side effect of a correctness gate.
|
||||
decision.demote_mali_to_powervr = mali_profile && !has_arm_fetch;
|
||||
const bool effective_mali_profile = mali_profile && !decision.demote_mali_to_powervr;
|
||||
|
||||
if (!has_arm_fetch && !has_ext_fetch)
|
||||
decision.veto = GSFramebufferFetchVeto::NoExtension;
|
||||
else if (driver_blocklisted)
|
||||
decision.veto = GSFramebufferFetchVeto::DriverBlocklist;
|
||||
else if (user_disabled)
|
||||
decision.veto = GSFramebufferFetchVeto::UserSetting;
|
||||
else
|
||||
decision.veto = GSFramebufferFetchVeto::None;
|
||||
|
||||
decision.enabled = (decision.veto == GSFramebufferFetchVeto::None);
|
||||
|
||||
// Mirrors the `#if GPU_PROFILE_MALI` selection in tfx_fs.glsl: Mali reads back through
|
||||
// gl_LastFragColorARM even when EXT is also advertised, because the EXT inout path is broken
|
||||
// on every Mali driver tested; everything else prefers the EXT/PLS inout output and falls
|
||||
// back to the ARM builtin only when EXT is absent.
|
||||
if (!decision.enabled)
|
||||
decision.backend = GSFramebufferFetchBackend::None;
|
||||
else if (effective_mali_profile && has_arm_fetch)
|
||||
decision.backend = GSFramebufferFetchBackend::ARM;
|
||||
else if (has_ext_fetch || has_pls_fetch)
|
||||
decision.backend = GSFramebufferFetchBackend::EXT;
|
||||
else
|
||||
decision.backend = GSFramebufferFetchBackend::ARM;
|
||||
|
||||
return decision;
|
||||
}
|
||||
|
||||
// The regression itself: a blocklisted driver, or the user's setting, must survive the Mali
|
||||
// profile -- and must not drag the profile to PowerVR on the way.
|
||||
static_assert(!DecideGLFramebufferFetch(true, true, true, true, false, true).enabled);
|
||||
static_assert(!DecideGLFramebufferFetch(true, true, true, true, false, true).demote_mali_to_powervr);
|
||||
static_assert(!DecideGLFramebufferFetch(true, true, true, false, true, true).enabled);
|
||||
static_assert(!DecideGLFramebufferFetch(true, true, true, false, true, true).demote_mali_to_powervr);
|
||||
static_assert(DecideGLFramebufferFetch(true, true, true, false, false, true).enabled);
|
||||
static_assert(DecideGLFramebufferFetch(true, true, true, false, false, true).backend ==
|
||||
GSFramebufferFetchBackend::ARM);
|
||||
static_assert(DecideGLFramebufferFetch(false, true, true, false, false, true).demote_mali_to_powervr);
|
||||
@@ -4,6 +4,7 @@
|
||||
#include "GS/Renderers/OpenGL/GLContext.h"
|
||||
#include "GS/Renderers/OpenGL/GSDeviceOGL.h"
|
||||
#include "GS/Renderers/OpenGL/GLState.h"
|
||||
#include "GS/Renderers/Common/GSFramebufferFetchPolicy.h"
|
||||
#include "GS/Renderers/Common/GSGPUProfile.h"
|
||||
#include "GS/GSState.h"
|
||||
#include "GS/Renderers/Common/GSRenderer.h"
|
||||
@@ -1050,28 +1051,32 @@ bool GSDeviceOGL::CheckFeatures()
|
||||
m_features.dual_source_blend =
|
||||
!m_is_gles || GLAD_GL_EXT_blend_func_extended || GLAD_GL_ARB_blend_func_extended;
|
||||
|
||||
m_features.framebuffer_fetch = (GLAD_GL_ARM_shader_framebuffer_fetch || GLAD_GL_EXT_shader_framebuffer_fetch);
|
||||
|
||||
// The framebuffer-fetch decision is made ONCE, here, by DecideGLFramebufferFetch (see
|
||||
// GSFramebufferFetchPolicy.h for why it is a separate pure function). Nothing below may write
|
||||
// m_features.framebuffer_fetch -- read `fbfetch` instead if you need to know what was decided.
|
||||
//
|
||||
// The Mali r44p1 blob loses the rendering context under the in-tile framebuffer-fetch blend path,
|
||||
// exactly as it loses the Vulkan device under attachment-feedback-loop (VK_ERROR_DEVICE_LOST on
|
||||
// effectively every game -- Mali-G615 r44p1). Mirror the Vulkan-side r44p1 gate (see GSDeviceVK.cpp)
|
||||
// and drop this one blob to the non-fetch (copy) blend path. Narrow by driver version, not vendor,
|
||||
// so other (working) Mali blobs keep the fast path. GL_VERSION reads e.g. "OpenGL ES 3.2 v1.r44p1-...".
|
||||
if (m_features.framebuffer_fetch)
|
||||
{
|
||||
const char* gl_version = reinterpret_cast<const char*>(glGetString(GL_VERSION));
|
||||
if (gl_version && std::strstr(gl_version, "r44p1"))
|
||||
{
|
||||
Console.WriteLn("Mali r44p1: disabling framebuffer fetch (GL context-lost workaround; matches the Vulkan gate).");
|
||||
m_features.framebuffer_fetch = false;
|
||||
}
|
||||
}
|
||||
const bool fbfetch_driver_blocklisted = (std::strstr(gl_version_str, "r44p1") != nullptr);
|
||||
const GSFramebufferFetchDecision fbfetch = DecideGLFramebufferFetch(GLAD_GL_ARM_shader_framebuffer_fetch,
|
||||
GLAD_GL_EXT_shader_framebuffer_fetch, GLAD_GL_EXT_shader_pixel_local_storage, fbfetch_driver_blocklisted,
|
||||
GSConfig.DisableFramebufferFetch, use_mali_profile);
|
||||
m_features.framebuffer_fetch = fbfetch.enabled;
|
||||
|
||||
if (m_features.framebuffer_fetch && GSConfig.DisableFramebufferFetch)
|
||||
switch (fbfetch.veto)
|
||||
{
|
||||
Host::AddOSDMessage(
|
||||
"Framebuffer fetch was found but is disabled. This will reduce performance.", Host::OSD_ERROR_DURATION);
|
||||
m_features.framebuffer_fetch = false;
|
||||
case GSFramebufferFetchVeto::DriverBlocklist:
|
||||
Console.WriteLn("Mali r44p1: disabling framebuffer fetch (GL context-lost workaround; matches the Vulkan gate).");
|
||||
break;
|
||||
case GSFramebufferFetchVeto::UserSetting:
|
||||
Host::AddOSDMessage(
|
||||
"Framebuffer fetch was found but is disabled. This will reduce performance.", Host::OSD_ERROR_DURATION);
|
||||
break;
|
||||
default:
|
||||
break;
|
||||
}
|
||||
|
||||
if (GSConfig.OverrideTextureBarriers == 0)
|
||||
@@ -1160,15 +1165,23 @@ bool GSDeviceOGL::CheckFeatures()
|
||||
// device was force-overridden to Mali but lacks ARM fbfetch (rare but
|
||||
// possible), demote to PowerVR profile which uses the same EXT/PLS path the
|
||||
// catch-all default uses.
|
||||
if (GLAD_GL_ARM_shader_framebuffer_fetch)
|
||||
//
|
||||
// ⚠️ This block must NOT re-enable framebuffer fetch, and nothing here may write
|
||||
// m_features.framebuffer_fetch. It used to set it unconditionally true off the raw
|
||||
// GLAD_GL_ARM_shader_framebuffer_fetch extension rather than the decision made ~100 lines
|
||||
// above, which resurrected fetch after both the r44p1 driver guard and the user's
|
||||
// DisableFramebufferFetch setting -- so on Mali GL there was no way to turn fetch off at
|
||||
// all. Demotion stays keyed on the extension because that is what it has always meant (a
|
||||
// Mali profile that cannot reach the ARM shader path is on the wrong profile), but fetch
|
||||
// being switched off is a blend-path choice, not a reason to change profile.
|
||||
if (!fbfetch.demote_mali_to_powervr)
|
||||
{
|
||||
Console.WriteLn(Color_Yellow, "GL: Applying Mali-specific optimizations for tile-based rendering.");
|
||||
m_features.framebuffer_fetch = true;
|
||||
if (GSConfig.OverrideTextureBarriers == -1)
|
||||
{
|
||||
m_features.texture_barrier = m_features.framebuffer_fetch;
|
||||
// texture_barrier already reflects the fetch decision: on GLES the ARB/NV barrier
|
||||
// extensions are absent, so the Auto branch above resolves to exactly
|
||||
// framebuffer_fetch. Nothing left to override here -- only to report.
|
||||
if (m_features.framebuffer_fetch && GSConfig.OverrideTextureBarriers == -1)
|
||||
Console.WriteLn("GL: Mali optimization - using ARM framebuffer fetch over texture barriers.");
|
||||
}
|
||||
}
|
||||
else
|
||||
{
|
||||
@@ -1223,29 +1236,25 @@ bool GSDeviceOGL::CheckFeatures()
|
||||
}
|
||||
|
||||
{
|
||||
const bool has_arm_fetch = GLAD_GL_ARM_shader_framebuffer_fetch;
|
||||
const bool has_ext_fetch = GLAD_GL_EXT_shader_framebuffer_fetch;
|
||||
const bool has_pls_fetch = GLAD_GL_EXT_shader_pixel_local_storage;
|
||||
Console.WriteLn("GL: Framebuffer fetch extension caps: arm=%d ext=%d pls=%d.",
|
||||
has_arm_fetch ? 1 : 0, has_ext_fetch ? 1 : 0, has_pls_fetch ? 1 : 0);
|
||||
GLAD_GL_ARM_shader_framebuffer_fetch ? 1 : 0, GLAD_GL_EXT_shader_framebuffer_fetch ? 1 : 0,
|
||||
GLAD_GL_EXT_shader_pixel_local_storage ? 1 : 0);
|
||||
|
||||
const char* active_profile_name = use_mali_profile ? "Mali" :
|
||||
(use_powervr_profile ? "PowerVR" :
|
||||
(use_adreno_profile ? "Adreno" : "Generic"));
|
||||
const char* active_fetch_backend = "None";
|
||||
if (m_features.framebuffer_fetch)
|
||||
{
|
||||
if (use_mali_profile)
|
||||
active_fetch_backend = "ARM";
|
||||
else if (has_ext_fetch || has_pls_fetch)
|
||||
active_fetch_backend = "EXT/PLS";
|
||||
else if (has_arm_fetch)
|
||||
active_fetch_backend = "ARM";
|
||||
}
|
||||
Console.WriteLn("GL: Active framebuffer fetch backend (%s profile): %s.", active_profile_name, active_fetch_backend);
|
||||
|
||||
if (use_mali_profile && !has_arm_fetch)
|
||||
Console.Warning("GL: Mali profile selected but ARM framebuffer fetch is unavailable; using non-fetch fallback.");
|
||||
const char* active_fetch_backend =
|
||||
(fbfetch.backend == GSFramebufferFetchBackend::ARM) ? "ARM" :
|
||||
((fbfetch.backend == GSFramebufferFetchBackend::EXT) ? "EXT/PLS" : "None");
|
||||
// The reason rides on the same line as the verdict, deliberately: the resurrection bug
|
||||
// this policy replaced printed "disabling framebuffer fetch" and "backend: ARM" a tenth of
|
||||
// a millisecond apart, and neither line said what had decided it.
|
||||
const char* fetch_veto_reason =
|
||||
(fbfetch.veto == GSFramebufferFetchVeto::NoExtension) ? " (no fetch extension)" :
|
||||
((fbfetch.veto == GSFramebufferFetchVeto::DriverBlocklist) ? " (blocked for this driver build)" :
|
||||
((fbfetch.veto == GSFramebufferFetchVeto::UserSetting) ? " (disabled in settings)" : ""));
|
||||
Console.WriteLn("GL: Active framebuffer fetch backend (%s profile): %s%s.", active_profile_name,
|
||||
active_fetch_backend, fetch_veto_reason);
|
||||
}
|
||||
|
||||
if (GLAD_GL_ARB_shader_storage_buffer_object)
|
||||
|
||||
Reference in New Issue
Block a user