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

Apply updates to XY and Anchor positions based on EXIF metadata #221

Merged
merged 9 commits into from
Feb 9, 2022
12 changes: 5 additions & 7 deletions .github/CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# How to contribute to ImageSharp.Web
JimBobSquarePants marked this conversation as resolved.
Show resolved Hide resolved
# How to contribute to SixLabors.ImageSharp.Web

#### **Did you find a bug?**

Expand All @@ -16,20 +16,18 @@

#### **Do you intend to add a new feature or change an existing one?**

* Suggest your change in the [ImageSharp Gitter Chat Room](https://gitter.im/ImageSharp/General) and start writing code.
* Suggest your change in the [Ideas Discussions Channel](https://github.com/SixLabors/ImageSharp.Web/discussions?discussions_q=category%3AIdeas) and start writing code.

* Do not open an issue on GitHub until you have collected positive feedback about the change. GitHub issues are primarily intended for bug reports and fixes.

#### **Do you have questions about consuming the library or the source code?**

* Ask any question about how to use ImageSharp in the [ImageSharp Gitter Chat Room](https://gitter.im/ImageSharp/General).
* Ask any question about how to use SixLabors.ImageSharp.Web in the [Help Discussions Channel](https://github.com/SixLabors/ImageSharp.Web/discussions?discussions_q=category%3AHelp).

#### Code of Conduct
#### Code of Conduct
This project has adopted the code of conduct defined by the [Contributor Covenant](https://contributor-covenant.org/) to clarify expected behavior in our community.
For more information, see the [.NET Foundation Code of Conduct](https://dotnetfoundation.org/code-of-conduct).

And please remember. ImageSharp.Web is the work of a very, very, small number of developers who struggle balancing time to contribute to the project with family time and work commitments. We encourage you to pitch in and help make our vision of simple accessible imageprocessing available to all. Open Source can only exist with your help.
And please remember. SixLabors.ImageSharp.Web is the work of a very, very, small number of developers who struggle balancing time to contribute to the project with family time and work commitments. We encourage you to pitch in and help make our vision of simple accessible image processing available to all. Open Source can only exist with your help.

Thanks for reading!

James Jackson-South :heart:
40 changes: 40 additions & 0 deletions src/ImageSharp.Web/FormattedImage.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Runtime.CompilerServices;
using SixLabors.ImageSharp.Advanced;
using SixLabors.ImageSharp.Formats;
using SixLabors.ImageSharp.Metadata.Profiles.Exif;
using SixLabors.ImageSharp.PixelFormats;

namespace SixLabors.ImageSharp.Web
Expand Down Expand Up @@ -97,6 +98,45 @@ public static FormattedImage Load(Configuration configuration, Stream source)
/// <param name="destination">The destination stream.</param>
public void Save(Stream destination) => this.Image.Save(destination, this.encoder);

/// <summary>
/// Returns a value indicating whether the source image contains EXIF metadata for <see cref="ExifTag.Orientation"/>
/// indicating that the image is rotated (not flipped).
/// </summary>
/// <param name="orientation">The decoded orientation. Use <see cref="ExifOrientationMode"/> for comparison.</param>
/// <returns>The <see cref="bool"/> indicating whether the image contains EXIF metadata indicating that the image is rotated.</returns>
public bool IsExifRotated(out ushort orientation)
Copy link
Member Author

Choose a reason for hiding this comment

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

I've made this public as it's useful for other processor implementations. The EXIF parsing logic is identical to AutoOrientProcessor<T>.GetExifOrientation()

Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this only checking for rotated images and not flipped ones? I think that made sense when only considering width/height, but because the ResizeWebProcessor also needs to handle the correct X/Y and anchor position, this should also return true when it's flipped.

I'd also rename it to TryGetExifOrientation(out ushort orientation) and return whether the image contains a valid EXIF orientation value or not. Then just check if it's false or the orientation is set to 1 (top/left) before doing any of the transforms.

Copy link
Member Author

@JimBobSquarePants JimBobSquarePants Feb 8, 2022

Choose a reason for hiding this comment

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

Why is this only checking for rotated images and not flipped ones?

Because rotated is an important metric as that affects Size. Flipped does not affect that property and I wanted a quick way to tell, writing the method to reflect that desire. Can easily refactor though to suit your design and move rotate the check inside the processor itself.

Copy link
Member Author

Choose a reason for hiding this comment

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

There you go. Refactored. Much cleaner now.

{
orientation = ExifOrientationMode.Unknown;
if (this.Image.Metadata.ExifProfile != null)
{
IExifValue<ushort> value = this.Image.Metadata.ExifProfile.GetValue(ExifTag.Orientation);
if (value is null)
{
return false;
}

if (value.DataType == ExifDataType.Short)
{
orientation = value.Value;
}
else
{
orientation = Convert.ToUInt16(value.Value);
}

return orientation switch
{
ExifOrientationMode.LeftTop
or ExifOrientationMode.RightTop
or ExifOrientationMode.RightBottom
or ExifOrientationMode.LeftBottom => true,
_ => false,
};
}

return false;
}

/// <summary>
/// Performs application-defined tasks associated with freeing, releasing, or resetting
/// unmanaged resources.
Expand Down
210 changes: 174 additions & 36 deletions src/ImageSharp.Web/Processors/ResizeWebProcessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System.Collections.Generic;
using System.Globalization;
using System.Numerics;
using Microsoft.Extensions.Logging;
using SixLabors.ImageSharp.Metadata.Profiles.Exif;
using SixLabors.ImageSharp.Processing;
Expand Down Expand Up @@ -80,7 +81,7 @@ public FormattedImage Process(
CommandParser parser,
CultureInfo culture)
{
ResizeOptions options = GetResizeOptions(image.Image, commands, parser, culture);
ResizeOptions options = GetResizeOptions(image, commands, parser, culture);

if (options != null)
{
Expand All @@ -90,8 +91,18 @@ public FormattedImage Process(
return image;
}

private static ResizeOptions GetResizeOptions(
Image image,
/// <summary>
/// Parses the command collection returning the resize options.
/// </summary>
/// <param name="image">The image to process.</param>
/// <param name="commands">The ordered collection containing the processing commands.</param>
/// <param name="parser">The command parser use for parting commands.</param>
/// <param name="culture">
/// The <see cref="CultureInfo"/> to use as the current parsing culture.
/// </param>
/// <returns>The <see cref="ResizeOptions"/>.</returns>
internal static ResizeOptions GetResizeOptions(
FormattedImage image,
CommandCollection commands,
CommandParser parser,
CultureInfo culture)
Expand All @@ -101,18 +112,20 @@ private static ResizeOptions GetResizeOptions(
return null;
}

Size size = ParseSize(image, commands, parser, culture);
bool rotated = ShouldHandleExifRotation(image, commands, parser, culture, out ushort orientation);

Size size = ParseSize(rotated, commands, parser, culture);

if (size.Width <= 0 && size.Height <= 0)
{
return null;
}

var options = new ResizeOptions
ResizeOptions options = new()
{
Size = size,
CenterCoordinates = GetCenter(commands, parser, culture),
Position = GetAnchor(commands, parser, culture),
CenterCoordinates = GetCenter(image, orientation, commands, parser, culture),
Position = GetAnchor(orientation, commands, parser, culture),
Mode = GetMode(commands, parser, culture),
Compand = GetCompandMode(commands, parser, culture),
};
Expand All @@ -124,43 +137,20 @@ private static ResizeOptions GetResizeOptions(
}

private static Size ParseSize(
Image image,
bool rotated,
CommandCollection commands,
CommandParser parser,
CultureInfo culture)
{
// The command parser will reject negative numbers as it clamps values to ranges.
int width = (int)parser.ParseValue<uint>(commands.GetValueOrDefault(Width), culture);
int height = (int)parser.ParseValue<uint>(commands.GetValueOrDefault(Height), culture);

// Browsers now implement 'image-orientation: from-image' by default.
// https://developer.mozilla.org/en-US/docs/web/css/image-orientation
// This makes orientation handling confusing for users who expect images to be resized in accordance
// to what they observe rather than pure (and correct) methods.
//
// To accomodate this we parse the dimensions to use based upon decoded EXIF orientation values, switching
// the width/height parameters when images are rotated (not flipped).
// We default to 'true' for EXIF orientation handling. By passing 'false' it can be turned off.
if (!commands.Contains(Orient) || parser.ParseValue<bool>(commands.GetValueOrDefault(Orient), culture))
{
if (image.Metadata.ExifProfile != null)
{
IExifValue<ushort> orientation = image.Metadata.ExifProfile.GetValue(ExifTag.Orientation);
return orientation.Value switch
{
ExifOrientationMode.LeftTop
or ExifOrientationMode.RightTop
or ExifOrientationMode.RightBottom
or ExifOrientationMode.LeftBottom => new Size(height, width),
_ => new Size(width, height),
};
}
}

return new Size(width, height);
return rotated ? new Size(height, width) : new Size(width, height);
}

private static PointF? GetCenter(
FormattedImage image,
ushort orientation,
CommandCollection commands,
CommandParser parser,
CultureInfo culture)
Expand All @@ -172,7 +162,48 @@ or ExifOrientationMode.RightBottom
return null;
}

return new PointF(coordinates[0], coordinates[1]);
PointF center = new(coordinates[0], coordinates[1]);
if (orientation is >= ExifOrientationMode.Unknown and <= ExifOrientationMode.TopLeft)
{
return center;
}

AffineTransformBuilder builder = new();
Size size = image.Image.Size();
Copy link
Contributor

Choose a reason for hiding this comment

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

The image size shouldn't be required when dealing with coordinates ranging from 0 to 1.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd actually forgotten that those coordinates were 0-1. They're not documented well on the underlying type. Suggested fixes are much more useful here that in a separate PR.

I don't want the other changes. I don't want to have to create yet another public type, especially one that is mostly filled with methods specific to ResizeWebProcessor. The PointF Transform method for example requires the X/Y properties to be within a 0-1 range. Not useful for anything that currently exists. You're creating solutions to problems I don't and won't have. Processor implementation outside of this library are the responsibility of whoever develops them not me. I'm building a framework, not a utility belt.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've implemented a proper fix for XY transforms now anyway, took me longer than I'm proud of to figure out. If you look at the image above and compare it to the code the process actually becomes pretty clear.

Copy link
Contributor

Choose a reason for hiding this comment

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

Most of those methods are specific to the ResizeOptions indeed, which can be re-used on different web processors that require resizing (e.g. when trying to crop to a specific target rectangle, which Umbraco does in the CropWebProcessor based on crop coordinates), so it's not really specific to ResizeWebProcessor.

Making the PointF transform use a different range would be as simple as moving the Size variable to a parameter, but I get the point you're making here 😉

I've quickly tested the latest changes and it all seems to work as expected 👍🏻

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for testing, that's very useful 👍


// New XY is calculated based on flipping and rotating the input XY.
// Operations are based upon AutoOrientProcessor implementation.
switch (orientation)
{
case ExifOrientationMode.TopRight:
builder.AppendTranslation(new PointF(size.Width - center.X, 0));
break;
case ExifOrientationMode.BottomRight:
builder.AppendRotationDegrees(180);
break;
case ExifOrientationMode.BottomLeft:
builder.AppendTranslation(new PointF(0, size.Height - center.Y));
break;
case ExifOrientationMode.LeftTop:
builder.AppendRotationDegrees(90);
builder.AppendTranslation(new PointF(size.Width - center.X, 0));
break;
case ExifOrientationMode.RightTop:
builder.AppendRotationDegrees(90);
break;
case ExifOrientationMode.RightBottom:
builder.AppendTranslation(new PointF(0, size.Height - center.Y));
builder.AppendRotationDegrees(270);
break;
case ExifOrientationMode.LeftBottom:
builder.AppendRotationDegrees(270);
break;
default:
return center;
}

Matrix3x2 matrix = builder.BuildMatrix(size);
return Vector2.Transform(center, matrix);
}

private static ResizeMode GetMode(
Expand All @@ -182,10 +213,98 @@ private static ResizeMode GetMode(
=> parser.ParseValue<ResizeMode>(commands.GetValueOrDefault(Mode), culture);

private static AnchorPositionMode GetAnchor(
ushort orientation,
CommandCollection commands,
CommandParser parser,
CultureInfo culture)
=> parser.ParseValue<AnchorPositionMode>(commands.GetValueOrDefault(Anchor), culture);
{
AnchorPositionMode anchor = parser.ParseValue<AnchorPositionMode>(commands.GetValueOrDefault(Anchor), culture);
if (orientation is >= ExifOrientationMode.Unknown and <= ExifOrientationMode.TopLeft)
{
return anchor;
}

/*
Copy link
Member Author

Choose a reason for hiding this comment

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

I had to use a notepad for this bit. I think my logic is sound but could do with a second opinion.

New anchor position is determined by calculating the direction of the anchor relative to the source image.
In the following example, the TopRight anchor becomes BottomRight

T L
+-----------------------+ +--------------+
| *| | |
| | | |
L | TL | R | |
| | | |
| | B | LB | T
| | | |
+-----------------------+ | |
B | |
| |
| *|
+--------------+
R
*/
return anchor switch
{
AnchorPositionMode.Center => anchor,
AnchorPositionMode.Top => orientation switch
{
ExifOrientationMode.BottomLeft or ExifOrientationMode.BottomRight => AnchorPositionMode.Bottom,
ExifOrientationMode.LeftTop or ExifOrientationMode.RightTop => AnchorPositionMode.Left,
ExifOrientationMode.LeftBottom or ExifOrientationMode.RightBottom => AnchorPositionMode.Right,
_ => anchor,
},
AnchorPositionMode.Bottom => orientation switch
{
ExifOrientationMode.BottomLeft or ExifOrientationMode.BottomRight => AnchorPositionMode.Top,
ExifOrientationMode.LeftTop or ExifOrientationMode.RightTop => AnchorPositionMode.Right,
ExifOrientationMode.LeftBottom or ExifOrientationMode.RightBottom => AnchorPositionMode.Left,
_ => anchor,
},
AnchorPositionMode.Left => orientation switch
{
ExifOrientationMode.TopRight or ExifOrientationMode.BottomLeft => AnchorPositionMode.Right,
ExifOrientationMode.LeftTop or ExifOrientationMode.LeftBottom => AnchorPositionMode.Top,
ExifOrientationMode.RightTop or ExifOrientationMode.RightBottom => AnchorPositionMode.Bottom,
_ => anchor,
},
AnchorPositionMode.Right => orientation switch
{
ExifOrientationMode.TopRight or ExifOrientationMode.BottomLeft => AnchorPositionMode.Left,
ExifOrientationMode.LeftTop or ExifOrientationMode.LeftBottom => AnchorPositionMode.Top,
ExifOrientationMode.RightTop or ExifOrientationMode.RightBottom => AnchorPositionMode.Bottom,
_ => anchor,
},
AnchorPositionMode.TopLeft => orientation switch
{
ExifOrientationMode.TopRight or ExifOrientationMode.LeftBottom => AnchorPositionMode.TopRight,
ExifOrientationMode.BottomRight or ExifOrientationMode.RightTop => AnchorPositionMode.BottomLeft,
ExifOrientationMode.BottomLeft or ExifOrientationMode.RightBottom => AnchorPositionMode.BottomRight,
_ => anchor,
},
AnchorPositionMode.TopRight => orientation switch
{
ExifOrientationMode.TopRight or ExifOrientationMode.RightTop => AnchorPositionMode.TopLeft,
ExifOrientationMode.BottomRight or ExifOrientationMode.LeftBottom => AnchorPositionMode.BottomRight,
ExifOrientationMode.BottomLeft or ExifOrientationMode.LeftTop => AnchorPositionMode.BottomLeft,
_ => anchor,
},
AnchorPositionMode.BottomRight => orientation switch
{
ExifOrientationMode.TopRight or ExifOrientationMode.LeftBottom => AnchorPositionMode.BottomLeft,
ExifOrientationMode.BottomRight or ExifOrientationMode.RightTop => AnchorPositionMode.TopRight,
ExifOrientationMode.BottomLeft or ExifOrientationMode.RightBottom => AnchorPositionMode.TopLeft,
_ => anchor,
},
AnchorPositionMode.BottomLeft => orientation switch
{
ExifOrientationMode.TopRight or ExifOrientationMode.RightTop => AnchorPositionMode.BottomRight,
ExifOrientationMode.BottomRight or ExifOrientationMode.LeftBottom => AnchorPositionMode.TopLeft,
ExifOrientationMode.BottomLeft or ExifOrientationMode.LeftTop => AnchorPositionMode.TopRight,
_ => anchor,
},
_ => anchor,
};
}

private static bool GetCompandMode(
CommandCollection commands,
Expand Down Expand Up @@ -222,5 +341,24 @@ private static IResampler GetSampler(CommandCollection commands)

return KnownResamplers.Bicubic;
}

private static bool ShouldHandleExifRotation(FormattedImage image, CommandCollection commands, CommandParser parser, CultureInfo culture, out ushort orientation)
{
// Browsers now implement 'image-orientation: from-image' by default.
// https://developer.mozilla.org/en-US/docs/web/css/image-orientation
// This makes orientation handling confusing for users who expect images to be resized in accordance
// to what they observe rather than pure (and correct) methods.
//
// To accomodate this we parse the dimensions to use based upon decoded EXIF orientation values, switching
// the width/height parameters when images are rotated (not flipped).
// We default to 'true' for EXIF orientation handling. By passing 'false' it can be turned off.
if (commands.Contains(Orient) && !parser.ParseValue<bool>(commands.GetValueOrDefault(Orient), culture))
{
orientation = ExifOrientationMode.Unknown;
return false;
}

return image.IsExifRotated(out orientation);
}
}
}
Loading