-
Notifications
You must be signed in to change notification settings - Fork 945
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
Add Effects to System.Drawing #8835
Comments
Tagging subscribers to this area: @safern, @tannergooding |
I've made an initial implementation and modified the proposal. The basic changes from the initial proposal:
All types and names and values are mapped to GDI+ where applicable. Unusual cases are called out. All are adds. namespace System.Drawing.Imaging.Effects
public abstract class Effect : IDisposable
{
// Cannot be created outside of System.Drawing assembly - has private protected constructor
public void Dispose();
~Effect();
}
// All derived classes don't actually exist as classes in GDI+, but as the int adjustment value bounds are
// so different for each effect type it seems better from a documentation perspective that subclasses with
// fully documented <param>'s are better than having to cross lookup against an enum of ColorCurveEffect types.
public abstract class ColorCurveEffect : Effect
{
// Cannot be created outside of System.Drawing assembly - has private protected constructor
public CurveChannel Channel { get; }
}
public enum CurveChannel
{
// Directly mapped to GDI+ values
CurveChannelAll
CurveChannelRed
CurveChannelGreen
CurveChannelBlue
}
// Basically a black point per channel effect
public class BlackSaturationEffect : ColorCurveEffect
{
public BlackSaturationEffect(CurveChannel channel, int blackSaturation);
public int BlackSaturation { get; }
}
public class BlurEffect : Effect
{
public BlurEffect(float radius, bool expandEdge) ;
public float Radius { get; }
public bool ExpandEdge { get; }
}
public class BrightnessContrastEffect : Effect
{
public BrightnessContrastEffect(int brightnessLevel, int contrastLevel);
public int BrightnessLevel { get; }
public int ContrastLevel { get; }
}
public class ColorBalanceEffect : Effect
{
public ColorBalanceEffect(int cyanRed, int magentaGreen, int yellowBlue) : base(PInvoke.ColorBalanceEffectGuid);
public int CyanRed { get; }
public int MagentaGreen { get; }
public int YellowBlue { get; }
}
public class ColorLookupTableEffect : Effect
{
// A copy of lookup table data is kept in the class
public ColorLookupTableEffect(
ReadOnlySpan<byte> redLookupTable,
ReadOnlySpan<byte> greenLookupTable,
ReadOnlySpan<byte> blueLookupTable,
ReadOnlySpan<byte> alphaLookupTable);
public ColorLookupTableEffect(
byte[] redLookupTable,
byte[] greenLookupTable,
byte[] blueLookupTable,
byte[] alphaLookupTable);
public ReadOnlySpan<byte> RedLookupTable { get; }
public ReadOnlySpan<byte> GreenLookupTable { get; }
public ReadOnlySpan<byte> BlueLookupTable { get; }
public ReadOnlySpan<byte> AlphaLookupTable { get; }
}
public class ColorMatrixEffect : Effect
{
public ColorMatrixEffect(ColorMatrix matrix);
// ColorMatrix is a mutable class, but won't change what this effect does.
public ColorMatrix Matrix { get; }
}
// These are just some predefined ColorMatrix values to cover some common scenarios.
public class GrayScaleEffect : ColorMatrixEffect
{
public GrayScaleEffect();
}
public class SepiaEffect : ColorMatrixEffect
{
public GrayScaleEffect();
}
public class VividEffect : ColorMatrixEffect
{
public GrayScaleEffect();
}
public class InvertEffect : ColorMatrixEffect
{
public GrayScaleEffect();
}
// Different from BrightnessContrast in that you can adjust a single channel (and the implementation seems completely different)
public class ContrastEffect : ColorCurveEffect
{
public ContrastEffect(CurveChannel channel, int contrast);
public int Contrast { get; }
}
public class DensityEffect : ColorCurveEffect
{
public DensityEffect(CurveChannel channel, int density);
public int Density { get; }
}
public class ExposureEffect : ColorCurveEffect
{
public ExposureEffect(CurveChannel channel, int exposure);
public int Exposure { get; }
}
public class HighlightEffect : ColorCurveEffect
{
public HighlightEffect(CurveChannel channel, int highlight);
public int Highlight { get; }
}
public class LevelsEffect : Effect
{
public LevelsEffect(int highlight, int midtone, int shadow);
public int Highlight { get; }
public int Midtone { get; }
public int Shadow { get; }
}
public class MidtoneEffect : ColorCurveEffect
{
public MidtoneEffect(CurveChannel channel, int midtone);
public int Midtone { get; }
}
public class RedEyeCorrectionEffect: Effect
{
public RedEyeCorrectionEffect(ReadOnlySpan<Point> areas);
public RedEyeCorrectionEffect(Point[] areas);
// As this is a one and done sort of effect I don't want to expose the areas to avoid unnecessary copies.
// Or could and say that this will allocate? All GDI+ effects make a copy the incoming data.
// public Point[] Areas { get; }
}
public class ShadowEffect : ColorCurveEffect
{
public ShadowEffect(CurveChannel channel, int shadow);
public int Shadow { get; }
}
public class SharpenEffect : Effect
{
public SharpenEffect(float radius, float amount);
public float Radius => _sharpenParams.radius;
public float Amount => _sharpenParams.amount;
}
public class TintEffect : Effect
{
// Convenience constructor. It's lossy so I don't expose Color in a getter.
public TintEffect(Color color, int amount);
// This is the only effect where I've changed behavior. The hue goes from -180 to 180 in GDI+ on this API,
// but Color.GetHue() goes from 0 to 360. As such, this takes 0-360.
public TintEffect(int hue, int amount);
public int Hue { get; }
public int Amount { get; }
}
public class WhiteSaturationEffect : ColorCurveEffect
{
public WhiteSaturationEffect(CurveChannel channel, int whiteSaturation);
public int WhiteSaturation { get; }
} Here is the diff for namespace System.Drawing;
public class Bitmap
{
+ // Empty rectangle is the entire image
+ public void ApplyEffect(Effect effect, Rectangle area = default);
}
public class Graphics
{
+ public void DrawImage(
+ Image image,
+ Effect effect,
+ RectangleF srcRect = default,
+ Matrix? transform = default,
+ GraphicsUnit srcUnit = GraphicsUnit.Pixel,
+ ImageAttributes? imageAttr = default);
} Related to this I'm also asking for a new constructor on namespace System.Drawing.Imaging;
public sealed class ColorMatrix
{
public ColorMatrix();
public ColorMatrix(float[][] newColorMatrix);
+ public ColorMatrix(ReadOnlySpan<float> newColorMatrix);
} |
@tannergooding fyi |
Can you elaborate on how it's an outlier? This is actually the only GDI+ effect that I use in paint.net 😂 Originally I was p/invoking to my own C++ wrapper for it, then I transitioned to @tannergooding 's TerraFX.Interop.Windows exposure of it so I could eliminate more non-C# code (which has been a very successful endeavor) |
@rickbrew I can put it in. I was a little concerned about the expectation that there would be an idea that other "retouching" effects would be forthcoming. |
@rickbrew Related to your TerraFX comment, note that I just completely redid System.Drawing to use CsWin32, which is part of why I was able to tackle this now. Not having to manually define the interop definitions is a major timesaver (both from an implementation and review perspective). If there are any pain points I can address please let me know. |
I try to minimize my use of GDI+ and System.Drawing as much as possible in favor of WIC, D2D, and my own stuff. so it’s not necessary for my sake — just pointing out it does get use, and I was curious about the exclusion. |
Not sure what you mean—it is exposed through Effect::GetAuxData. Agree that it's a bit niche, though.
Any reason you skipped the non-mutating Effect::ApplyEffect? I do realize it isn't quite right in my proposal ("Integer that specifies the number of input bitmaps... must be set to 1," so, it shouldn't take an array) but it seems generally useful to have something that returns a copied Bitmap. |
Sorry, getting confused by the various ways the code is exposed. The
Because it wasn't that much more efficient than |
namespace System.Drawing.Imaging.Effects;
public abstract class Effect : IDisposable
{
// Cannot be created outside of System.Drawing assembly - has private protected constructor
private protected Effect();
public void Dispose();
protected virtual void Dispose(bool disposing);
~Effect();
}
// All derived classes don't actually exist as classes in GDI+, but as the int adjustment value bounds are
// so different for each effect type it seems better from a documentation perspective that subclasses with
// fully documented <param>'s are better than having to cross lookup against an enum of ColorCurveEffect types.
public abstract class ColorCurveEffect : Effect
{
// Cannot be created outside of System.Drawing assembly - has private protected constructor
private protected Effect();
public CurveChannel Channel { get; }
}
public enum CurveChannel
{
// Directly mapped to GDI+ values
All,
Red,
Green,
Blue,
}
// Basically a black point per channel effect
public class BlackSaturationCurveEffect : ColorCurveEffect
{
public BlackSaturationCurveEffect(CurveChannel channel, int blackSaturation);
public int BlackSaturation { get; }
}
public class BlurEffect : Effect
{
public BlurEffect(float radius, bool expandEdge) ;
public float Radius { get; }
public bool ExpandEdge { get; }
}
public class BrightnessContrastEffect : Effect
{
public BrightnessContrastEffect(int brightnessLevel, int contrastLevel);
public int BrightnessLevel { get; }
public int ContrastLevel { get; }
}
public class ColorBalanceEffect : Effect
{
public ColorBalanceEffect(int cyanRed, int magentaGreen, int yellowBlue) : base(PInvoke.ColorBalanceEffectGuid);
public int CyanRed { get; }
public int MagentaGreen { get; }
public int YellowBlue { get; }
}
public class ColorLookupTableEffect : Effect
{
// A copy of lookup table data is kept in the class
public ColorLookupTableEffect(
ReadOnlySpan<byte> redLookupTable,
ReadOnlySpan<byte> greenLookupTable,
ReadOnlySpan<byte> blueLookupTable,
ReadOnlySpan<byte> alphaLookupTable);
public ColorLookupTableEffect(
byte[] redLookupTable,
byte[] greenLookupTable,
byte[] blueLookupTable,
byte[] alphaLookupTable);
public ReadOnlyMemory<byte> RedLookupTable { get; }
public ReadOnlyMemory<byte> GreenLookupTable { get; }
public ReadOnlyMemory<byte> BlueLookupTable { get; }
public ReadOnlyMemory<byte> AlphaLookupTable { get; }
}
public class ColorMatrixEffect : Effect
{
public ColorMatrixEffect(ColorMatrix matrix);
// ColorMatrix is a mutable class, but won't change what this effect does.
public ColorMatrix Matrix { get; }
}
// These are just some predefined ColorMatrix values to cover some common scenarios.
public sealed class GrayScaleEffect : ColorMatrixEffect
{
public GrayScaleEffect();
}
public sealed class SepiaEffect : ColorMatrixEffect
{
public SepiaEffect();
}
public sealed class VividEffect : ColorMatrixEffect
{
public VividEffect();
}
public sealed class InvertEffect : ColorMatrixEffect
{
public InvertEffect();
}
// Different from BrightnessContrast in that you can adjust a single channel (and the implementation seems completely different)
public class ContrastCurveEffect : ColorCurveEffect
{
public ContrastCurveEffect(CurveChannel channel, int contrast);
public int Contrast { get; }
}
public class DensityCurveEffect : ColorCurveEffect
{
public DensityCurveEffect(CurveChannel channel, int density);
public int Density { get; }
}
public class ExposureCurveEffect : ColorCurveEffect
{
public ExposureCurveEffect(CurveChannel channel, int exposure);
public int Exposure { get; }
}
public class HighlightCurveEffect : ColorCurveEffect
{
public HighlightCurveEffect(CurveChannel channel, int highlight);
public int Highlight { get; }
}
public class LevelsEffect : Effect
{
public LevelsEffect(int highlight, int midtone, int shadow);
public int Highlight { get; }
public int Midtone { get; }
public int Shadow { get; }
}
public class MidtoneCurveEffect : ColorCurveEffect
{
public MidtoneCurveEffect(CurveChannel channel, int midtone);
public int Midtone { get; }
}
public class RedEyeCorrectionEffect: Effect
{
public RedEyeCorrectionEffect(params ReadOnlySpan<Point> areas);
public RedEyeCorrectionEffect(params Point[] areas);
// As this is a one and done sort of effect I don't want to expose the areas to avoid unnecessary copies.
// Or could and say that this will allocate? All GDI+ effects make a copy the incoming data.
// public Point[] Areas { get; }
public ReadOnlyMemory<Point> Areas { get; }
}
public class ShadowCurveEffect : ColorCurveEffect
{
public ShadowEffect(CurveChannel channel, int shadow);
public int Shadow { get; }
}
public class SharpenEffect : Effect
{
public SharpenEffect(float radius, float amount);
public float Radius => _sharpenParams.radius;
public float Amount => _sharpenParams.amount;
}
public class TintEffect : Effect
{
// Convenience constructor. It's lossy so I don't expose Color in a getter.
public TintEffect(Color color, int amount);
// This is the only effect where I've changed behavior. The hue goes from -180 to 180 in GDI+ on this API,
// but Color.GetHue() goes from 0 to 360. As such, this takes 0-360.
public TintEffect(int hue, int amount);
public int Hue { get; }
public int Amount { get; }
}
public class WhiteSaturationCurveEffect : ColorCurveEffect
{
public WhiteSaturationEffect(CurveChannel channel, int whiteSaturation);
public int WhiteSaturation { get; }
}
namespace System.Drawing;
public partial class Bitmap
{
// Empty rectangle is the entire image
public void ApplyEffect(Effect effect, Rectangle area = default);
}
public partial class Graphics
{
public void DrawImage(Image image, Effect effect);
public void DrawImage(
Image image,
Effect effect,
RectangleF srcRect = default,
Matrix? transform = default,
GraphicsUnit srcUnit = GraphicsUnit.Pixel,
ImageAttributes? imageAttr = default);
}
namespace System.Drawing.Imaging;
public sealed partial class ColorMatrix
{
public ColorMatrix(ReadOnlySpan<float> newColorMatrix);
} |
This updates Effects to match the approved API and removes [RequiresPreviewFeatures]. Fixes dotnet#8835 dotnet#8835 (comment)
This updates Effects to match the approved API and removes [RequiresPreviewFeatures]. Fixes #8835 #8835 (comment)
Everything is implemented except for RedEyeReduction. |
This updates Effects to match the approved API and removes [RequiresPreviewFeatures]. Fixes dotnet#8835 dotnet#8835 (comment)
Background
A major addition in GDI+1.1 was the addition of effects. GDI+ comes with eleven effect types which perform various adjustments and transformations on images, like blurs, color balancing, brightness/contrast adjustments, etc. This proposal is one of many to add missing GDI+ 1.1 functionality to
System.Drawing
.API Proposal
Updated proposal: #8835 (comment)
See the documentation for the gdipluseffects.h header and the corresponding GDI+ flat APIs.
GetColorLookupTableFromAuxiliaryData
is a bit of a mouthful, but I can justify the choice. In GDI+, anEffect
has a propertyUseAuxData
and a methodVOID* GetAuxData()
. After callingApplyEffect
, ifUseAuxData
is set to true, the aux data is populated based on the actual type of the effect used. At this moment, that can either be no data (where this method would throw an exception) or aColorLookupTable
. Technically, another iteration of GDI+ could add a third type of aux data, but that's likely to never happen.This API differs from the native definitions slightly:
Effect
suffix is not present in the C++ definitionsThis requires changes to libgdiplus in order to support it on non-Windows platforms.
The text was updated successfully, but these errors were encountered: