`CPU raster performance regression: commit 356c7267fa removed default SK_ENABLE_LEGACY_SHADERCONTEXT, forcing all software draws through SkRasterPipelineBlitter`

[Janne Gustafsson]

Hi all,

There were some visibility issues with the filing of this reasonably critical bug report to issues.skia.org (see  https://issues.skia.org/issues/534377142  ; seems to require permissions to view), so I thought that I will file it here as well, as this is a pretty critical bug especially for CPU rendering on Windows. The report in markdown format is below:

## Description

Commit `356c7267fa` ("Remove SK_DISABLE_LEGACY_SHADERCONTEXT", [skia-review/1150076](https://skia-review.googlesource.com/c/skia/+/1150076)) removed the following default-enable block from `include/core/SkTypes.h`:

```cpp
#if !defined(SK_DISABLE_LEGACY_SHADERCONTEXT) && !defined(SK_ENABLE_LEGACY_SHADERCONTEXT)
#   define SK_ENABLE_LEGACY_SHADERCONTEXT
#endif
```

The commit message states "This macro no longer guards any code", referring to `SK_DISABLE_LEGACY_SHADERCONTEXT`. That is correct — `SK_DISABLE` was unused. However, the removed block also served as the **default definition** for `SK_ENABLE_LEGACY_SHADERCONTEXT`, which still guards substantial code throughout `src/shaders/` and `src/core/`.

Without this default, any downstream consumer that does not explicitly pass `-DSK_ENABLE_LEGACY_SHADERCONTEXT` loses all legacy shader contexts and falls back to the raster pipeline for every software bitmap draw.

### What breaks

In `SkShaderBase::makeContext()` (`src/shaders/SkShaderBase.cpp:97`):

```cpp
SkShaderBase::Context* SkShaderBase::makeContext(const ContextRec& rec, SkArenaAlloc* alloc) const {
#ifdef SK_ENABLE_LEGACY_SHADERCONTEXT
    auto totalMatrix = rec.fMatrixRec.totalMatrix();
    if (totalMatrix.hasPerspective() || !totalMatrix.invert()) {
        return nullptr;
    }
    return this->onMakeContext(rec, alloc);
#else
    return nullptr;   // <-- always taken without the macro
#endif
}
```

When `makeContext()` returns `nullptr`, `SkBlitter::Choose()` (`src/core/SkBlitter.cpp:744`) falls through to `CreateSkRPBlitter()`, bypassing the fast legacy blitters entirely:

```cpp
if (!shaderContext) {
    return CreateSkRPBlitter();   // raster pipeline fallback
}
```

This means the hand-optimized paths — `SkARGB32_Shader_Blitter`, `Repeat_S32_D32_nofilter_trans_shaderproc` (which is a tight `memcpy` loop), and the SSSE3/NEON-accelerated `S32_alpha_D32_filter_DX` — are all dead code without the macro.

### Performance impact

The legacy blitter for a repeating, unfiltered, translate-only BGRA_8888 draw reduces to:

```cpp
// src/core/SkBitmapProcState.cpp:383
for (;;) {
    int n = std::min(stopX - ix, count);
    memcpy(colors, row + ix, n * sizeof(SkPMColor));
    count -= n;
    if (0 == count) return;
    colors += n;
    ix = 0;
}
```

The raster pipeline replacement executes 5–8+ stage function calls per pixel group (`seed_shader` → matrix → `gather_8888` → color-space steps → `srcover_rgba_8888` → store), operating in float4 vectors with integer↔float conversion overhead for `kN32` surfaces.

In a real-world .NET MAUI application using SkiaSharp (via `SKCanvasView` software-rendered UI controls drawing tiled backgrounds and custom buttons), this regression caused **severe frame drops and visible flickering** on all platforms. The effect was most pronounced on Windows but measurable everywhere. SkiaSharp has shipped a workaround by explicitly passing `-DSK_ENABLE_LEGACY_SHADERCONTEXT` in their build flags for all platforms (see [SkiaSharp PR #4421](https://github.com/mono/SkiaSharp/pull/4421)).

Note: even with Clang and `[[clang::musttail]]` enabled (`SK_HAS_MUSTTAIL=1`), the pipeline's multi-stage dispatch, float-domain operation, and per-pixel gather pattern remain fundamentally slower than the legacy path's monolithic `memcpy` loops for common 2D bitmap operations.

---

## Steps to reproduce

Draw a tiled BGRA_8888 bitmap on a CPU-backed (`SkSurface::MakeRaster`) canvas without `-DSK_ENABLE_LEGACY_SHADERCONTEXT`:

```cpp
#include "include/core/SkCanvas.h"
#include "include/core/SkSurface.h"
#include "include/core/SkBitmap.h"

void benchmark() {
    // Create a small tile bitmap
    SkBitmap tile;
    tile.allocN32Pixels(64, 64);
    tile.eraseColor(SK_ColorRED);
    auto tileImage = tile.asImage();

    // Create a CPU-backed destination surface
    auto surface = SkSurfaces::Raster(SkImageInfo::MakeN32Premul(1920, 1080));
    auto canvas = surface->getCanvas();

    SkPaint paint;
    // Draw tiled background — each drawImage call hits the blitter
    for (int frame = 0; frame < 1000; ++frame) {
        for (int y = 0; y < 1080; y += 64) {
            for (int x = 0; x < 1920; x += 64) {
                canvas->drawImage(tileImage, x, y, SkSamplingOptions(), &paint);
            }
        }
    }
}
```

Profile with and without `-DSK_ENABLE_LEGACY_SHADERCONTEXT`:
- **Without**: CPU hotspot is `gather_8888` and raster pipeline stage dispatch
- **With**: CPU hotspot is `memcpy` inside `Repeat_S32_D32_nofilter_trans_shaderproc` (bandwidth-limited, optimal)

---

## Affected milestones

- **First bad**: m146 (commit `356c7267fa`, 2026-01-26)
- **Last good**: m145 (the default-enable block was still present)
- **Preparatory commit**: `1a15ed6b17` ("Allow users to set SK_ENABLE_LEGACY_SHADERCONTEXT", [skia-review/1150216](https://skia-review.googlesource.com/c/skia/+/1150216)) — same day, same author — changed the guard to allow explicit opt-in, anticipating the removal

---

## Suggested fix

Restore the default-enable block in `include/core/SkTypes.h` so that the legacy shader contexts remain active unless explicitly disabled:

```cpp
#if !defined(SK_DISABLE_LEGACY_SHADERCONTEXT) && !defined(SK_ENABLE_LEGACY_SHADERCONTEXT)
#   define SK_ENABLE_LEGACY_SHADERCONTEXT
#endif
```

Alternatively, if the intent is to eventually remove the legacy code entirely, ensure the raster pipeline achieves performance parity for common cases (unfiltered N32 → N32 tile draws, SrcOver blend, same color space) before removing the fast path. The current pipeline is not a performance-equivalent replacement.

---

## Downstream impact

- **SkiaSharp** (https://github.com/mono/SkiaSharp): Shipped workaround in PR #4421, adding `-DSK_ENABLE_LEGACY_SHADERCONTEXT` to build flags on **all 8 platforms** (Windows, macOS, Linux, iOS, Android, tvOS, Tizen, WASM)
- Any other downstream consumer building Skia without this flag since m146 is silently affected