Skip to content

[CoreImage] Enable Lab/RGB conversion filters. Fixes #15662 - #26918

Open
rolfbjarne wants to merge 6 commits into
mainfrom
dev/rolf/coreimage-lab-filters
Open

rolfbjarne wants to merge 6 commits into
mainfrom
dev/rolf/coreimage-lab-filters

Conversation

@rolfbjarne

Copy link
Copy Markdown
Member

The Lab/RGB conversion filters introduced in Xcode 14 were disabled because the original bindings used factory-method names and mapped both filters to a nonexistent CIConvertLab class, causing filter construction and introspection failures.

Expose CIConvertLabToRGB and CIConvertRGBtoLab using their native filter names and the existing shared CIConvertLabProtocol. This preserves separate, strongly typed filters without introducing a shared concrete class. Add them to the handle-based filter lookup, remove their introspection exclusions, and extend the existing documentation baseline.

Regression coverage exercises both conversion directions, named lookup, nullable image input, output-image creation, and normalization round trips.

Fixes #15662

🤖 Pull request created by Copilot

rolfbjarne and others added 2 commits October 19, 2026 13:56
Bind CIConvertLabToRGB and CIConvertRGBtoLab using their native filter names and the shared CIConvertLab protocol. The previously disabled declarations used factory-method names and incorrectly mapped both filters to a CIConvertLab class, which prevented native filter creation.

Restore introspection coverage, wire the handle-based filter lookup, and add regression coverage for construction, named lookup, image input/output, and normalization. Extend the existing documentation baseline for the new filter types and generated properties.

Fixes #15662

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 41349df9-d66f-40c1-9d7b-e974f10d6d6a
Keep the Lab conversion regression focused on successful native lookup and the selected filter name. CIFilter.FromName returns a CIFilter wrapper rather than promising a concrete generated filter subclass. Create and dispose the input color explicitly using the existing CIColor factory.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 41349df9-d66f-40c1-9d7b-e974f10d6d6a
@rolfbjarne
rolfbjarne requested a review from dalexsoto as a code owner October 19, 2026 13:16
Copilot AI balanced review requested due to automatic review settings October 19, 2026 13:16

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Native construction and cross-platform introspection validation remain unconfirmed.

1 open issue
What changed in this PR

Enables strongly typed CoreImage Lab/RGB conversion filters, addressing #15662 without introducing a shared concrete class.

Changes:

  • Uses native filter names and adds handle-based wrapper mappings.
  • Removes introspection exclusions and extends the documentation baseline.
  • Adds regression tests for both conversion directions.
File Description
tests/​monotouch-test/​CoreImage/​FilterTest.cs Tests construction, properties, output, and named lookup.
tests/​introspection/​ApiCoreImageFiltersTest.cs Removes conversion-filter exclusions.
tests/​cecil-tests/​Documentation.KnownFailures.txt Records undocumented new APIs.
src/​CoreImage/​CIFilter.cs Adds handle-based conversion-filter mappings.
src/​coreimage.cs Enables separate bindings with platform availability.

Review effort: Balanced

Comment thread src/CoreImage/CIFilter.cs
@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

Allow the Bto fragment produced when the spelling test splits CIConvertRGBtoLab. The managed filter deliberately follows the native filter name, so this fragment is not a spelling mistake.

Apply the eight AppSizeTest baseline diffs posted on PR #26918 for CI build 20261019.17. These update preserved registrar callback identifiers and the macOS CoreCLR app sizes after enabling the two conversion filters.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 41349df9-d66f-40c1-9d7b-e974f10d6d6a
Copilot AI balanced review requested due to automatic review settings October 20, 2026 12:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Native construction, introspection behavior, and updated baselines still need platform CI validation.

1 open issue
1 previously missed issue

In code that hasn't changed since last review

Medium severity Add regression coverage for handle-based ConvertLab mapping

src/​CoreImage/​CIFilter.cs:742

💡 Testing — The new ConvertLab test calls the public FromName(string) binding (filterWithName:), not this internal handle-based overload used by CIImage.WrapFilters. Removing or swapping these cases would leave that test passing. Add regression coverage that exercises this overload for both filter names and asserts the corresponding concrete wrapper type, while preserving native-handle ownership.

[!NOTE]
AI-generated by GitHub Copilot.

Review effort: Balanced

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

Copilot AI balanced review requested due to automatic review settings October 23, 2026 07:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Native construction and introspection behavior remain unconfirmed without completed Apple-platform build, simulator, and API-diff validation.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

Copy link
Copy Markdown
Collaborator

⚠️ AppSizeTest expected files changed ⚠️

The AppSizeTest detected changes in the expected app size files.

To update the expected files, add a comment with the following command:

/apply-gist https://gist.github.com/vs-mobiletools-engineering-service2/de7bec2ec66eea22937a206830b5eb27
Updated files
  • iOS-CoreCLR-Interpreter-preservedapis.txt
  • iOS-CoreCLR-R2R-preservedapis.txt
  • MacCatalyst-CoreCLR-Interpreter-preservedapis.txt
  • MacCatalyst-CoreCLR-R2R-preservedapis.txt
  • MacOSX-CoreCLR-Interpreter-size.txt
  • MacOSX-CoreCLR-R2R-size.txt
  • TVOS-CoreCLR-Interpreter-preservedapis.txt
  • TVOS-CoreCLR-R2R-preservedapis.txt

Pipeline on Agent
Hash: 6c4cc0ab7815849a64cd59157c17576ce7431db5 [PR build]

@rolfbjarne

Copy link
Copy Markdown
Member Author

/apply-gist https://gist.github.com/vs-mobiletools-engineering-service2/de7bec2ec66eea22937a206830b5eb27

@github-actions

Copy link
Copy Markdown
Contributor

✅ Applied expected app size files from gist.

Copilot AI balanced review requested due to automatic review settings October 23, 2026 15:38

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Cross-platform native filter behavior and refreshed baselines still require CI confirmation before human approval.

No open issues 🎉

Review effort: Balanced

@vs-mobiletools-engineering-service2

Copy link
Copy Markdown
Collaborator

✅ API diff for current PR / commit

NET (empty diffs)

✅ API diff vs stable

NET (empty diffs)

ℹ️ Generator diff

Generator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes)

Pipeline on Agent
Hash: d45d62464c1710365e6bdd5b4362659bd73779e6 [PR build]

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

Copy link
Copy Markdown
Collaborator

🔥 [CI Build #d45d624] Test results 🔥

Test results

❌ Tests failed on VSTS: test results

0 tests crashed, 1 tests failed, 216 tests passed.

Failures

❌ dotnettests tests (macOS) [attempt 2]

1 tests failed, 0 tests passed.

Failed tests

  • DotNet tests: Failed (Execution failed with exit code 1)
    • Xamarin.Tests.AssetsTest.TestXCAssets(MacOSX,"osx-x64",True): '/Users/cloudtest/vss/_work/1/s/macios/tests/dotnet/AppWithXCAssets/macOS/bin/Debug/net11.0-macos/osx-x64/AppWithXCAssets.app/Co...

Html Report (VSDrops) Download

Successes

✅ assembly-processing: All 1 tests passed. Html Report (VSDrops) Download
✅ cecil: All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (iOS): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (MacCatalyst): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (Multiple platforms): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (tvOS): All 1 tests passed. Html Report (VSDrops) Download
✅ framework: All 2 tests passed. Html Report (VSDrops) Download
✅ fsharp: All 4 tests passed. Html Report (VSDrops) Download
✅ generator: All 1 tests passed. Html Report (VSDrops) Download
✅ interdependent-binding-projects: All 4 tests passed. Html Report (VSDrops) Download
✅ introspection: All 4 tests passed. Html Report (VSDrops) Download
✅ linker (iOS): All 21 tests passed. Html Report (VSDrops) Download
✅ linker (MacCatalyst): All 21 tests passed. Html Report (VSDrops) Download
✅ linker (macOS): All 21 tests passed. Html Report (VSDrops) Download
✅ linker (tvOS): All 21 tests passed. Html Report (VSDrops) Download
✅ monotouch (iOS): All 20 tests passed. Html Report (VSDrops) Download
✅ monotouch (MacCatalyst): All 20 tests passed. Html Report (VSDrops) Download
✅ monotouch (macOS): All 20 tests passed. Html Report (VSDrops) Download
✅ monotouch (tvOS): All 20 tests passed. Html Report (VSDrops) Download
✅ msbuild: All 2 tests passed. Html Report (VSDrops) Download
✅ sharpie: All 1 tests passed. Html Report (VSDrops) Download
✅ windows: All 3 tests passed. Html Report (VSDrops) Download
✅ xcframework: All 4 tests passed. Html Report (VSDrops) Download
✅ xtro: All 1 tests passed. Html Report (VSDrops) Download

macOS tests

✅ Tests on macOS Sonoma (14): All 5 tests passed. Html Report (VSDrops) Download
✅ Tests on macOS Sequoia (15): All 5 tests passed. Html Report (VSDrops) Download
✅ Tests on macOS Tahoe (26): All 5 tests passed. Html Report (VSDrops) Download
✅ Tests on macOS Golden Gate (27): All 5 tests passed. Html Report (VSDrops) Download

Linux Build Verification

✅ Linux build succeeded

Pipeline on Agent
Hash: d45d62464c1710365e6bdd5b4362659bd73779e6 [PR build]

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[xcode14][coreimage] LabToRGBFilter and RGBtoLabFilter disabled in bindings

3 participants