Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initial implementation of GDI+ effects #10750

Merged
merged 4 commits into from Jan 30, 2024
Merged

Conversation

JeremyKuhne
Copy link
Member

@JeremyKuhne JeremyKuhne commented Jan 27, 2024

This exposes most of the GDI+ effects under the [RequiresPreviewFeatures] while we take it to .NET API review.

Effects are added under the Imaging\Effects namespace. The new effects are:

  • BlackSaturation
  • Blur
  • BrightnessContrast
  • ColorBalance
  • ColorLookupTable
  • ColorMatrix
  • Contrast
  • Density
  • Exposure
  • Highlight
  • Levels
  • Midtone
  • Shadow
  • Sharpen
  • Tint
  • WhiteSaturation

This also adds:

  • New, efficient constructor to ColorMatrix that takes a span of floats
  • Overload for Graphics.DrawImage that allows you to draw with an effect (and fix the perf of the indexer)
  • An ApplyEffect method on Bitmap that lets you apply an effect to an existing bitmap
  • Adds the uses preview features flag to the scratch projects

Effects are applied to bitmaps, not associated with them. The image doesn't retain a handle, it just modifies the bits using the effect. As such I have not exposed a way to mutate the effect classes after creation. The cost of creating an effects class is not high, but they should be disposed to avoid having them clutter the finalizer queue.

I also did not include the red eye removal effect as it is sort of a one-off from general image effects. GDI+ won't be adding any new effects (these were added over 20 years ago).

Tests are pending.

Microsoft Reviewers: Open in CodeFlow

This exposes most of the GDI+ effects under the [RequiresPreviewFeatures] while we take it to .NET API review.

Effects are added under the `Imaging\Effects` namespace. The new effects are:

- BlackSaturation
- Blur
- BrightnessContrast
- ColorBalance
- ColorLookupTable
- ColorMatrix
- Contrast
- Density
- Exposure
- Highlight
- Levels
- Midtone
- Shadow
- Sharpen
- Tint
- WhiteSaturation

This also adds:

- New, efficient constructor to ColorMatrix that takes a span of floats
- Overload for Graphics.DrawImage that allows you to draw with an effect (and fix the perf of the indexer)
- An ApplyEffect method on Bitmap that lets you apply an effect to an existing bitmap
- Adds the uses preview features flag to the scratch projects

Effects are applied to bitmaps, not associated with them. The image doesn't retain a handle, it just modifies the bits using the effect. As such I have not exposed a way to mutate the effect classes after creation. The cost of creating an effects class is not high, but they should be disposed to avoid having them clutter the finalizer queue.

I also did not include the red eye removal effect as it is sort of a one-off from general image effects. GDI+ won't be adding any new effects (these were added over 20 years ago).

Tests are pending.
@JeremyKuhne JeremyKuhne requested a review from a team as a code owner January 27, 2024 00:33
@ghost ghost assigned JeremyKuhne Jan 27, 2024
@JeremyKuhne
Copy link
Member Author

JeremyKuhne commented Jan 27, 2024

Related to #8835

Copy link

codecov bot commented Jan 27, 2024

Codecov Report

Attention: 120 lines in your changes are missing coverage. Please review.

Comparison is base (daf3f4d) 72.66649% compared to head (98bdc32) 72.65214%.
Report is 13 commits behind head on main.

Additional details and impacted files
@@                 Coverage Diff                 @@
##                main      #10750         +/-   ##
===================================================
- Coverage   72.66649%   72.65214%   -0.01435%     
===================================================
  Files           2903        2925         +22     
  Lines         622024      622685        +661     
  Branches       46863       46918         +55     
===================================================
+ Hits          452003      452394        +391     
- Misses        161715      161952        +237     
- Partials        8306        8339         +33     
Flag Coverage Δ
Debug 72.65214% <77.98165%> (-0.01435%) ⬇️
production 43.47532% <68.50394%> (-0.00232%) ⬇️
test 97.34449% <100.00000%> (+0.00153%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Copy link
Member

@KlausLoeffelmann KlausLoeffelmann left a comment

Choose a reason for hiding this comment

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

Didn't look at each individual line, but it looks very reasonable to me.

One question though:
Don't we need a) modified public-API text files, and b) a list of concrete API changes in the PR/Issue description for the API review?

@JeremyKuhne
Copy link
Member Author

@KlausLoeffelmann #8835 has the API proposal and we don't have api.txt files for System.Drawing. We build for multiple frameworks so I'm not even sure it is possible; I'll look into it when I get a chance.

Copy link
Member

@lonitra lonitra left a comment

Choose a reason for hiding this comment

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

Few minor comments but overall looks good! This is exciting 🥳

Copy link
Member

@lonitra lonitra left a comment

Choose a reason for hiding this comment

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

:shipit:

@JeremyKuhne JeremyKuhne merged commit b5d9e67 into dotnet:main Jan 30, 2024
9 checks passed
@ghost ghost added this to the 9.0 Preview2 milestone Jan 30, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Mar 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants