-
-
Notifications
You must be signed in to change notification settings - Fork 887
Add Property bag to Configuration and Image Processing Context #1176
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
Changes from 6 commits
9df82f5
0ab64c2
5df21ad
6013926
63453cf
4661fc9
3738418
8738afe
d38764d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,100 @@ | ||
| // Copyright (c) Six Labors and contributors. | ||
| // Licensed under the Apache License, Version 2.0. | ||
|
|
||
| using System; | ||
| using SixLabors.ImageSharp.Processing; | ||
|
|
||
| namespace SixLabors.ImageSharp | ||
| { | ||
| /// <summary> | ||
| /// Adds extensions that allow the processing of images to the <see cref="Image{TPixel}"/> type. | ||
| /// </summary> | ||
| public static class GraphicOptionsDefaultsExtensions | ||
| { | ||
| /// <summary> | ||
| /// Sets the default options against the image processing context. | ||
| /// </summary> | ||
| /// <param name="context">The image processing context to store default against.</param> | ||
| /// <param name="optionsBuilder">The action to update instance of the default options used.</param> | ||
| /// <returns>The passed in <paramref name="context"/> to allow chaining.</returns> | ||
| public static IImageProcessingContext SetGraphicsOptions(this IImageProcessingContext context, Action<GraphicsOptions> optionsBuilder) | ||
| { | ||
| var cloned = context.GetGraphicsOptions().DeepClone(); | ||
| optionsBuilder(cloned); | ||
| context.Properties[typeof(GraphicsOptions)] = cloned; | ||
| return context; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Sets the default options against the configuration. | ||
| /// </summary> | ||
| /// <param name="context">The image processing context to store default against.</param> | ||
| /// <param name="optionsBuilder">The default options to use.</param> | ||
| public static void SetGraphicsOptions(this Configuration context, Action<GraphicsOptions> optionsBuilder) | ||
tocsoft marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| { | ||
| var cloned = context.GetGraphicsOptions().DeepClone(); | ||
| optionsBuilder(cloned); | ||
| context.Properties[typeof(GraphicsOptions)] = cloned; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Sets the default options against the image processing context. | ||
| /// </summary> | ||
| /// <param name="context">The image processing context to store default against.</param> | ||
| /// <param name="options">The default options to use.</param> | ||
| /// <returns>The passed in <paramref name="context"/> to allow chaining.</returns> | ||
| public static IImageProcessingContext SetGraphicsOptions(this IImageProcessingContext context, GraphicsOptions options) | ||
| { | ||
| context.Properties[typeof(GraphicsOptions)] = options; | ||
| return context; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Sets the default options against the configuration. | ||
| /// </summary> | ||
| /// <param name="context">The image processing context to store default against.</param> | ||
| /// <param name="options">The default options to use.</param> | ||
| public static void SetGraphicsOptions(this Configuration context, GraphicsOptions options) | ||
tocsoft marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| { | ||
| context.Properties[typeof(GraphicsOptions)] = options; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Gets the default options against the image processing context. | ||
| /// </summary> | ||
| /// <param name="context">The image processing context to retrieve defaults from.</param> | ||
| /// <returns>The globaly configued default options.</returns> | ||
| public static GraphicsOptions GetGraphicsOptions(this IImageProcessingContext context) | ||
| { | ||
| if (context.Properties.TryGetValue(typeof(GraphicsOptions), out var options) && options is GraphicsOptions go) | ||
| { | ||
| return go; | ||
| } | ||
|
|
||
| var configOptions = context.Configuration.GetGraphicsOptions(); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are we really sure we want to repeat the defaulting logic in all the What are the pros for this decision?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah yeah... If our keys are strongly typed that makes comparison faster and easier too.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FYI an interesting API proposal that also contains the concept of property bags: dotnet/runtime#1793 In total, a bit too verbose for my style, but I like the idea of having custom types for the bag, and (in some form) for the keys. I would also apply it for values, if it makes sense.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I do kinda like the pattern proposed in the runtime but, (and it is a big but) it feels soo overengineered, sometimes (and in my experience usually) the simple solution is the right one. |
||
|
|
||
| // do not cache the fall back to config into the the processing context | ||
| // in case someone want to change the value on the config and expects it re trflow thru | ||
| return configOptions; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Gets the default options against the image processing context. | ||
| /// </summary> | ||
| /// <param name="context">The image processing context to retrieve defaults from.</param> | ||
| /// <returns>The globaly configued default options.</returns> | ||
| public static GraphicsOptions GetGraphicsOptions(this Configuration context) | ||
| { | ||
| if (context.Properties.TryGetValue(typeof(GraphicsOptions), out var options) && options is GraphicsOptions go) | ||
| { | ||
| return go; | ||
| } | ||
|
|
||
| var configOptions = new GraphicsOptions(); | ||
|
|
||
| // capture the fallback so the same instance will always be returned in case its mutated | ||
| context.Properties[typeof(GraphicsOptions)] = configOptions; | ||
| return configOptions; | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| // Copyright (c) Six Labors and contributors. | ||
| // Licensed under the Apache License, Version 2.0. | ||
|
|
||
| using System.Collections.Generic; | ||
| using SixLabors.ImageSharp.Processing.Processors; | ||
|
|
||
| namespace SixLabors.ImageSharp.Processing | ||
|
|
@@ -15,6 +16,12 @@ public interface IImageProcessingContext | |
| /// </summary> | ||
| Configuration Configuration { get; } | ||
|
|
||
| /// <summary> | ||
| /// Gets a set of properties for the Image Processing Context. | ||
| /// </summary> | ||
| /// <remarks>This can be used for storing global settings and defaults to be accessable to processors.</remarks> | ||
| IDictionary<object, object> Properties { get; } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if we want to consider something more strongly-typed here?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no it should be explicitly be as loosely typed as possible and we should not be limiting anything from being but in the property bag, that's why I even when as far as using |
||
|
|
||
| /// <summary> | ||
| /// Gets the image dimensions at the current point in the processing pipeline. | ||
| /// </summary> | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.