#6055 WIP Optimize LLImageGL::analyzeAlpha

[akleshchev]

Preliminary variant of executing analyzeAlpha in ImageRequest::finishRequest, should cover majority of cases.

Purpose: analyzeAlpha is expensive, when possible do it on an image worker's thread.

This is not ready yet. Some of it is copilot generated, cases are likely missing, some of analyze calls I added just for the tracking purposes, so I need to go over it with a toothcomb.

P.S. This ended up overcomplicated and not dev-friendly. developers will have to remember to clear/set this data and direct manipulation of raw data invaidates cached calue without a way to detect the change. I will have to think it over a bit more.

[Copilot]

Pull request overview

This PR introduces cached alpha-mask analysis on LLImageRaw and uses that cached result during GPU texture upload (LLImageGL) to avoid re-running expensive LLImageGL::analyzeAlpha() on the main thread. It also adds a first pass at performing alpha analysis in the image worker pipeline and annotates a number of call sites to either precompute or explicitly disable alpha analysis.

Changes:

• Add LLImageRaw alpha-analysis caching (AlphaAnalysis) plus helpers to compute/copy/reset it.
• Teach LLImageGL to consume precomputed LLImageRaw alpha analysis and to persist analysis state.
• Add/adjust call sites to precompute (analyzeAlpha()) or suppress (setAlphaAnalysis(false)) alpha analysis in various texture/image creation paths.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
indra/newview/llvosky.cpp	Marks sky textures as opaque masks via cached alpha analysis.
indra/newview/llviewertexture.cpp	Triggers alpha analysis on a local black image.
indra/newview/llviewerparceloverlay.cpp	Disables alpha analysis for parcel overlay raw image.
indra/newview/llviewermedia.cpp	Disables alpha analysis for media update raw frames.
indra/newview/lltexturecache.cpp	Runs alpha analysis when reading tiny fast-cache images.
indra/newview/lllocalbitmaps.cpp	Triggers alpha analysis after local bitmap decode.
indra/newview/lldynamictexture.cpp	Disables alpha analysis for dynamic texture backing raw image.
indra/newview/lldrawpoolbump.cpp	Attempts to propagate alpha analysis when generating normal maps.
indra/llrender/llimagegl.h	Adds alpha-analysis helper API and a cached-analysis flag to LLImageGL.
indra/llrender/llimagegl.cpp	Consumes cached LLImageRaw analysis and changes alpha analysis implementation/caching.
indra/llrender/llcubemap.cpp	Adds alpha analysis calls on cube map raw images.
indra/llimage/tests/llimageworker_test.cpp	Updates test stubs for the new LLImageRaw::analyzeAlpha().
indra/llimage/llimageworker.cpp	Runs LLImageRaw::analyzeAlpha() during request completion.
indra/llimage/llimage.h	Defines LLImageRaw::AlphaAnalysis and related APIs.
indra/llimage/llimage.cpp	Implements alpha analysis caching and propagates/reset logic through image operations.
indra/llappearance/lltexlayerparams.cpp	Triggers alpha analysis after decoding static alpha TGAs.
indra/llappearance/lltexlayer.cpp	Pre-analyzes mask-format images before GL texture creation.

Comments suppressed due to low confidence (2)

indra/llrender/llimagegl.cpp:1652

• Same cache-invalidation issue as in setImage(): createGLTexture(const LLImageRaw*) can be called multiple times on the same LLImageGL. Without resetting mAlphaAnalyzed/mIsMask before each upload, mask state can be carried over from a previous image when the new imageraw doesn't carry cached analysis.

    // Use pre-computed alpha analysis if available
    if (mNeedsAlphaAndPickMask && imageraw && imageraw->hasAlphaAnalysis())
    {
        const auto& analysis = imageraw->getAlphaAnalysis();
        mIsMask = analysis.is_mask;
        mAlphaAnalyzed = true;

        LL_DEBUGS("Texture") << "Using pre-analyzed alpha: is_mask="
            << mIsMask << LL_ENDL;
    }

indra/llrender/llimagegl.cpp:762

• The new setImage(imageraw, data_in, ...) overload also relies on mAlphaAnalyzed as a one-time cache but doesn't reset it for new uploads. This has the same stale-mask risk as the other upload paths if this overload is used for dynamic/replaced textures.

bool LLImageGL::setImage(const LLImageRaw* imageraw, const U8* data_in, bool data_hasmips, S32 usename)
{
    // Use cached alpha analysis if available
    if (imageraw && imageraw->hasAlphaAnalysis() && mNeedsAlphaAndPickMask && !mAlphaAnalyzed)
    {
        const auto& analysis = imageraw->getAlphaAnalysis();
        mIsMask = analysis.is_mask;
        mAlphaAnalyzed = true;
    }
    return setImage(data_in, data_hasmips, usename);

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[akleshchev]

This feels like a wrong approach. The data probably should be provided side-by side with the image (like we do with aux), not inside the raw image, regardles of circumstances.

Issues of the curent approach:

1. Storing that data in raw image is out of context for the class and will catch developers unaware.
2. This early in texture pipeline we don't know if we actually need that data, nor the format it is set to render as
3. Raw image's data can be manipulated externally, mAlphaAnalysis becomes obsolete and there is no way to detect external manipulations to reset cached value.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (4)

indra/llrender/llimagegl.cpp:2315

• data_size is computed with U32 arithmetic and multiplies by signed S8 mAlphaStride, which can overflow before widening to size_t and is vulnerable to signed/unsigned conversion surprises. Capture alpha_stride first and compute data_size using size_t casts (and assert stride > 0).

    // Copy data for worker thread
    size_t data_size = w * h * mAlphaStride;
    U8* data_copy = new U8[data_size];
    memcpy(data_copy, static_cast<const U8*>(data_in), data_size);

indra/llrender/llimagegl.cpp:2352

• WorkQueueBase::postTo() returns bool and can fail (e.g. target queue closed). The current code ignores the return value, which can leak data_copy and leave the extra ref() unreleased (unref() only runs in the follow-up callback). Handle the false return by deleting data_copy and unref() (and optionally doing a synchronous fallback). Also fix the profiler label typo while touching this block.

        mainq->postTo(
            workerq,
            // Worker thread: analyze alpha
            [data_copy, w, h, alpha_offset, alpha_stride]() -> bool
        {

indra/llrender/llimagegl.h:117

• queueAsyncAlphaAnalysis() is declared in the header but has no definition and is not used anywhere. This adds dead/unfinished API surface to LLImageGL and can confuse future callers; remove it until implemented (or add an implementation in this PR).

    static bool analyzeAlphaData(const void* data_in, U32 w, U32 h, S8 alpha_offset, S8 alpha_stride);
    void analyzeAlpha(const void* data_in, U32 w, U32 h);
    void queueAsyncAlphaAnalysis(const void* data_in, U32 w, U32 h);
    void calcAlphaChannelOffsetAndStride();

indra/llrender/llimagegl.cpp:2351

• The async path sets mIsMask = false until the worker finishes. That guarantees getIsAlphaMask() will report "not a mask" for some period even when the image is a mask, changing behavior compared to the prior synchronous analysis and potentially causing visible popping (e.g. LLFace::canAutoMaskAlpha queries this during rendering). Consider introducing a pending/unknown state (tri-state) or deferring mask decisions until the analysis result is ready, instead of forcing a false value.

        // Conservative default until analysis completes
        mIsMask = false;

[akleshchev]

Closing, will recreate in a new PR, to drop copilot's context and to make it cleaner since I'm dropping original solution entirely.