GL state handling: fixes from a state-API review

doc/ARCHITECTURE.md

@@ -121,7 +121,7 @@ Depth of field runs **before** bloom and before the tonemapper, on linear HDR. G
 - **Auto-exposure:** Progressive histogram (`gLuminanceProgram`, `gExposureProgram`) with history fade
 - **Lens flare sun state:** a 2x1 temporal pass (`gLensFlareStateProgram`, `lensFlareStateF.glsl`) run just before colour correction measures how much of the sun disc is unoccluded and its colour, and filters both under a rate limit; `computeLensFlare` in `postEffectUtilsF.glsl` reads one texel of it. The shader is the source of truth for the filter; `scripts/content_tools/check_lens_flare_state.py` mirrors it and is its regression test
 - **Bloom:** Bright-area extraction → downsample/upsample pyramid over `bloomMip[]` with warmth correction and optional halation carried in alpha. The additive composite is **not** a pass of its own: it is folded into `colorCorrect`'s `BLOOM_COMPOSITE` permutation. `compositeBloomHDR` is a standalone equivalent that nothing currently calls
-- **Cross-screen (star) filter:** `gCrossFilterProgram` streaks every thresholded highlight, seeded from `bloomMip[0]` and accumulated through three quadrants of `mWaterDis`, which the two shaders address through `uCrossRegion`/`uCrossClamp` so a tap at the quadrant's edge clamps exactly as it did at a dedicated target's edge. One strictly one-sided three-pass chain per arm, at strides that tile the reachable offsets exactly once — the tiling is load-bearing and the shader explains why. Composited in `colorCorrect` alongside the pyramid, so it inherits bloom strength
+- **Cross-screen (star) filter:** `gCrossFilterProgram` streaks every thresholded highlight, seeded from `bloomMip[0]` and accumulated through three quadrants of `mWaterDis`, which the two shaders address through `uCrossRegion`/`uCrossClamp` so a tap at the quadrant's edge clamps exactly as it did at a dedicated target's edge. One strictly one-sided three-pass chain per arm, at strides that tile the reachable offsets exactly once — the tiling is load-bearing and the shader explains why. Composited in `colorCorrect` alongside the pyramid, so it inherits bloom strength. Each pass reads a quadrant the pass before it wrote while `mWaterDis` is still attached, which GL defines only across `glTextureBarrier`, so the filter runs only where that resolved (GL 4.5, or `GL_ARB_texture_barrier`/`GL_NV_texture_barrier`) and the Lightbox hides its sections elsewhere
 - **Lens dirt:** `gLensDirtGenProgram` draws the grime plate into `mLensDirtMap` — dust motes, wipe smudges, stray fibres, fine grit and optional scratches, built from hashes and distance fields rather than loaded from an image. Not a per-frame pass: it runs when a generation parameter moves or the window resizes, guarded by the cached parameter set in `mLensDirtParams`, which is also what makes a failed allocation stop retrying. Generated at the frame's own resolution, so nothing has to fit a square plate to a wide window. A rebuild is held off while `mLensDirtSliderHeld` is raised — the Lightbox raises it on a generation slider's mouse-down and lowers it on mouse-up, so a drag costs one plate on release rather than one per frame. Everything that is not a drag (typed values, resets, applying a Look, undo, window resizes) rebuilds immediately, which is what a settle timer would have delayed for no reason. `colorCorrect` multiplies the accumulated bloom and flare terms by it
 - **Depth of Field:** three passes — circle of confusion (`gDeferredCoFProgram`, writing sharp linear colour plus signed CoF in alpha), a gather blur at `CameraDoFResScale`, and a combine (`gDeferredDoFCombineProgram`) back over `mRT->screen` under `setColorMask(true, false)` so the legacy prim-glow alpha tag survives. It owns no targets: the sharp copy borrows `mWaterDis`, which the water passes refill each frame and have finished with, and the blur borrows `bloomMip[0]` between the luminance read and the extract pass (`postPongMap` without HDR). The blur viewport is clamped to what mip 0 can hold when `RenderBloomResolutionScale` shrinks it, and the combine is told the fraction it actually covers via `res_scale` and where it sits via `dof_uv_scale`. The gather has four compile-time variants over two axes, `FRONT_BLUR` × `DOF_SHAPED`: `gDeferredPostProgram`, `gDeferredPostProgramNoNear`, `gDeferredPostProgramShaped`, `gDeferredPostProgramNoNearShaped`. Optics: `CameraFNumber`, `CameraFocalLength`, `CameraFieldOfView`, `CameraMaxCoF`, `CameraDoFResScale`
 - **Bokeh shaping** (inside the `DOF_SHAPED` gather): polygonal aperture (`RenderBokehApertureBlades`, `Rotation`, `Curvature`), anamorphic squeeze, cat's-eye optical vignetting, defocus fringing, and the lens aberrations — spherical (`RenderBokehSphericalAberration`), field stretch for swirl and coma (`RenderBokehFieldStretch`, `FieldFalloff`) and comatic asymmetry. The CPU picks a shaped variant only when one of them is actually doing something

doc/LIGHTBOX.md

@@ -1223,7 +1223,10 @@ only; apply per row, not on the parent panel. Reference patterns:
   Advanced section apply while `RenderHDREnabled` is on (on GL above 4.05, the
   pipeline's own test), and so do Cross Filter and its Advanced section, which
   streak what the bloom pyramid holds; Glow (Legacy) and its Advanced section
-  apply while it is off. An effect that only runs on one side of the fork
+  apply while it is off. Cross Filter also needs a texture barrier, so on a
+  driver with neither GL 4.5 nor a texture barrier extension its sections stay
+  hidden under HDR too, as the renderer leaves it off there. An effect that only
+  runs on one side of the fork, or only with some capability,
   belongs in the table in `refreshBloomSections()`, not just behind an
   `enabled_control`. `refreshBloomSections()` hides the other side's accordion tabs
   and re-arranges the accordion, live on the setting's signal. A tab's

indra/llrender/altextureslot.cpp

@@ -390,19 +390,20 @@ bool ALTextureSlot::bind(LLCubeMap* cubeMap, ALSampler key)
             mCurrTexture = cubeMap->mImages[0]->getTexName();
             glBindTexture(GL_TEXTURE_CUBE_MAP, mCurrTexture);
             cubeMap->mImages[0]->updateBindStats();
-            // Named by the caller, not read off face 0. A cube map is as much a shared
-            // resource as any other texture -- the environment map is sampled by every
-            // shiny surface in the scene -- so which sampler a pass wants is the pass's
-            // business.
-            bindSampler(gGL.getSampler(key));
-            return true;
         }
         else
         {
             LL_WARNS() << "Using cube map without extension!" << LL_ENDL;
             return false;
         }
     }
+
+    // Named by the caller, not read off face 0. A cube map is as much a shared
+    // resource as any other texture -- the environment map is sampled by every
+    // shiny surface in the scene -- so which sampler a pass wants is the pass's
+    // business. Outside the redundancy check, as in bindImpl: the same cube can be
+    // bound again after a pass that left a different sampler on this slot.
+    bindSampler(gGL.getSampler(key));
     return true;
 }
 

indra/llrender/llfonttextcache.cpp

@@ -76,13 +76,16 @@ namespace
 bool ALFontCacheKey::environmentMoved(const LLFontGL* fontp)
 {
     const U64 font_cache_gen = fontp ? fontp->getCacheGeneration() : 0;
+    // The capture keeps the sampler's GL name, which a sampler rebuild deletes.
+    const U32 sampler_gen = gGL.getSamplerGeneration();
     if (mFont == fontp
         && mScaleX == LLFontGL::sScaleX
         && mScaleY == LLFontGL::sScaleY
         && mVertDPI == LLFontGL::sVertDPI
         && mHorizDPI == LLFontGL::sHorizDPI
         && mResGeneration == LLFontGL::sResolutionGeneration
-        && mFontCacheGen == font_cache_gen)
+        && mFontCacheGen == font_cache_gen
+        && mSamplerGen == sampler_gen)
     {
         return false;
     }
@@ -94,6 +97,7 @@ bool ALFontCacheKey::environmentMoved(const LLFontGL* fontp)
     mHorizDPI      = LLFontGL::sHorizDPI;
     mResGeneration = LLFontGL::sResolutionGeneration;
     mFontCacheGen  = font_cache_gen;
+    mSamplerGen    = sampler_gen;
     return true;
 }
 

indra/llrender/llfonttextcache.h

@@ -146,6 +146,7 @@ class ALFontCacheKey
     F32             mHorizDPI = 0.f;
     S32             mResGeneration = 0;
     U64             mFontCacheGen = 0;
+    U32             mSamplerGen = 0;    // 0 is never a generation, so the first ask records
 };
 
 // Everything one piece of text costs to draw, kept so it can be reused: the

indra/llrender/llgl.cpp

@@ -2420,6 +2420,18 @@ void LLGLManager::initExtensions()
         glGetnUniformuiv         = (PFNGLGETNUNIFORMUIVPROC)LL_GET_PROC_ADDRESS("glGetnUniformuiv");
     }
 
+    // Texture barrier. Core in 4.5, and GL_ARB_texture_barrier names it without a suffix on a
+    // lower context, so it is resolved here for the GL_VERSION_4_5 ladder's early-out as above.
+    // GL_NV_texture_barrier, which ARB's took over unchanged, names it glTextureBarrierNV.
+    if (mGLVersion >= 4.49f || mGLExtensions.contains("GL_ARB_texture_barrier"))
+    {
+        glTextureBarrier = (PFNGLTEXTUREBARRIERPROC)LL_GET_PROC_ADDRESS("glTextureBarrier");
+    }
+    if (!glTextureBarrier && mGLExtensions.contains("GL_NV_texture_barrier"))
+    {
+        glTextureBarrier = (PFNGLTEXTUREBARRIERPROC)LL_GET_PROC_ADDRESS("glTextureBarrierNV");
+    }
+
     // GL_EXT_memory_object
     if (mHasEXTMemoryObject)
     {
@@ -3292,6 +3304,11 @@ GLboolean LLGLDepthTest::sWriteEnabled = GL_TRUE; // OpenGL default
 //static
 void LLGLState::initClass()
 {
+    // Runs per context, from initGL: the map and the depth state describe this context, and
+    // GL starts every context at its defaults.
+    sStateMap.clear();
+    LLGLDepthTest::initClass();
+
     sStateMap[GL_DITHER] = GL_TRUE;
     // sStateMap[GL_TEXTURE_2D] = GL_TRUE;
 
@@ -3677,6 +3694,15 @@ GLenum LLGLDepthTest::remap(GLenum func)
     }
 }
 
+//static
+void LLGLDepthTest::initClass()
+{
+    sDepthEnabled = GL_FALSE;
+    sDepthFunc = GL_LESS;
+    sWriteEnabled = GL_TRUE;
+    glDepthFunc(remap(sDepthFunc));
+}
+
 void LLGLDepthTest::rebase()
 {
     // Force the physical depth func to agree with the current translation of the tracked

indra/llrender/llgl.h

@@ -319,6 +319,10 @@ class LLGLState
     static void initClass();
     static void restoreGL();
 
+    // Record a cap set with a raw glEnable/glDisable, so a scoped state does not take it for
+    // GL's default. The map describes the main context; call on the main thread only.
+    static void seedState(LLGLenum state, LLGLboolean enabled) { sStateMap[state] = enabled; }
+
     static void resetTextureStates();
     static void dumpStates();
 
@@ -334,6 +338,9 @@ class LLGLState
     enum { CURRENT_STATE = -2, DISABLED_STATE = 0, ENABLED_STATE = 1 };
     LLGLState(LLGLenum state, S32 enabled = CURRENT_STATE);
     ~LLGLState();
+    // A copy would restore the cap a second time.
+    LLGLState(const LLGLState&) = delete;
+    LLGLState& operator=(const LLGLState&) = delete;
     void setEnabled(S32 enabled);
     void enable() { setEnabled(ENABLED_STATE); }
     void disable() { setEnabled(DISABLED_STATE); }

indra/llrender/llglslshader.cpp

@@ -484,6 +484,14 @@ LLGLSLShader::~LLGLSLShader()
     // than a leak. Recursion covers the subtree: each child erases itself on the way out.
     sInstances.erase(this);
 
+    // Nor may the bound-program statics name this object. They are cleared without a GL call,
+    // for the reason below.
+    if (sCurBoundShaderPtr == this)
+    {
+        sCurBoundShaderPtr = nullptr;
+        sCurBoundShader = 0;
+    }
+
     // Free the owned subtree's OBJECTS, so a program destroyed without unload() does not leak
     // them. Deliberately not unload(): a global program's destructor runs at static destruction
     // with no GL context, where deleting program objects is undefined -- and the driver reclaims
@@ -644,20 +652,29 @@ bool LLGLSLShader::createVariant(EVariant axis)
 
 void LLGLSLShader::unload()
 {
+    freeOwnedVariants();
+
+    unloadInternal();
+
+    // After unloadInternal: unbinding a bound program ends its profile queries by the
+    // features they were begun with.
     mShaderFiles.clear();
     mDefines.clear();
     mPermutationsAdded = false;
     mFeatures = LLShaderFeatures();
-
-    freeOwnedVariants();
-
-    unloadInternal();
 }
 
 void LLGLSLShader::unloadInternal()
 {
     sInstances.erase(this);
 
+    // A bound program is unbound before it is deleted, so the bound-program statics never
+    // name a dead program, or an owned variant freed right after this.
+    if (sCurBoundShaderPtr == this || (mProgramObject != 0 && sCurBoundShader == mProgramObject))
+    {
+        unbind();
+    }
+
     stop_glerror();
     mAttribute.clear();
     mTexture.clear();
@@ -714,6 +731,12 @@ void LLGLSLShader::unloadInternal()
         mSamplesQuery = 0;
     }
 
+    if (mPrimitivesQuery)
+    {
+        glDeleteQueries(1, &mPrimitivesQuery);
+        mPrimitivesQuery = 0;
+    }
+
     stop_glerror();
 }
 

indra/llrender/llglstates.h

@@ -40,8 +40,17 @@ class LLGLDepthTest
 
     ~LLGLDepthTest();
 
+    // A copy would restore the depth state a second time.
+    LLGLDepthTest(const LLGLDepthTest&) = delete;
+    LLGLDepthTest& operator=(const LLGLDepthTest&) = delete;
+
     void checkState();
 
+    // Take the tracked state back to a fresh context's: test off, writes on, GL_LESS, with the
+    // func issued through remap so GL agrees under whichever convention is latched. Run per
+    // context by LLGLState::initClass.
+    static void initClass();
+
     // Reverse-Z depth-func translation. The tracked state (sDepthFunc) and every call
     // site stay in the forward/semantic convention; the physical glDepthFunc is fed the
     // translated func so a call site asking for "nearer wins" keeps that meaning under

indra/llrender/llimagegl.cpp

@@ -42,6 +42,7 @@
 #include "llgl.h"
 #include "llglslshader.h"
 #include "llrender.h"
+#include "llrendertarget.h"
 #include "llwindow.h"
 #include "llframetimer.h"
 #include <atomic>
@@ -3221,6 +3222,24 @@ bool LLImageGL::scaleDown(S32 desired_discard)
     S32 desired_width = getWidth(desired_discard);
     S32 desired_height = getHeight(desired_discard);
 
+    // A deprecated source format is held under a swizzle, and the FBO path cannot carry
+    // one: it samples the old texture THROUGH the swizzle and copies the result back by
+    // position, so luminance-alpha's alpha comes back as luminance and alpha-only loses
+    // its alpha entirely. The PBO path copies the stored channels as they are.
+    //
+    // The FBO path also draws into the target the caller bound -- the texture list's
+    // 1024x1024 scratch -- and copies back out of it, so a result larger than that target
+    // copies undefined pixels from outside the framebuffer. Such a texture takes the PBO
+    // path, which reads the level out of the texture's own mip chain; without one there
+    // is nothing it can read, however it came to that path, so the downscale is refused.
+    const bool fits_target = desired_width <= (S32)LLRenderTarget::sCurResX
+                          && desired_height <= (S32)LLRenderTarget::sCurResY;
+    const bool use_fbo = gGLManager.mDownScaleMethod == 0 && mDeprecatedSourceFormat == 0 && fits_target;
+    if (!use_fbo && !mHasMipMaps)
+    {
+        return false;
+    }
+
     // Downscale into a NEW texture object rather than reallocating this one in place.
     // glTexImage2D on the live name is illegal once the texture has immutable storage
     // (GL_INVALID_OPERATION, silently leaving it at the old size), and it has no D3D11
@@ -3235,12 +3254,6 @@ bool LLImageGL::scaleDown(S32 desired_discard)
         return false;
     }
 
-    // A deprecated source format is held under a swizzle, and the FBO path cannot carry
-    // one: it samples the old texture THROUGH the swizzle and copies the result back by
-    // position, so luminance-alpha's alpha comes back as luminance and alpha-only loses
-    // its alpha entirely. The PBO path copies the stored channels as they are.
-    const bool use_fbo = gGLManager.mDownScaleMethod == 0 && mDeprecatedSourceFormat == 0;
-
     if (use_fbo)
     { // use an FBO to downscale the texture
         glViewport(0, 0, desired_width, desired_height);
@@ -3283,7 +3296,10 @@ bool LLImageGL::scaleDown(S32 desired_discard)
     else
     { // use a PBO to downscale the texture
         U64 size = getBytes(desired_discard);
-        llassert(size <= 2048 * 2048 * 4); // we shouldn't be using this method to downscale huge textures, but it'll work
+        // At most a level below the largest texture the viewer keeps (a larger one is set
+        // missing on load), at four bytes a texel. The scratch buffer grows to whatever it is
+        // asked for; this bounds how far.
+        llassert(size <= (U64)(MAX_IMAGE_SIZE / 2) * (MAX_IMAGE_SIZE / 2) * 4);
         gGL.getTextureSlot(0)->bind(this, false, true);
 
         if (sScratchPBO == 0)