|
| 1 | +{ |
| 2 | + "meta": { |
| 3 | + "schemaVersion": "1.0", |
| 4 | + "number": 3490, |
| 5 | + "repo": "mono/SkiaSharp", |
| 6 | + "analyzedAt": "2026-02-13T00:14:31Z", |
| 7 | + "currentLabels": ["copilot"] |
| 8 | + }, |
| 9 | + "summary": "Draft PR by maintainer (mattleibow) that adds new HarfBuzzSharp C# binding APIs for OpenType font features, variable font manipulation, language matching, and buffer cloning. Updates the binding generator (CppAst 0.24.0 upgrade with compatibility fixes), regenerates HarfBuzzApi.generated.cs (+89 new extern functions, -28 removed), adds 586 lines of tests across 4 test files, and includes 5 new documentation files covering migration planning and binding generation. The CI build is currently failing.", |
| 10 | + "classification": { |
| 11 | + "type": { "value": "type/enhancement", "confidence": 0.95 }, |
| 12 | + "area": { "value": "area/HarfBuzzSharp", "confidence": 0.98 } |
| 13 | + }, |
| 14 | + "evidence": { |
| 15 | + "versionAnalysis": { |
| 16 | + "mentionedVersions": ["3.119.2-pr.3490.1"], |
| 17 | + "currentRelevance": "likely", |
| 18 | + "relevanceReason": "Active draft PR targeting main. Adds new API surface not yet available." |
| 19 | + } |
| 20 | + }, |
| 21 | + "analysis": { |
| 22 | + "summary": "This is a large enhancement PR that modernizes the HarfBuzzSharp binding. It progresses through incremental HarfBuzz header versions (2.9.1 → 3.4.0 → 4.4.1 → 5.3.1 → 7.3.0 → 8.3.0) in separate commits, adds manual C# wrappers for Face/Font/Buffer/Language, upgrades the CppAst parser to 0.24.0 with compatibility fixes (space-around-pointers normalization, suffix-const removal, signed/unsigned long long mappings, cross-platform SDK include paths), and excludes new hb_paint_/hb_color_line_ namespaces from generation. The CI build (#3.119.2-pr.3490.1) is failing. The nuget.config change adding nuget.org as an explicit source may be unintentional or could conflict with the existing dotnet-public feed.", |
| 23 | + "rationale": "Classified as type/enhancement because it adds substantial new API surface (OpenType layout queries, variable font controls, language matching) to an existing component. Not a feature-request (it's a PR with code). Area is area/HarfBuzzSharp since all new C# wrappers are in binding/HarfBuzzSharp/ and all new C API functions are HarfBuzz functions. No platform specificity — these are cross-platform APIs.", |
| 24 | + "keySignals": [ |
| 25 | + { "text": "19 commits progressing through HarfBuzz header versions 2.9.1 → 8.3.0", "source": "git log", "interpretation": "Methodical incremental binding update, not a one-shot change. Each commit regenerates bindings for a specific HarfBuzz version." }, |
| 26 | + { "text": "Draft PR with failing CI build (#3.119.2-pr.3490.1)", "source": "GitHub PR status", "interpretation": "Work in progress. The build failure needs investigation before this can be reviewed for merge." }, |
| 27 | + { "text": "+89 new extern functions, -28 removed in HarfBuzzApi.generated.cs", "source": "diff analysis", "interpretation": "Significant API surface change. Removed functions may indicate renamed/refactored HarfBuzz C API entries across versions." }, |
| 28 | + { "text": "CppAst upgraded from older version to 0.24.0 with space-around-pointer normalization", "source": "utils/SkiaSharpGenerator/BaseTool.cs diff", "interpretation": "Generator compatibility fix — CppAst 0.24.0 changes type display names, requiring normalization of pointer syntax." }, |
| 29 | + { "text": "hb_paint_ and hb_color_line_ namespaces excluded from generation", "source": "binding/libHarfBuzzSharp.json diff", "interpretation": "Paint/color-line APIs intentionally deferred — complex callback-heavy APIs not yet ready for binding." }, |
| 30 | + { "text": "586 lines of new tests across 4 files", "source": "diff stat", "interpretation": "Good test coverage for new APIs. Tests cover Buffer.CreateSimilar, Language.Matches, Face OpenType queries, Font variable/synthetic properties." } |
| 31 | + ], |
| 32 | + "codeInvestigation": [ |
| 33 | + { "file": "binding/HarfBuzzSharp/Face.cs", "finding": "New methods: GetName, TryGetName (with language overloads), GetAllNameEntries, GetOpenTypeLayoutScriptTags, GetOpenTypeLayoutFeatureTags, TryGetOpenTypeLayoutFeatureNameIds. All follow correct patterns: ArgumentNullException for null params, stackalloc for buffer allocation, two-pass pattern (get length then get data) for string retrieval.", "relevance": "direct" }, |
| 34 | + { "file": "binding/HarfBuzzSharp/Font.cs", "finding": "New APIs: GetPpem/SetPpem, Ptem property, SyntheticSlant property, SetSyntheticBold/GetSyntheticBold, SetVariation (uint and string tag overloads), SetVariations (array and ReadOnlySpan overloads), NamedInstance property. String tag overload validates length == 4. All use Handle correctly via P/Invoke.", "relevance": "direct" }, |
| 35 | + { "file": "binding/HarfBuzzSharp/Buffer.cs", "finding": "New static method CreateSimilar(Buffer source) — validates null, delegates to hb_buffer_create_similar. Returns new Buffer wrapping the native handle. Follows factory method pattern.", "relevance": "direct" }, |
| 36 | + { "file": "binding/HarfBuzzSharp/Language.cs", "finding": "New Matches(Language specific) method — validates null, delegates to hb_language_matches. Returns bool. Correct pattern.", "relevance": "direct" }, |
| 37 | + { "file": "binding/HarfBuzzSharp/Definitions.cs", "finding": "New OpenTypeLayoutTableTag enum (Gsub, Gpos) with correct HarfBuzz tag encoding using bit shifts. New OpenTypeFeatureNameIds struct with 5 properties matching hb_ot_layout_feature_get_name_ids output parameters.", "relevance": "direct" }, |
| 38 | + { "file": "utils/SkiaSharpGenerator/BaseTool.cs", "finding": "Three categories of changes: (1) Cross-platform SDK include path resolution for CppAst parsing (macOS, Linux, Windows). (2) Type mapping fixes: unsigned char→Byte, signed char→SByte, added signed/unsigned long long variants. (3) Type name normalization: remove suffix 'const', normalize 'type *' to 'type*' for CppAst 0.24.0 compatibility.", "relevance": "related" }, |
| 39 | + { "file": "binding/libHarfBuzzSharp.json", "finding": "Excludes hb_paint_ and hb_color_line_ namespaces. Adds hb-paint.h to excluded headers. Adds 20+ paint/color_line types to excluded types. Adds hb_style_tag_t enum config. Adds generateProxy:false for 15+ callback function types (draw, paint).", "relevance": "related" }, |
| 40 | + { "file": "nuget.config", "finding": "Adds explicit nuget.org source before the existing dotnet-public Azure DevOps feed. This may have been needed for the CppAst 0.24.0 NuGet package during development.", "relevance": "context" } |
| 41 | + ], |
| 42 | + "nextQuestions": [ |
| 43 | + "What is the CI build failure? Need to check Azure DevOps build logs for #3.119.2-pr.3490.1.", |
| 44 | + "Are the 28 removed extern functions expected removals (API renames across HarfBuzz versions) or accidental losses?", |
| 45 | + "Is the nuget.config change intentional for the final PR or a development artifact?", |
| 46 | + "The SkiaApi.generated.cs shows 990 lines changed — are those just comment/formatting changes from the CppAst upgrade or actual signature changes?", |
| 47 | + "Should the hb_paint_/hb_color_line_ APIs be tracked as a follow-up issue?" |
| 48 | + ], |
| 49 | + "resolution": { |
| 50 | + "hypothesis": "This is a well-structured enhancement PR that methodically updates HarfBuzzSharp bindings. The CI failure is the primary blocker. The generator changes (CppAst 0.24.0) affect both HarfBuzzSharp and SkiaSharp generated files.", |
| 51 | + "proposals": [ |
| 52 | + { "title": "Fix CI build failure", "description": "Investigate the Azure DevOps build failure for #3.119.2-pr.3490.1. This is the primary blocker for the PR.", "confidence": 0.90, "effort": "small" }, |
| 53 | + { "title": "Verify removed extern functions", "description": "Audit the 28 removed extern functions in HarfBuzzApi.generated.cs to confirm they are expected API renames/removals across HarfBuzz versions and not accidental losses.", "confidence": 0.70, "effort": "small" }, |
| 54 | + { "title": "Review nuget.config change", "description": "Determine if the nuget.org source addition is needed for the final PR or should be reverted as a development artifact.", "confidence": 0.60, "effort": "trivial" }, |
| 55 | + { "title": "Track deferred paint APIs", "description": "Create a follow-up issue to track the intentionally excluded hb_paint_ and hb_color_line_ APIs for future binding work.", "confidence": 0.50, "effort": "trivial" } |
| 56 | + ], |
| 57 | + "recommendedProposal": "Fix CI build failure", |
| 58 | + "recommendedReason": "The build failure is the only hard blocker. All other items are review observations that can be addressed during code review." |
| 59 | + } |
| 60 | + }, |
| 61 | + "output": { |
| 62 | + "actionability": { |
| 63 | + "suggestedAction": "needs-investigation", |
| 64 | + "confidence": 0.90, |
| 65 | + "reason": "Draft PR with failing CI. Needs build failure diagnosis before it can proceed to review." |
| 66 | + }, |
| 67 | + "actions": [ |
| 68 | + { |
| 69 | + "type": "update-labels", |
| 70 | + "description": "Apply enhancement and HarfBuzzSharp labels", |
| 71 | + "risk": "low", |
| 72 | + "confidence": 0.95, |
| 73 | + "labels": ["type/enhancement", "area/HarfBuzzSharp"] |
| 74 | + } |
| 75 | + ] |
| 76 | + } |
| 77 | +} |
0 commit comments