LT-22324: add OpenType font feature options#870
Open
johnml1135 wants to merge 7 commits intomainfrom
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds OpenType font-feature support to FieldWorks’ existing WinForms/Views stack (LT-22324), including a renderer-neutral feature string model, UI integration, native Uniscribe OpenType shaping, and layered test coverage plus OpenSpec/docs artifacts.
Changes:
- Introduces renderer-neutral
tag=valuefeature parsing/normalization and integrates it into WinForms font-feature UI. - Adds native Uniscribe OpenType shaping/placing path driven by run feature strings, plus deterministic native tests using a committed SIL font fixture.
- Adds managed cache-identity tests and test-only HarfBuzzSharp/SkiaSharp comparison coverage; includes OpenSpec requirements/research/manual evidence docs.
Reviewed changes
Copilot reviewed 33 out of 38 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| openspec/changes/add-opentype-font-features/views-migration-matrix.md | Adds a Views subsystem inventory/migration matrix to guide phased migration and feature parity work. |
| openspec/changes/add-opentype-font-features/tasks.md | Documents implementation/task checklist for Phase 1 OpenType features and associated testing. |
| openspec/changes/add-opentype-font-features/specs/font-feature-settings/spec.md | Defines requirements for feature independence from Graphite and renderer-neutral storage/application. |
| openspec/changes/add-opentype-font-features/specs/architecture/ui-framework/winforms-patterns/spec.md | Specifies WinForms composition/localization requirements for feature controls. |
| openspec/changes/add-opentype-font-features/specs/architecture/ui-framework/views-rendering/spec.md | Specifies Views rendering + cache identity requirements around feature strings. |
| openspec/changes/add-opentype-font-features/specs/architecture/testing/test-strategy/spec.md | Defines layered test strategy including visual baselines and cross-renderer comparisons. |
| openspec/changes/add-opentype-font-features/research.md | Captures research notes and external references (Uniscribe OT APIs, HarfBuzz, Skia/Avalonia). |
| openspec/changes/add-opentype-font-features/proposal.md | Summarizes the change scope, goals, and impacted areas for LT-22324. |
| openspec/changes/add-opentype-font-features/manual-testing.md | Records manual WinApp/WinForms MCP evidence steps and screenshots for the UI changes. |
| openspec/changes/add-opentype-font-features/design.md | Documents design decisions (renderer-neutral model, Uniscribe OT, provider UI seam, test tooling). |
| openspec/changes/add-opentype-font-features/.openspec.yaml | Adds OpenSpec metadata for the change set. |
| Src/views/lib/UniscribeSegment.cpp | Adds OT feature parsing and optional ScriptShape/Place OpenType path during shaping/placing. |
| Src/views/Test/TestViews.vcxproj.filters | Adds Charis SIL test font fixture files to VS filters. |
| Src/views/Test/TestViews.vcxproj | Copies Charis SIL font fixture beside TestViews.exe for deterministic native tests. |
| Src/views/Test/TestUniscribeEngine.h | Adds deterministic native tests for OT feature metrics/pixel deltas and state switching. |
| Src/views/Test/TestData/Fonts/CharisSIL-5.000/README.txt | Adds redistributable font README to support deterministic fixture usage. |
| Src/views/Test/TestData/Fonts/CharisSIL-5.000/OFL.txt | Adds SIL OFL license text for the committed test font fixture. |
| Src/views/Test/RenderEngineTestBase.h | Extends TxtSrc to carry szFontVar feature strings into render props for tests. |
| Src/FwCoreDlgs/FwCoreDlgsTests/FwFontDialogTests.cs | Adds test ensuring OpenType features round-trip and normalize through font dialog save. |
| Src/FwCoreDlgs/FwCoreDlgControls/FwCoreDlgControlsTests/TestFontFeaturesButton.cs | Adds tests for renderer-neutral tag emission and normalization behavior in the control. |
| Src/FwCoreDlgs/FwCoreDlgControls/FwCoreDlgControlsTests/FwFontTabTests.cs | Adds style/font-tab tests ensuring OpenType feature strings round-trip and normalize. |
| Src/FwCoreDlgs/FwCoreDlgControls/FwCoreDlgControlsTests/FwAttributesTests.cs | Adds test verifying attributes control returns normalized feature strings and inheritance status. |
| Src/FwCoreDlgs/FwCoreDlgControls/FontFeaturesButton.cs | Refactors button around provider seam; adds OpenType discovery via GDI table parsing; normalizes input. |
| Src/FwCoreDlgs/FwCoreDlgControls/DefaultFontsControl.resx | Updates help text and group label to generic “Font Options” wording. |
| Src/FwCoreDlgs/FwCoreDlgControls/DefaultFontsControl.cs | Decouples UI enablement from Graphite checkbox; wires provider preference and setup flow. |
| Src/Common/SimpleRootSite/SimpleRootSiteTests/RenderEngineFactoryTests.cs | Adds tests for feature normalization propagation and cache identity behavior. |
| Src/Common/SimpleRootSite/RenderEngineFactory.cs | Adds feature strings into renderer cache key and normalizes/copies features into graphics props. |
| Src/Common/RenderVerification/RenderComparisonTests/RenderComparisonTests.csproj | Adds a test-only comparison project with HarfBuzzSharp/SkiaSharp dependencies. |
| Src/Common/RenderVerification/RenderComparisonTests/HarfBuzzSkiaComparisonTests.cs | Adds HarfBuzz shaping-data toggle test and a basic Skia “non-blank render” comparison test. |
| Src/Common/FwUtils/FwUtilsTests/FontFeatureSettingsTests.cs | Adds unit tests for parsing/normalizing renderer-neutral feature strings. |
| Src/Common/FwUtils/FontFeatureSettings.cs | Introduces renderer-neutral parser/normalizer and validity checks for OpenType tags. |
| Docs/opentype-font-features.md | Documents feature-string model, UI usage, renderer boundaries, and export notes. |
| Directory.Packages.props | Adds centralized package versions for HarfBuzzSharp and SkiaSharp (test infrastructure). |
Comment on lines
+90
to
+92
| if (fontName == ws.DefaultFontName) | ||
| return FontFeatureSettings.Normalize(ws.DefaultFontFeatures); | ||
| return chrp.szFontVar == null ? string.Empty : FontFeatureSettings.Normalize(MarshalEx.UShortToString(chrp.szFontVar)); |
Comment on lines
+196
to
+207
| static const OLECHAR rgchLatn[] = { L'l', L'a', L't', L'n', 0 }; | ||
| static const OLECHAR rgchDflt[] = { L'D', L'F', L'L', L'T', 0 }; | ||
| FwOpenTypeTag rgtagScript[] = { MakeOpenTypeTag(rgchLatn), MakeOpenTypeTag(rgchDflt), 0 }; | ||
| HRESULT hr = E_FAIL; | ||
| for (int itag = 0; itag < isizeof(rgtagScript) / isizeof(rgtagScript[0]); ++itag) | ||
| { | ||
| DISABLE_MULTISCRIBE | ||
| { | ||
| IgnoreHr(hr = pfnShape(uri.hdc, &uri.sc, uri.psa, rgtagScript[itag], 0, vrgich.Begin(), | ||
| &prangeProperties, 1, uri.prgch, uri.cch, cglyphMax, uri.prgCluster, | ||
| vcharProps.Begin(), uri.prgGlyph, vglyphProps.Begin(), &uri.cglyph)); | ||
| } |
Comment on lines
+688
to
+691
| var menu = components.ContextMenu("ContextMenu"); | ||
| m_ids = m_featureProvider.GetFeatureIds(); | ||
| var parserFeatureString = GraphiteFontFeatures.ConvertFontFeatureCodesToIds(m_fontFeatures); | ||
| m_values = ParseFeatureString(m_ids, parserFeatureString); |
Comment on lines
+969
to
+979
| public int[] GetFeatureValues(int featureId, int maxValues, out int valueCount, out int defaultValue) | ||
| { | ||
| defaultValue = 0; | ||
| valueCount = 2; | ||
| return new[] { 0, 1 }; | ||
| } | ||
|
|
||
| public string GetFeatureValueLabel(int featureId, int valueId, int languageId) | ||
| { | ||
| return valueId == 0 ? "Off" : "On"; | ||
| } |
Comment on lines
+997
to
+1018
| public static IReadOnlyList<string> GetFeatureTags(IntPtr hdc) | ||
| { | ||
| var tags = new SortedSet<string>(StringComparer.Ordinal); | ||
| foreach (var table in s_layoutTables) | ||
| { | ||
| var tableData = ReadTable(hdc, table); | ||
| if (tableData != null) | ||
| ReadFeatureList(tableData, tags); | ||
| } | ||
| return tags.ToArray(); | ||
| } | ||
|
|
||
| private static byte[] ReadTable(IntPtr hdc, uint table) | ||
| { | ||
| var size = GetFontData(hdc, table, 0, null, 0); | ||
| if (size == GdiError || size == 0) | ||
| return null; | ||
|
|
||
| var data = new byte[size]; | ||
| var bytesRead = GetFontData(hdc, table, 0, data, size); | ||
| return bytesRead == GdiError ? null : data; | ||
| } |
Comment on lines
+44
to
+46
| HDC hdc = ::CreateCompatibleDC(::GetDC(::GetDesktopWindow())); | ||
| HBITMAP hbm = ::CreateCompatibleBitmap(hdc, dxMax, dxMax); | ||
| ::SelectObject(hdc, hbm); |
Comment on lines
+246
to
+248
| m_defaultFontFeaturesButton.UseGraphiteFeatures = m_ws.IsGraphiteEnabled; | ||
| m_defaultFontFeaturesButton.FontName = m_defaultFontComboBox.Text; | ||
| m_defaultFontFeaturesButton.SetupFontFeatures(); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR contains only the direct OpenType font feature work from LT-22324 after splitting the broader branch into separate reviewable pieces.
Included
Excluded
This change is