Skip to content

PDF-2231: Per-page multi-page TIFF (mixed orientation) + resolution/Rgb24 writer fixes #150

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.

Sign up for GitHub

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

Open
Sawraz-IS wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

PDF-2231: Per-page multi-page TIFF (mixed orientation) + resolution/Rgb24 writer fixes #150

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
05a1614
PDF-2231: Fixed Multi Page TIFF Mixed Orientation
Sawraz-IS Jun 8, 2026
85fdaf0
Fixed resolution unit-conversion bug
Sawraz-IS Jun 8, 2026
6c3a9c9
Fixed corruption of true 3-byte Rgb24 input
Sawraz-IS Jun 8, 2026
9abef96
update(PDF-2231): Address PR Review Feedbacks
Sawraz-IS Jun 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
204 changes: 204 additions & 0 deletions IronSoftware.Drawing/IronSoftware.Drawing.Common.Tests/UnitTests/AnyBitmapFunctionality.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
using BitMiracle.LibTiff.Classic;
using FluentAssertions;
using SixLabors.ImageSharp;
using SixLabors.ImageSharp.Metadata;
using SixLabors.ImageSharp.PixelFormats;
using SixLabors.ImageSharp.Processing;
using System;
Expand Down Expand Up @@ -624,6 +626,208 @@ public void Create_Multi_page_Tiff_Paths()
File.Delete(outputImagePath);
}

[FactWithAutomaticDisplayName]
public void CreateMultiFrameTiffStream_Preserves_Mixed_Orientation()
{
// A multi-page TIFF must keep each page's native dimensions.
// Pages of differing orientation must not be scaled to a common size.
using var portrait = CreateSolidBitmap(120, 200, new Rgb24(220, 30, 30), 150);
using var landscape = CreateSolidBitmap(200, 120, new Rgb24(30, 30, 220), 150);

using var stream = AnyBitmap.CreateMultiFrameTiffStream(new[] { portrait, landscape });
var pages = ReadTiffDirectories(stream.ToArray());

pages.Should().HaveCount(2, "each source image becomes one TIFF page");
pages[0].Width.Should().Be(120);
pages[0].Height.Should().Be(200);
pages[1].Width.Should().Be(200);
pages[1].Height.Should().Be(120);
}

[FactWithAutomaticDisplayName]
public void CreateMultiFrameTiffBytes_Preserves_Per_Page_Dimensions_And_Resolution()
{
using var first = CreateSolidBitmap(300, 400, new Rgb24(10, 200, 10), 150);
using var second = CreateSolidBitmap(640, 360, new Rgb24(200, 200, 10), 150);
using var third = CreateSolidBitmap(200, 200, new Rgb24(10, 10, 200), 150);

byte[] tiff = AnyBitmap.CreateMultiFrameTiffBytes(new[] { first, second, third });
var pages = ReadTiffDirectories(tiff);

pages.Should().HaveCount(3);
pages[0].Width.Should().Be(300);
pages[0].Height.Should().Be(400);
pages[1].Width.Should().Be(640);
pages[1].Height.Should().Be(360);
pages[2].Width.Should().Be(200);
pages[2].Height.Should().Be(200);

foreach (var page in pages)
{
ToDotsPerInch(page.XResolution, page.ResolutionUnit)

@mee-ironsoftware mee-ironsoftware Jun 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Low/Nit: This DPI assertion only ever sees the INCH branch because CreateSolidBitmap sets PixelsPerInch. The ToDotsPerInch helper already handles CENTIMETER, so adding a PixelsPerMeter source here would extend this same test to cover the actual resolution fix (see the Medium on AnyBitmap.cs).

.Should().BeApproximately(150d, 2d);
}
}

[FactWithAutomaticDisplayName]
public void CreateMultiFrameTiffStream_Empty_Sequence_Throws()
{
Action act = () => AnyBitmap.CreateMultiFrameTiffStream(new List<AnyBitmap>());
act.Should().Throw<ArgumentException>();
}

[FactWithAutomaticDisplayName]
public void CreateMultiFrameTiffBytes_PreservesResolution_FromPixelsPerMeterSource()
{
const double dpi = 300d;
double pixelsPerMetre = dpi / 0.0254d; // 300 DPI expressed in pixels per metre

using var page = MakeBitmapWithResolution(220, 300, new Rgb24(40, 90, 160),
pixelsPerMetre, PixelResolutionUnit.PixelsPerMeter);

byte[] tiff = AnyBitmap.CreateMultiFrameTiffBytes(new[] { page });
var pages = ReadTiffDirectories(tiff);

pages.Should().ContainSingle();
ToDotsPerInch(pages[0].XResolution, pages[0].ResolutionUnit)
.Should().BeApproximately(dpi, 2d);
}

[FactWithAutomaticDisplayName]
public void CreateMultiFrameTiff_Preserves_Rgb24_Pixels()
{
string jpgPath = GetRelativeFilePath("mountainclimbers.jpg");
using var expected = SixLabors.ImageSharp.Image.Load<Rgb24>(jpgPath);

using var result = AnyBitmap.CreateMultiFrameTiff(new List<string> { jpgPath });

result.Width.Should().Be(expected.Width);
result.Height.Should().Be(expected.Height);

var points = new[]
{
(1, 0),
(expected.Width - 1, 0),
(expected.Width / 3, expected.Height / 2),
(expected.Width / 2, expected.Height / 3),
(0, expected.Height - 1),
(expected.Width - 1, expected.Height - 1)
};

foreach (var (x, y) in points)
{
Rgb24 e = expected[x, y];
var a = result.GetPixel(x, y);
a.R.Should().Be(e.R, $"red channel at ({x},{y})");
a.G.Should().Be(e.G, $"green channel at ({x},{y})");
a.B.Should().Be(e.B, $"blue channel at ({x},{y})");
}
}

[TheoryWithAutomaticDisplayName]
[InlineData("Rgb24")]
[InlineData("Bgr24")]
[InlineData("Rgba32")]
[InlineData("Bgra32")]
[InlineData("Abgr32")]
[InlineData("Argb32")]
public void CreateMultiFrameTiff_PreservesColors_ForAllPixelFormats(string pixelFormat)
{
const byte r = 10, g = 120, b = 240;
using var bmp = MakeSolidBitmapOfFormat(pixelFormat, 64, 48, r, g, b);

using var result = AnyBitmap.CreateMultiFrameTiff(new[] { bmp });

result.Width.Should().Be(64);
result.Height.Should().Be(48);

foreach (var (x, y) in new[] { (0, 0), (63, 0), (0, 47), (32, 24), (63, 47) })
{
var px = result.GetPixel(x, y);
px.R.Should().Be(r, $"R at ({x},{y}) for {pixelFormat}");
px.G.Should().Be(g, $"G at ({x},{y}) for {pixelFormat}");
px.B.Should().Be(b, $"B at ({x},{y}) for {pixelFormat}");
}
}

/// <summary>
/// Builds a solid <see cref="AnyBitmap"/> whose backing image uses the requested
/// ImageSharp pixel format. The colour is given in logical R,G,B order regardless of
/// the format's in-memory byte layout. The image is force-loaded so the original
/// pixel format (not a re-encoded copy) reaches the TIFF writer.
/// </summary>
private static AnyBitmap MakeSolidBitmapOfFormat(string format, int width, int height, byte r, byte g, byte b)
{
Image image = format switch
{
"Rgb24" => new Image<Rgb24>(width, height, new Rgb24(r, g, b)),
"Bgr24" => new Image<Bgr24>(width, height, new Bgr24(r, g, b)),
"Rgba32" => new Image<Rgba32>(width, height, new Rgba32(r, g, b, 255)),
"Bgra32" => new Image<Bgra32>(width, height, new Bgra32(r, g, b, 255)),
"Abgr32" => new Image<Abgr32>(width, height, new Abgr32(r, g, b, 255)),
"Argb32" => new Image<Argb32>(width, height, new Argb32(r, g, b, 255)),
_ => throw new ArgumentOutOfRangeException(nameof(format), format, "Unsupported pixel format")
};

var bitmap = (AnyBitmap)image;
_ = bitmap.Width; // materialise so the original pixel format reaches the writer
return bitmap;
}

private static AnyBitmap CreateSolidBitmap(int width, int height, Rgb24 color, int dpi)
{
return MakeBitmapWithResolution(width, height, color, dpi, PixelResolutionUnit.PixelsPerInch);
}

private static AnyBitmap MakeBitmapWithResolution(int width, int height, Rgb24 color,
double resolution, PixelResolutionUnit unit)
{
var image = new SixLabors.ImageSharp.Image<Rgb24>(width, height, color);
image.Metadata.HorizontalResolution = resolution;
image.Metadata.VerticalResolution = resolution;
image.Metadata.ResolutionUnits = unit;
return image;
}

private static List<(int Width, int Height, float XResolution, ResUnit ResolutionUnit)> ReadTiffDirectories(byte[] tiffData)
{
var result = new List<(int, int, float, ResUnit)>();
using var ms = new MemoryStream(tiffData);
using var tiff = Tiff.ClientOpen("in-memory", "r", ms, new TiffStream());
tiff.Should().NotBeNull("the produced bytes should be a valid TIFF");

short directoryCount = tiff.NumberOfDirectories();
for (short i = 0; i < directoryCount; i++)
{
tiff.SetDirectory(i);
int width = tiff.GetField(TiffTag.IMAGEWIDTH)[0].ToInt();
int height = tiff.GetField(TiffTag.IMAGELENGTH)[0].ToInt();
FieldValue[] xres = tiff.GetField(TiffTag.XRESOLUTION);
FieldValue[] unit = tiff.GetField(TiffTag.RESOLUTIONUNIT);
result.Add((
width,
height,
xres != null ? xres[0].ToFloat() : 0f,
unit != null ? (ResUnit)unit[0].ToInt() : ResUnit.NONE));
}

return result;
}

/// <summary>
/// Normalises a TIFF page's resolution back to dots-per-inch, regardless of the
/// unit the value was stored in.
/// </summary>
private static double ToDotsPerInch(float xResolution, ResUnit unit)
{
return unit switch
{
ResUnit.INCH => xResolution,
ResUnit.CENTIMETER => xResolution * 2.54,
_ => xResolution
};
}

[FactWithAutomaticDisplayName]
public void Create_Multi_page_Gif()
{
Expand Down
101 changes: 94 additions & 7 deletions IronSoftware.Drawing/IronSoftware.Drawing.Common/AnyBitmap.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1073,6 +1073,64 @@ public static AnyBitmap CreateMultiFrameTiff(IEnumerable<AnyBitmap> images)
return FromStream(stream);
}

/// <summary>
/// Combines multiple images into a multi-page TIFF and returns the raw TIFF
/// as a <see cref="MemoryStream"/>.
/// <para>Unlike <see cref="CreateMultiFrameTiff(IEnumerable{AnyBitmap})"/>, each
/// page keeps its own dimensions, orientation and resolution. Pages are not
/// scaled to a common size. This makes it suitable for mixed-orientation
/// documents (for example portrait + landscape pages).</para>
/// <para>The result is the encoded TIFF itself, not an <see cref="AnyBitmap"/>,
/// so it avoids the lossy round-trip through a single ImageSharp image (which
/// cannot represent frames of differing sizes).</para>
/// </summary>
/// <param name="images">Images to combine, one per TIFF page.</param>
/// <returns>A <see cref="MemoryStream"/> positioned at 0 containing the multi-page TIFF.</returns>
public static MemoryStream CreateMultiFrameTiffStream(IEnumerable<AnyBitmap> images)
{
if (images == null)
throw new ArgumentNullException(nameof(images));

var frames = new List<Image>();

@mee-ironsoftware mee-ironsoftware Jun 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Low: Nice touch forcing every frame through CloneAs<Rgba32> so the writer's default (correct-channel-order) branch is always taken — a clean correctness firewall. One memory note: the clones are eagerly materialised into this List<Image> before any write, so for a moment both the caller's pages and the full set of Rgba32 clones are alive (transient ~2x page memory). Fine for typical use and no worse than the old IronPdf path, but for very large / high-DPI documents a lazy Select(... CloneAs<Rgba32> ...) with per-frame dispose after WriteDirectory would lower the peak. Not blocking.

@Sawraz-IS Sawraz-IS Jun 11, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The suggested lazy + per-frame-dispose can't be done safely in InternalSaveAsMultiPageTiff because it's shared with CreateMultiFrameTiff(IEnumerable<AnyBitmap>) / (IEnumerable<string>), which pass borrowed images (the (Image)anyBitmap cast returns the AnyBitmap's internal frame), disposing those inside the writer would corrupt the source bitmaps. Intentionally left as-is.

try
{
foreach (var image in images)
{
if (image == null)
throw new ArgumentException("The image sequence contains a null element.", nameof(images));

frames.Add(((Image)image).CloneAs<Rgba32>());
}

if (frames.Count == 0)
throw new ArgumentException("No images provided to create multi-frame TIFF.", nameof(images));

var stream = new MemoryStream();
InternalSaveAsMultiPageTiff(frames, stream); // already resets stream.Position to 0
return stream;
}
finally
{
foreach (var frame in frames)
{
frame.Dispose();
}
}
}

/// <summary>
/// Combines multiple images into a multi-page TIFF and returns the raw TIFF bytes.
/// <para>Each page keeps its own dimensions, orientation and resolution. Pages are
/// not scaled to a common size, making it suitable for mixed-orientation documents.</para>
/// </summary>
/// <param name="images">Images to combine, one per TIFF page.</param>
/// <returns>A byte array containing the multi-page TIFF.</returns>
public static byte[] CreateMultiFrameTiffBytes(IEnumerable<AnyBitmap> images)
{
using var stream = CreateMultiFrameTiffStream(images);
return stream.ToArray();
}

/// <summary>
/// Creates a multi-frame GIF image from multiple AnyBitmaps.
/// <para>All images should have the same dimension.</para>
Expand Down Expand Up @@ -3351,19 +3409,48 @@ private static void InternalSaveAsMultiPageTiff(IEnumerable<Image> images, Strea
switch (image)
{
case Image<Rgb24> imageAsFormat:
imageAsFormat.CopyPixelDataTo(buffer);
{
// Rgb24 is 3 bytes/pixel, but the buffer/stride above and the
// TIFF tags below are 4 samples/pixel (RGBA). Copying the 3-bpp
// data directly would leave each row short by 'width' bytes,
// shifting every subsequent row and corrupting the page. Convert
// to Rgba32 so the bytes line up with the 4-bpp layout.
using var rgba = imageAsFormat.CloneAs<Rgba32>();
rgba.CopyPixelDataTo(buffer);
}
break;
case Image<Abgr32> imageAsFormat:
imageAsFormat.CopyPixelDataTo(buffer);
{
// 4 bytes/pixel, but the bytes are A,B,G,R. The wrong sample
// order for PHOTOMETRIC.RGB (which expects R,G,B,A). Convert to
// Rgba32 so the channels are not written swapped.
using var rgba = imageAsFormat.CloneAs<Rgba32>();
rgba.CopyPixelDataTo(buffer);
}
break;
case Image<Argb32> imageAsFormat:
imageAsFormat.CopyPixelDataTo(buffer);
{
// Bytes are A,R,G,B; convert to Rgba32 for correct channel order.
using var rgba = imageAsFormat.CloneAs<Rgba32>();
rgba.CopyPixelDataTo(buffer);
}
break;
case Image<Bgr24> imageAsFormat:
imageAsFormat.CopyPixelDataTo(buffer);
{
// Bgr24 is likewise 3 bytes/pixel; convert to Rgba32 for the
// same reason as Rgb24 above (this also yields the correct
// R,G,B sample order under PHOTOMETRIC.RGB).
using var rgba = imageAsFormat.CloneAs<Rgba32>();

@mee-ironsoftware mee-ironsoftware Jun 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Low/Nit: Good fix here (and on the Rgb24 case) — the 3-bpp source packed into the 4-bpp buffer was misaligning every scanline, and CloneAs<Rgba32> also gives the correct R,G,B sample order. Out-of-scope follow-up: the sibling Abgr32 / Argb32 / Bgra32 cases just below still CopyPixelDataTo directly, but the writer emits PHOTOMETRIC.RGB with sample order R,G,B,A — so e.g. Bgra32 (bytes B,G,R,A) would land channel-swapped if ever hit. Pre-existing and the new public API avoids it entirely (it CloneAs<Rgba32> upfront), but worth a ticket to normalise these or confirm they're unreachable.

@Sawraz-IS Sawraz-IS Jun 11, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Normalized all three to CloneAs<Rgba32>() (matching the Rgb24/Bgr24 pattern), so the writer always gets correct R,G,B,A order under PHOTOMETRIC.RGB.

rgba.CopyPixelDataTo(buffer);
}
break;
case Image<Bgra32> imageAsFormat:
imageAsFormat.CopyPixelDataTo(buffer);
{
// Bytes are B,G,R,A; convert to Rgba32 so they are not written
// channel-swapped under PHOTOMETRIC.RGB.
using var rgba = imageAsFormat.CloneAs<Rgba32>();
rgba.CopyPixelDataTo(buffer);
}
break;
default:
(image as Image<Rgba32>).CopyPixelDataTo(buffer);
Expand Down Expand Up @@ -3393,8 +3480,8 @@ private static void InternalSaveAsMultiPageTiff(IEnumerable<Image> images, Strea
output.SetField(TiffTag.RESOLUTIONUNIT, ResUnit.CENTIMETER);
break;
case SixLabors.ImageSharp.Metadata.PixelResolutionUnit.PixelsPerMeter:
output.SetField(TiffTag.XRESOLUTION, image.Metadata.HorizontalResolution * 100);
output.SetField(TiffTag.YRESOLUTION, image.Metadata.VerticalResolution * 100);
output.SetField(TiffTag.XRESOLUTION, image.Metadata.HorizontalResolution / 100);

@mee-ironsoftware mee-ironsoftware Jun 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Medium: This / 100 fix is correct (TIFF has no per-metre unit; with RESOLUTIONUNIT = CENTIMETER, px/m / 100 = px/cm — the old * 100 wrote a value ~10^4x too large). But no test actually exercises this branch: CreateMultiFrameTiffBytes_Preserves_Per_Page_Dimensions_And_Resolution builds its bitmaps with PixelsPerInch, so it runs the unchanged Inch branch above. The PR description's claim that that test "proves the unit-conversion fix" isn't accurate, and the fix has no regression guard. Please add a case that sets PixelsPerMeter on the source and asserts the read-back DPI via the existing ToDotsPerInch helper — that test would fail under the old * 100.

@Sawraz-IS Sawraz-IS Jun 11, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Added CreateMultiFrameTiffBytes_PreservesResolution_FromPixelsPerMeterSource, which builds a source with PixelResolutionUnit.PixelsPerMeter (300 DPI expressed as px/m), runs it through CreateMultiFrameTiffBytes, and asserts the read-back resolution via ToDotsPerInch ≈ 300. I verified it fails under the old * 100 (got 2999994).

output.SetField(TiffTag.YRESOLUTION, image.Metadata.VerticalResolution / 100);
output.SetField(TiffTag.RESOLUTIONUNIT, ResUnit.CENTIMETER);
break;
}
Expand Down
Loading