update skia skill improvements

.github/skills/update-skia/SKILL.md

@@ -26,7 +26,8 @@ Update Google Skia to a new Chrome milestone in SkiaSharp's mono/skia fork.
 ## Key References
 
 - **[references/breaking-changes-checklist.md](references/breaking-changes-checklist.md)** — How to analyze breaking changes between milestones
-- **[references/known-gotchas.md](references/known-gotchas.md)** — 10 hard-won lessons and troubleshooting table
+- **[references/upstream-merge-guide.md](references/upstream-merge-guide.md)** — Detailed merge strategy and conflict resolution for mono/skia
+- **[references/known-gotchas.md](references/known-gotchas.md)** — Hard-won lessons and troubleshooting table
 - **[references/typical-changes.md](references/typical-changes.md)** — Files typically changed during an update
 - **[documentation/dev/dependencies.md](../../../documentation/dev/dependencies.md)** — Dependency tracking and cgmanifest.json format
 - **[RELEASE_NOTES.md in upstream Skia](https://raw.githubusercontent.com/google/skia/main/RELEASE_NOTES.md)** — Official Skia release notes
@@ -175,16 +176,34 @@ milestone numbers and paste your breaking change analysis table. The default exp
    git checkout -b dev/update-skia-{TARGET}
    ```
 
-2. **Merge upstream**:
+2. **Merge upstream** — Use `--no-commit` for manual conflict resolution:
    ```bash
-   git merge upstream/chrome/m{TARGET}
+   git merge --no-commit upstream/chrome/m{TARGET}
    ```
 
-3. **Resolve conflicts** — Common conflicts:
-   - `DEPS` — Keep our dependency pins, accept upstream structure changes
-   - `RELEASE_NOTES.md` — Accept upstream
-   - `infra/bots/jobs.json` — Accept upstream
-   - Source files in `src/` — Carefully resolve (don't lose our C API)
+3. **⚠️ CRITICAL: Genuine conflict-resolved merge required**
+
+   Each conflict MUST be resolved individually. See [references/upstream-merge-guide.md](references/upstream-merge-guide.md) for the full guide.
+
+   **❌ FORBIDDEN: Tree-Override Merge** — Never use `git merge -s ours`, `git read-tree --reset`, or blanket resolution that takes one side for all files. This destroys `git blame` attribution for all C API files.
+
+   **Conflict resolution by file category:**
+
+   | File Category | Strategy | Details |
+   |--------------|----------|---------|
+   | `BUILD.gn` | **Combine both** | Keep new milestone's structure AND SkiaSharp's platform flags and `skiasharp_build` target. Usually the most complex file. |
+   | `DEPS` | **Combine both** | Keep our dependency pins, accept upstream structure changes |
+   | `RELEASE_NOTES.md` | **Take upstream** | Accept upstream |
+   | `infra/bots/jobs.json` | **Take upstream** | Accept upstream |
+   | C API headers (`include/c/`) | **Keep SkiaSharp** | These don't exist upstream, conflicts are rare |
+   | C API source (`src/c/`) | **Keep SkiaSharp + adapt** | Take implementations, update includes and API calls in post-merge commits |
+   | GL/Vulkan interfaces | **Combine** | Adapt SkiaSharp's dynamic GL loading to new API structure |
+   | Infrastructure (`.gitignore`, CI) | **Take upstream** for CI/jobs, **keep SkiaSharp** for `.disabled.*` configs |
+
+4. **Commit the merge**:
+   ```bash
+   git commit  # Creates proper two-parent merge
+   ```
 
 4. **Verify our C API files survived the merge**:
    ```bash
@@ -205,8 +224,9 @@ milestone numbers and paste your breaking change analysis table. The default exp
 > git diff upstream/chrome/m{CURRENT}..upstream/chrome/m{TARGET} --diff-filter=AD --name-only -- src/ include/
 >                                                   # Review added/deleted files
 > git diff --check                                  # Zero conflict markers remain
+> git blame src/c/sk_canvas.cpp | head -20         # Attribution shows original commits, not just merge
 > ```
-> All three must produce expected output before proceeding.
+> All four must produce expected output before proceeding.
 
 ### Phase 5: Fix C API Shim Layer
 
@@ -317,6 +337,12 @@ New functions from upstream changes are usually additive and can be deferred.
 - Use `[Obsolete]` for deprecated APIs with migration guidance
 - Return `null` from factory methods on failure (don't throw)
 
+**API evolution policy** — when deciding whether to break SkiaSharp's managed API:
+1. Prefer minimal changes — absorb internally when existing API is still sensible
+2. Allow logical changes — when keeping old API would be misleading or semantically wrong
+3. Follow Skia's direction — if Skia renamed/restructured for good reason, follow suit
+4. Document all breaking changes — every managed API change needs upstream Skia motivation. Do NOT add backward-compatibility shims for APIs that Skia intentionally removed.
+
 ### Phase 9: Build & Test
 
 ```bash
@@ -387,9 +413,12 @@ Before proceeding to merge, verify ALL of these:
 - [ ] mono/skia PR targets `skiasharp` branch
 - [ ] mono/SkiaSharp PR targets `main` branch
 - [ ] **SkiaSharp's `externals/skia` submodule points to the mono/skia PR branch** (`git submodule status`)
+- [ ] `.gitmodules` branch updated to match new mono/skia branch name
 - [ ] `cgmanifest.json` updated with new commit hash, version, and chrome_milestone
 - [ ] `scripts/VERSIONS.txt` updated (ALL version lines, not just milestone)
+- [ ] `scripts/azure-pipelines-variables.yml` updated
 - [ ] `SkiaApi.generated.cs` regenerated and committed
+- [ ] `changelogs/SkiaSharp/3.{TARGET}.0/SkiaSharp.md` created with breaking changes documented
 - [ ] Both PRs cross-reference each other
 - [ ] Native build passes on at least one platform
 - [ ] C# build passes with 0 errors
@@ -424,6 +453,15 @@ Before proceeding past each step, verify:
 These files contain lookup information — consult them when you hit a problem or need context,
 not necessarily upfront:
 
-- **[references/known-gotchas.md](references/known-gotchas.md)** — 10 hard-won lessons from past updates (DEF_STRUCT_MAP, emsdk, BUILD.gn flags, HarfBuzz, DEPS forks, etc.) and a troubleshooting table
+- **[references/known-gotchas.md](references/known-gotchas.md)** — Hard-won lessons from past updates and troubleshooting table
+- **[references/upstream-merge-guide.md](references/upstream-merge-guide.md)** — Detailed merge strategy, conflict resolution by file category, and verification steps
 - **[references/typical-changes.md](references/typical-changes.md)** — Files typically changed in each repository during an update
 - **[references/breaking-changes-checklist.md](references/breaking-changes-checklist.md)** — How to analyze breaking changes between milestones
+
+## PR Review Strategy for Large Diffs
+
+mono/skia PRs can have thousands of upstream commits. Guide reviewers with:
+
+1. **Compare PR branch against `chrome/m{TARGET}`** for non-C-API content — changes should be minimal
+2. **Compare PR branch against `skiasharp`** for C API modifications (`include/c/`, `src/c/`) — this is where the real review happens
+3. **Highlight the focused post-merge commits** — these contain all the actual SkiaSharp-specific adaptation work

.github/skills/update-skia/references/breaking-changes-checklist.md

@@ -105,6 +105,48 @@ After applying fixes:
 3. Build C#: `dotnet build binding/SkiaSharp/SkiaSharp.csproj`
 4. Test: `dotnet test tests/SkiaSharp.Tests.Console/SkiaSharp.Tests.Console.csproj`
 
+## Recurring Breaking Change Categories
+
+These categories recur across milestone bumps. Use them as a checklist when analyzing a new bump:
+
+### Category: Removed Static Factory Methods (HIGH Risk)
+
+Skia periodically removes static factory methods from classes like `SkTypeface`, `SkFontMgr`, etc. and replaces them with instance methods or free functions that require an explicit context object (e.g., a font manager).
+
+**C API fix pattern:** Add optional context parameter to C API wrapper (NULL = use platform default internally).  
+**C# fix pattern:** Add new overloads with context parameter. Follow Skia's explicit-context direction rather than hiding the change.
+
+### Category: Header Path Reorganization (MEDIUM Risk)
+
+Skia reorganizes headers into subdirectories (e.g., GPU headers into backend-specific paths under `ganesh/`, `graphite/`, etc.).
+
+**C API fix pattern:** Update `#include` paths in C API source files. No API surface change.
+
+### Category: Factory Methods Moved to Namespace Functions (HIGH Risk)
+
+Static methods on classes get moved to free functions in a namespace (e.g., `ClassName::Make*()` → `ClassNames::Make*()`, with new dedicated headers).
+
+**C API fix pattern:** Update function calls and add new `#include` directives.
+
+### Category: Type Renames + Struct Changes (HIGH Risk)
+
+Types get renamed (often into namespaces like `skgpu::`) and structs gain/lose fields.
+
+**C API fix pattern:** Update type references, update struct definitions (remove dead fields, add new ones).  
+**C# fix pattern:** Update managed struct definitions to match. Breaking but necessary — dead fields are misleading.
+
+### Category: Removed Files / APIs (LOW-MEDIUM Risk)
+
+Entire headers or individual APIs removed from upstream.
+
+**C API fix pattern:** Remove includes and any wrappers for removed APIs.
+
+### Category: New Types / Enum Values (LOW Risk, Additive)
+
+New color types, enum values, struct fields added.
+
+**C API fix pattern:** Add to enum mappings. Note: mid-enum insertions renumber all subsequent values — always regenerate bindings, never hand-edit.
+
 ## Historical Examples
 
 ### m118 → m119 Changes Required

.github/skills/update-skia/references/known-gotchas.md

@@ -65,6 +65,55 @@ HarfBuzz updates are **always separate** from Skia milestone updates. When the h
 
 HarfBuzz version bumps should be done separately via the `native-dependency-update` skill, which includes writing the required delegate proxies.
 
+## 11. Git History: Genuine Merge vs Tree-Override (CRITICAL)
+
+**Never** use a tree-override merge (e.g., `git merge -s ours`, or `git read-tree --reset` after `--no-commit`) when merging upstream into the mono/skia fork. This creates a merge commit that records parentage without actually combining content, which:
+- Destroys `git blame` attribution — all lines attributed to the single merge commit
+- Makes `git log --follow` useless for C API files
+- Forces subsequent cherry-picks that further obscure history
+
+**Always** use a genuine conflict-resolved merge where each conflicting file is resolved individually. See [upstream-merge-guide.md](upstream-merge-guide.md).
+
+## 12. Prefer Native Memory Patterns
+
+When bridging managed and native code (e.g., stream duplication, data snapshots), prefer handing data to native Skia types (like `SKData`, `SkMemoryStream`) over managed workarounds with weak references or shared state. Native ref-counted types integrate with Skia's internal ownership model and expose fast paths (e.g., `getMemoryBase()` zero-copy) that managed wrappers cannot.
+
+## 13. Isolate Behavioral Changes Into Separate PRs
+
+If a version bump requires a significant behavioral refactor (e.g., changing stream duplication semantics), extract it into its own PR. This keeps the main bump PR focused on the mechanical version update and lets the behavioral change be reviewed, tested, and validated in isolation. It also makes bisection easier if issues arise later.
+
+## 14. GPU Test Coverage Is Incomplete
+
+GPU tests may not run in CI for all backends and platforms. WebAssembly in particular has no runtime tests beyond crash detection. When making changes that affect GPU paths, call out which platforms were actually tested and provide evidence. Don't assume CI covers it.
+
+## 15. GN Build Flag Churn Between Milestones
+
+Build flags get renamed, removed, and added frequently between milestones. Obsolete flags cause build errors. Always diff the target milestone's `BUILD.gn` against the current one to identify flag changes before building. Pay special attention to `skia_enable_fontmgr_*` flags which are particularly volatile.
+
+## 16. `.gitmodules` Branch Name Must Be Updated
+
+When the mono/skia target branch name changes for a new milestone, the `.gitmodules` file in SkiaSharp must be updated to track the new branch. This is easy to forget and causes submodule operations to fail silently or track the wrong branch.
+
+## 17. Fontconfig `#ifdef` Guards
+
+Platform font manager calls (e.g., `SkFontMgr_New_FontConfig`) must be guarded by feature-availability macros (e.g., `SK_FONTMGR_FONTCONFIG_AVAILABLE`), not platform macros (e.g., `SK_BUILD_FOR_UNIX`). When `skia_use_fontconfig=false`, platform macros leave the symbol unresolved. The `-Wl,--no-undefined` linker flag catches this at link time.
+
+## 18. Enum Value Renumbering
+
+When upstream inserts new enum values mid-sequence (e.g., new `SKColorType` entries), ALL subsequent values shift. This affects `sk_enums.cpp`, `Definitions.cs`, `EnumMappings.cs`, utility switch tables, and any test that hardcodes enum integer values. Always regenerate bindings — don't hand-edit enum values.
+
+## 19. Changelog Convention
+
+SkiaSharp uses structured changelogs at `changelogs/SkiaSharp/{version}/SkiaSharp.md`. Each version bump must add a changelog file documenting: assembly version change, new types/enums, behavioral changes, breaking changes, and known quirks.
+
+## 20. Reference Count Assertions May Need Relaxing
+
+Skia may change internal reference counting behavior between milestones. When tests fail on refcount assertions, consider using range checks (`Assert.InRange(refcount, expected, expected + 1)`) instead of exact equality. A refcount change is more likely a deliberate upstream shift than a bug in your port.
+
+## 21. Pixel Value Precision Changes
+
+Upstream Skia periodically improves color conversion precision (e.g., switching from integer to floating-point luma). This can shift expected pixel values by ±1. When pixel-exact test assertions break, check whether upstream changed the conversion — update expected values rather than treating it as a regression.
+
 ---
 
 ## Troubleshooting
@@ -74,5 +123,13 @@ HarfBuzz version bumps should be done separately via the `native-dependency-upda
 | `EntryPointNotFoundException` | Native lib not rebuilt after C API change | `dotnet cake --target=externals-{platform}` |
 | `error CS0246` missing type | Binding not regenerated | `pwsh ./utils/generate.ps1` |
 | Merge conflict in DEPS | Both forks updated deps independently | Keep our DEPS pins, accept upstream structure |
-| `LNK2001 unresolved external` | C function name mismatch | Verify C API function names match exactly |
-| Build fails after merge | Missing `#include` for moved headers | Check upstream header relocation notes |
+| `LNK2001 unresolved external` | C function name mismatch or missing system lib | Verify C API function names match; check system library linkage (fontconfig, etc.) |
+| Build fails after merge | Missing `#include` for moved headers | Check upstream header relocation patterns |
+| `git blame` shows all lines from merge commit | Tree-override merge was used | Redo merge as genuine conflict-resolved merge (see #11) |
+| `Unknown GN flag` error | Obsolete flag in build config | Remove flag; diff target BUILD.gn for current flags (see #15) |
+| Disk space CI failure | Skia DEPS pulls too many dependencies | Remove unused DEPS entries |
+| Submodule fetch fails / tracks wrong branch | `.gitmodules` still references old branch name | Update `.gitmodules` branch field (see #16) |
+| Undefined symbol `SkFontMgr_New_FontConfig` | Wrong `#ifdef` guard | Fix guard to check `SK_FONTMGR_FONTCONFIG_AVAILABLE` (see #17) |
+| Color type enum values don't match | New values inserted mid-enum | Regenerate bindings; never hand-edit (see #18) |
+| Refcount test assertions fail | Skia changed internal ownership | Use `Assert.InRange` instead of `Assert.Equal` (see #20) |
+| Pixel value mismatch by ±1 | Upstream precision improvement | Update expected test values (see #21) |

.github/skills/update-skia/references/typical-changes.md

@@ -3,16 +3,23 @@
 | Repository | File | Change |
 |-----------|------|--------|
 | mono/skia | `DEPS` | Merge conflict resolution |
+| mono/skia | `BUILD.gn` | Merge conflict resolution (most complex) |
 | mono/skia | `include/core/SkMilestone.h` | New milestone number (from upstream) |
 | mono/skia | `include/c/sk_types.h` | Enum/type updates |
 | mono/skia | `src/c/*.cpp` | C API fixes for new C++ APIs |
 | mono/skia | `src/c/sk_enums.cpp` | Enum mapping updates |
+| mono/skia | `src/c/sk_types_priv.h` | Include path + type conversion updates |
+| mono/SkiaSharp | `.gitmodules` | Submodule branch name |
 | mono/SkiaSharp | `externals/skia` | Submodule pointer |
 | mono/SkiaSharp | `scripts/VERSIONS.txt` | All version numbers |
 | mono/SkiaSharp | `cgmanifest.json` | Security tracking |
 | mono/SkiaSharp | `scripts/azure-pipelines-variables.yml` | CI config |
+| mono/SkiaSharp | `scripts/cake/native-shared.cake` | Build script adjustments (e.g., emsdk skip) |
+| mono/SkiaSharp | `native/*/build.cake` | Per-platform GN flag updates |
 | mono/SkiaSharp | `binding/SkiaSharp/SkiaApi.generated.cs` | Regenerated |
-| mono/SkiaSharp | `binding/SkiaSharp/Definitions.cs` | Type definitions |
+| mono/SkiaSharp | `binding/SkiaSharp/Definitions.cs` | Type definitions, new enums |
 | mono/SkiaSharp | `binding/SkiaSharp/EnumMappings.cs` | Enum mappings |
+| mono/SkiaSharp | `binding/SkiaSharp/GRDefinitions.cs` | GPU type changes |
 | mono/SkiaSharp | `binding/libSkiaSharp.json` | Type config |
+| mono/SkiaSharp | `changelogs/SkiaSharp/3.{TARGET}.0/SkiaSharp.md` | Changelog (new file) |
 | mono/SkiaSharp | `tests/Tests/SkiaSharp/*.cs` | Test updates |