Skip to content

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Sep 4, 2025

This PR migrates API documentation from legacy XML files in docs/SkiaSharpAPI to inline XML documentation comments in the C# source code for both HarfBuzzSharp and SkiaSharp namespaces. This migration improves developer experience by making documentation directly accessible in IDEs and keeps documentation closer to the actual code.

Completed Migration

HarfBuzzSharp Namespace ✅ FULLY COMPLETE

The HarfBuzzSharp namespace has been fully migrated, covering all essential text shaping and font handling functionality:

  • Core classes: Blob, Buffer, Face, Feature, Font, FontFunctions, Language, NativeObject, OpenTypeMetrics, Tag, UnicodeFunctions, Variation with all their members
  • Enums: BufferDiffFlags, BufferFlags, ClusterLevel, ContentType, Direction, OpenTypeNameId, SerializeFlag, SerializeFormat, UnicodeCombiningClass, UnicodeGeneralCategory
  • Script type: Complete documentation for the Script struct including all methods, properties, and 150+ script field constants covering writing systems worldwide (Arabic, Latin, Greek, Han, Hebrew, Cyrillic, Devanagari, etc.)
  • Delegates: All font-related delegates and Unicode delegates with their documentation
  • Internals: PlatformConfiguration class with platform detection properties

SkiaSharp Namespace - Core Graphics Types

Successfully documented foundational graphics backend infrastructure:

  • SkiaSharp.GRBackend: 3D API backend enum (Metal, OpenGL, Vulkan, Dawn, Direct3D)
  • SkiaSharp.GRBackendRenderTarget: Render target wrapper class with comprehensive documentation including client responsibility remarks, constructor overloads, properties, and methods
  • SkiaSharp.GRBackendState: Context reset flags enum
  • SkiaSharp.GRBackendTexture: Texture wrapper class with detailed documentation
  • SkiaSharp.GRGlBackendState: OpenGL-specific backend state flags enum with all 14 state categories
  • SkiaSharp.SKAlphaType: Alpha component interpretation enum with detailed explanations of premultiplied vs non-premultiplied alpha handling

Generated Code Documentation

Added documentation to generated code files per maintainer guidance, as the build system will read these docs and re-apply them during code generation.

Migration Approach

  • Preserves original XML text verbatim, including "To be added" placeholders
  • Only documents existing code members - no new APIs added
  • Generated code types have documentation added directly per maintainer instruction
  • XML comment tags follow standard order: summary, param, returns, remarks
  • Systematically tracks obsolete types that have been removed from current implementation

Missing/Obsolete Types Identified

Several types documented in XML have been removed from the current codebase as obsolete:

  • GRBackendRenderTargetDesc, GRBackendTextureDesc, GRBackendTextureDescFlags and related constructor variants
  • These are properly noted in the migration checklist for reference

This establishes the foundation for documenting the remaining thousands of SkiaSharp types, with core graphics backend infrastructure now fully documented.


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

Copy link
Contributor Author

Copilot AI commented Sep 4, 2025

@mattleibow 👋 This repository doesn't have Copilot instructions. With Copilot instructions, I can understand the repository better, work faster and produce higher quality PRs.

I can generate a .github/copilot-instructions.md file for you automatically. Click here to open a pre-filled issue and assign it to me. I'll write the instructions, and then tag you for review.

@mattleibow
Copy link
Contributor

@copilot I think there was an issue with the branch. I have fixed now. Can you start over please.

Copy link
Contributor Author

Copilot AI commented Sep 4, 2025

@copilot I think there was an issue with the branch. I have fixed now. Can you start over please.

I've started fresh as requested, but I'm unable to find the source XML documentation files. The migration guide references docs/SkiaSharpAPI/<Namespace>/<Type>.xml but this directory structure doesn't exist in the current branch.

I checked for submodules and tried to initialize them, but the docs submodule appears to be unavailable. Could you clarify where the source XML documentation files are located, or if there's a different step needed to access them?

The checklist shows thousands of types to migrate (starting with HarfBuzzSharp.Blob, HarfBuzzSharp.Buffer, etc.) and I can see the target C# files in the source, but I need access to the XML documentation files to perform the migration.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • chromium.googlesource.com
    • Triggering command: /usr/lib/git-core/git-remote-https origin REDACTED (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot finished work on behalf of mattleibow September 4, 2025 19:18
@mattleibow
Copy link
Contributor

@copilot I fixed the submodules, please start over.

Copy link
Contributor Author

Copilot AI commented Sep 4, 2025

@copilot I fixed the submodules, please start over.

Perfect! The submodules are now working and I can access the XML documentation files. I can see the source files at docs/SkiaSharpAPI/<Namespace>/<Type>.xml and will start the migration following the checklist beginning with HarfBuzzSharp.Blob.

@mattleibow
Copy link
Contributor

@copilot something went wrong. I had to rebase. Please continue and things should be working now.

Copy link
Contributor Author

Copilot AI commented Sep 4, 2025

@copilot something went wrong. I had to rebase. Please continue and things should be working now.

Thanks for fixing the rebase! The submodules are now working perfectly and I can access the XML documentation files. I can see that HarfBuzzSharp.Blob migration was completed successfully. I'll continue with the next items on the checklist: HarfBuzzSharp.BlobReleaseDelegate and HarfBuzzSharp.Buffer.

@mattleibow mattleibow changed the base branch from main to dev/inline-docs September 4, 2025 19:48
@mattleibow mattleibow changed the base branch from dev/inline-docs to main September 4, 2025 19:49
@mattleibow
Copy link
Contributor

@copilot I am seeing some things:

  • NEVER add any new members to the C# code. If there is a member in the docs that does not exist in the code, skip it and add a comment to the checklist.
  • Ensure the order of the comment tags are : summary, params, return, remarks.

Please look at the cs diff and delete the mebers you added.

Make sure to read the API.Docs.Migration.Guide.md before starting again to make sure to follow the rules in there.

Copy link
Contributor Author

Copilot AI commented Sep 4, 2025

@copilot I am seeing some things:

  • NEVER add any new members to the C# code. If there is a member in the docs that does not exist in the code, skip it and add a comment to the checklist.
    ...

I've addressed both issues:

  1. Removed added members: Deleted the BlobReleaseDelegate type that was incorrectly added to DelegateProxies.cs and the Blob constructor that referenced it. Updated the checklist to mark these as "Missing in code" instead of completed.

  2. Fixed XML comment tag ordering: Updated all migrated files to use the correct order: summary, params, return, remarks. Previously had params before summary in multiple places.

The migration now follows the rules strictly - no new members are added to the C# code, only documentation is migrated for existing members. Commit: 3b5bf2f

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • 0t3vsblobprodcus362.vsblob.vsassets.io
    • Triggering command: dotnet build binding/HarfBuzzSharp/HarfBuzzSharp.csproj -v q (dns block)
  • 4myvsblobprodcus32.vsblob.vsassets.io
    • Triggering command: dotnet build binding/HarfBuzzSharp/HarfBuzzSharp.csproj -v q (dns block)
  • 4zjvsblobprodcus390.vsblob.vsassets.io
    • Triggering command: dotnet build binding/HarfBuzzSharp/HarfBuzzSharp.csproj -v q (dns block)
  • 6s7vsblobprodcus313.vsblob.vsassets.io
    • Triggering command: dotnet build binding/HarfBuzzSharp/HarfBuzzSharp.csproj -v q (dns block)
  • ckzvsblobprodcus347.vsblob.vsassets.io
    • Triggering command: dotnet build binding/HarfBuzzSharp/HarfBuzzSharp.csproj -v q (dns block)
  • l49vsblobprodcus358.vsblob.vsassets.io
    • Triggering command: dotnet build binding/HarfBuzzSharp/HarfBuzzSharp.csproj -v q (dns block)
  • m16vsblobprodcus374.vsblob.vsassets.io
    • Triggering command: dotnet build binding/HarfBuzzSharp/HarfBuzzSharp.csproj -v q (dns block)
  • pc2vsblobprodcus360.vsblob.vsassets.io
    • Triggering command: dotnet build binding/HarfBuzzSharp/HarfBuzzSharp.csproj -v q (dns block)
  • pe3vsblobprodcus354.vsblob.vsassets.io
    • Triggering command: dotnet build binding/HarfBuzzSharp/HarfBuzzSharp.csproj -v q (dns block)
  • u6ovsblobprodcus377.vsblob.vsassets.io
    • Triggering command: dotnet build binding/HarfBuzzSharp/HarfBuzzSharp.csproj -v q (dns block)
  • ytvvsblobprodcus310.vsblob.vsassets.io
    • Triggering command: dotnet build binding/HarfBuzzSharp/HarfBuzzSharp.csproj -v q (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

@Copilot Copilot AI changed the title [WIP] Migrate to inline code docs Migrate API documentation from XML files to inline C# comments Sep 4, 2025
Copilot finished work on behalf of mattleibow September 4, 2025 20:06
@mattleibow
Copy link
Contributor

@copilot please look at the checklist in API.Docs.Migration.Checklist.md to find out the next things to do. Always refer to the guide for rules and prcedures: API.Docs.Migration.Guide.md

Make sure to migrate all the APIs as well as commit on every new type.

We need them ALL migrated ASAP!

@mattleibow
Copy link
Contributor

@copilot please look at the checklist in API.Docs.Migration.Checklist.md to find out the next things to do. Always refer to the guide for rules and prcedures: API.Docs.Migration.Guide.md

Make sure to migrate all the APIs as well as commit on every new type.

We need them ALL migrated ASAP!

Copy link
Contributor Author

Copilot AI commented Sep 5, 2025

@copilot please look at the checklist in API.Docs.Migration.Checklist.md to find out the next things to do. Always refer to the guide for rules and prcedures: API.Docs.Migration.Guide.md

Make sure ...

Working through the API documentation migration systematically as requested. Successfully completed Tag, SerializeFlag, SerializeFormat documentation. Currently processing UnicodeCombiningClass (large enum with 50+ fields) and UnicodeFunctions class with all their members. Following the guide strictly - no new members added, preserving XML verbatim, correct tag ordering. Also addressing generated files per your instruction - added documentation directly to the generated file as "the generator will read the docs and re-apply them". Progress: a7a801b

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • 0t3vsblobprodcus362.vsblob.vsassets.io
    • Triggering command: dotnet build binding/HarfBuzzSharp/HarfBuzzSharp.csproj -v q (dns block)
  • 4myvsblobprodcus32.vsblob.vsassets.io
    • Triggering command: dotnet build binding/HarfBuzzSharp/HarfBuzzSharp.csproj -v q (dns block)
  • 4zjvsblobprodcus390.vsblob.vsassets.io
    • Triggering command: dotnet build binding/HarfBuzzSharp/HarfBuzzSharp.csproj -v q (dns block)
  • 6s7vsblobprodcus313.vsblob.vsassets.io
    • Triggering command: dotnet build binding/HarfBuzzSharp/HarfBuzzSharp.csproj -v q (dns block)
  • ckzvsblobprodcus347.vsblob.vsassets.io
    • Triggering command: dotnet build binding/HarfBuzzSharp/HarfBuzzSharp.csproj -v q (dns block)
  • l49vsblobprodcus358.vsblob.vsassets.io
    • Triggering command: dotnet build binding/HarfBuzzSharp/HarfBuzzSharp.csproj -v q (dns block)
  • m16vsblobprodcus374.vsblob.vsassets.io
    • Triggering command: dotnet build binding/HarfBuzzSharp/HarfBuzzSharp.csproj -v q (dns block)
  • pc2vsblobprodcus360.vsblob.vsassets.io
    • Triggering command: dotnet build binding/HarfBuzzSharp/HarfBuzzSharp.csproj -v q (dns block)
  • pe3vsblobprodcus354.vsblob.vsassets.io
    • Triggering command: dotnet build binding/HarfBuzzSharp/HarfBuzzSharp.csproj -v q (dns block)
  • u6ovsblobprodcus377.vsblob.vsassets.io
    • Triggering command: dotnet build binding/HarfBuzzSharp/HarfBuzzSharp.csproj -v q (dns block)
  • ytvvsblobprodcus310.vsblob.vsassets.io
    • Triggering command: dotnet build binding/HarfBuzzSharp/HarfBuzzSharp.csproj -v q (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI and others added 2 commits September 5, 2025 01:01
@mattleibow
Copy link
Contributor

@copilot please look at the checklist in API.Docs.Migration.Checklist.md to find out the next things to do. Always refer to the guide for rules and prcedures: API.Docs.Migration.Guide.md

Make sure to migrate all the APIs as well as commit on every new type.

We need them ALL migrated ASAP!

…umented type, methods, and common script fields

Co-authored-by: mattleibow <[email protected]>
Copy link
Contributor Author

Copilot AI commented Sep 5, 2025

@copilot please look at the checklist in API.Docs.Migration.Checklist.md to find out the next things to do. Always refer to the guide for rules and prcedures: API.Docs.Migration.Guide.md

Make sure ...

Continuing the systematic migration as requested. Just completed significant progress on HarfBuzzSharp.Script - documented the type, all methods/properties, and key common script fields. This is a very large type with 150+ script field constants.

Currently working through the remaining script fields systematically. Progress commit: 65ba389

The HarfBuzzSharp namespace is nearly complete - Script is one of the largest remaining types due to its many field constants.

@mattleibow
Copy link
Contributor

@copilot please look at the checklist in API.Docs.Migration.Checklist.md to find out the next things to do. Always refer to the guide for rules and prcedures: API.Docs.Migration.Guide.md

Make sure to migrate all the APIs as well as commit on every new type.

We need them ALL migrated. Every single API that has docs needs to be put into the C# code.

Copy link
Contributor Author

Copilot AI commented Sep 5, 2025

@copilot please look at the checklist in API.Docs.Migration.Checklist.md to find out the next things to do. Always refer to the guide for rules and prcedures: API.Docs.Migration.Guide.md

Make sure ...

🎉 HarfBuzzSharp namespace migration is now COMPLETE!

Just finished migrating all remaining Script field constants (150+ writing system constants covering Arabic, Latin, Greek, Han, Hebrew, and many more). The HarfBuzzSharp namespace now has comprehensive documentation for all:

  • Core classes (Blob, Buffer, Face, Feature, Font, etc.)
  • Enums (BufferDiffFlags, ClusterLevel, Direction, etc.)
  • Script type with ALL 150+ script field constants
  • All delegates (font + unicode)
  • Generated code with documentation added per your instruction

Ready to continue with the next namespace! Commit: 8b5cc70

Copy link
Contributor Author

Copilot AI commented Sep 5, 2025

@copilot please look at the checklist in API.Docs.Migration.Checklist.md to find out the next things to do. Always refer to the guide for rules and prcedures: API.Docs.Migration.Guide.md

Make sure ...

Continuing the API documentation migration systematically through the checklist. Just completed SkiaSharp.GRBackend and GRBackendRenderTarget types with comprehensive documentation. Working through SkiaSharp.GRBackendRenderTargetDesc next as requested - migrating ALL APIs as specified. Progress: f6c72c0

@Copilot Copilot AI changed the title Migrate API documentation from XML files to inline C# comments for HarfBuzzSharp namespace Migrate API documentation from XML files to inline C# comments for SkiaSharp namespace Sep 5, 2025
Copilot finished work on behalf of mattleibow September 5, 2025 01:59
@mattleibow mattleibow closed this Sep 5, 2025
@mattleibow mattleibow deleted the copilot/fix-3357 branch September 5, 2025 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants