Complete Rewrite. #52
Replies: 4 comments 1 reply
-
|
Here's a draft design doc which covers my implementation intentions. ImageSharp.Textures Complete RewriteStatusDraft design document. PurposeImageSharp.Textures should be rebuilt as a texture library on top of ImageSharp v4, not maintained as a narrow decoder/export layer over the existing architecture. The current library can decode DDS, KTX, and KTX2 files into ImageSharp images, but the resulting images are detached from the texture data. Consumers cannot edit a mip level, write changes back to the texture, and save the result. The public model also forces consumers to cast from The rewrite should define texture concepts first, then provide ImageSharp interop over that model. ImageSharp v4 FoundationThe rewrite should target ImageSharp v4 from the start. Relevant ImageSharp v4 concepts:
ImageSharp v4 gives us the memory, pixel, metadata, and processing foundation. It does not provide the texture resource model itself. Core VocabularyThe rewrite needs precise texture terminology.
The texture format contract should mirror ImageSharp's /// <summary>
/// Defines the contract for a texture format.
/// </summary>
public interface ITextureFormat
{
/// <summary>
/// Gets the name that describes this texture format.
/// </summary>
public string Name { get; }
/// <summary>
/// Gets the default MIME type used by this texture format.
/// </summary>
public string DefaultMimeType { get; }
/// <summary>
/// Gets all MIME types used by this texture format.
/// </summary>
public IEnumerable<string> MimeTypes { get; }
/// <summary>
/// Gets the file extensions commonly used by this texture format.
/// </summary>
public IEnumerable<string> FileExtensions { get; }
}
/// <summary>
/// Defines the contract for a texture format containing metadata.
/// </summary>
/// <typeparam name="TFormatMetadata">The type of format metadata.</typeparam>
public interface ITextureFormat<out TFormatMetadata> : ITextureFormat
where TFormatMetadata : class
{
/// <summary>
/// Creates a default instance of the format metadata.
/// </summary>
/// <returns>The format metadata.</returns>
public TFormatMetadata CreateDefaultFormatMetadata();
}Texel type metadata describes how a
The useful texture equivalent of ImageSharp's Examples: Texture metadata should mirror ImageSharp's split between owning metadata, format-specific metadata, and connecting metadata: /// <summary>
/// Stores metadata for a texture resource.
/// </summary>
public sealed class TextureMetadata : IDeepCloneable<TextureMetadata>
{
/// <summary>
/// Gets the original texture format, if the texture was decoded from a file.
/// </summary>
public ITextureFormat? DecodedTextureFormat { get; internal set; }
/// <summary>
/// Gets the metadata value associated with the specified texture format.
/// </summary>
/// <typeparam name="TFormatMetadata">The type of format metadata.</typeparam>
/// <param name="key">The texture format.</param>
/// <returns>The format metadata.</returns>
public TFormatMetadata GetFormatMetadata<TFormatMetadata>(ITextureFormat<TFormatMetadata> key)
where TFormatMetadata : class, ITextureFormatMetadata<TFormatMetadata>;
/// <summary>
/// Creates a new instance of the metadata value associated with the specified texture format.
/// </summary>
/// <typeparam name="TFormatMetadata">The type of format metadata.</typeparam>
/// <param name="key">The texture format.</param>
/// <returns>The cloned format metadata.</returns>
public TFormatMetadata CloneFormatMetadata<TFormatMetadata>(ITextureFormat<TFormatMetadata> key)
where TFormatMetadata : class, ITextureFormatMetadata<TFormatMetadata>;
/// <summary>
/// Creates a deep clone of the texture metadata.
/// </summary>
/// <returns>The cloned texture metadata.</returns>
public TextureMetadata DeepClone();
}
/// <summary>
/// Stores metadata used to convert between texture formats.
/// </summary>
public sealed class TextureConnectingMetadata
{
/// <summary>
/// Gets information about the encoded texel type.
/// </summary>
public TexelTypeInfo TexelTypeInfo { get; init; }
}
It should contain only texture-wide information. Its texel type describes the encoded storage layout used by every surface in the texture, but it does not define separate per-surface layout and it does not perform pixel conversion.
For a normal DDS texture, the DDS metadata converts the DdsTextureMetadata ddsMetadata = texture.Metadata.GetFormatMetadata(DdsFormat.Instance);
TextureConnectingMetadata connectingMetadata = ddsMetadata.ToTextureConnectingMetadata();
Ktx2TextureMetadata ktx2Metadata = Ktx2TextureMetadata.FromTextureConnectingMetadata(connectingMetadata);/// <summary>
/// Describes the stored texel components of a texture surface.
/// </summary>
public readonly struct TexelTypeInfo
{
/// <summary>
/// Gets the color depth, in bits per texel.
/// </summary>
public int BitsPerTexel { get; init; }
/// <summary>
/// Gets the color type represented by the texel type.
/// </summary>
public TexelColorType ColorType { get; init; }
/// <summary>
/// Gets the component information for the texel type.
/// </summary>
public TexelComponentInfo ComponentInfo { get; init; }
/// <summary>
/// Gets the alpha transparency behavior.
/// </summary>
public TexelAlphaRepresentation AlphaRepresentation { get; init; }
}
/// <summary>
/// Describes the components that make up a texel type.
/// </summary>
public readonly struct TexelComponentInfo
{
/// <summary>
/// Gets the number of components in the texel type.
/// </summary>
public int ComponentCount { get; }
/// <summary>
/// Gets the number of padding bits in the texel type.
/// </summary>
public int Padding { get; }
/// <summary>
/// Gets the component information at the specified index.
/// </summary>
/// <param name="componentIndex">The component index.</param>
/// <returns>The component precision in bits.</returns>
public int GetComponentPrecision(int componentIndex);
/// <summary>
/// Gets the numeric interpretation used by the stored component value at the specified index.
/// </summary>
/// <param name="componentIndex">The component index.</param>
/// <returns>The component interpretation.</returns>
public TexelComponentInterpretation GetComponentInterpretation(int componentIndex);
}
/// <summary>
/// Represents the color type and component order of a texel type.
/// </summary>
[Flags]
public enum TexelColorType
{
/// <summary>
/// No color type.
/// </summary>
None = 0,
/// <summary>
/// Represents the red component.
/// </summary>
Red = 1 << 0,
/// <summary>
/// Represents the green component.
/// </summary>
Green = 1 << 1,
/// <summary>
/// Represents the blue component.
/// </summary>
Blue = 1 << 2,
/// <summary>
/// Represents the alpha component.
/// </summary>
Alpha = 1 << 3,
/// <summary>
/// Represents the exponent component used by shared-exponent formats.
/// </summary>
Exponent = 1 << 4,
/// <summary>
/// Represents luminance data.
/// </summary>
Luminance = 1 << 5,
/// <summary>
/// Represents the chrominance blue component.
/// </summary>
ChrominanceBlue = 1 << 6,
/// <summary>
/// Represents the chrominance red component.
/// </summary>
ChrominanceRed = 1 << 7,
/// <summary>
/// Represents depth data.
/// </summary>
Depth = 1 << 8,
/// <summary>
/// Represents stencil data.
/// </summary>
Stencil = 1 << 9,
/// <summary>
/// Indicates that the color type is RGB.
/// </summary>
RGB = Red | Green | Blue | (1 << 10),
/// <summary>
/// Indicates that the color type is BGR.
/// </summary>
BGR = Blue | Green | Red | (1 << 11),
/// <summary>
/// Indicates that the color type is YCbCr.
/// </summary>
YCbCr = Luminance | ChrominanceBlue | ChrominanceRed | (1 << 12),
/// <summary>
/// Indicates that the color type is depth-stencil.
/// </summary>
DepthStencil = Depth | Stencil,
/// <summary>
/// Indicates that the color type is not represented by another value.
/// </summary>
Other = 1 << 13
}
/// <summary>
/// Represents how component values are interpreted in a texel type.
/// </summary>
public enum TexelComponentInterpretation
{
/// <summary>
/// Unsigned normalized integer.
/// </summary>
UNorm,
/// <summary>
/// Signed normalized integer.
/// </summary>
SNorm,
/// <summary>
/// Unsigned integer.
/// </summary>
UInt,
/// <summary>
/// Signed integer.
/// </summary>
SInt,
/// <summary>
/// Floating point.
/// </summary>
Float,
/// <summary>
/// sRGB-encoded unsigned normalized integer.
/// </summary>
SRgbUNorm,
/// <summary>
/// Shared exponent floating-point representation.
/// </summary>
SharedExponent,
/// <summary>
/// Type is intentionally unspecified by the container format.
/// </summary>
Typeless
}
/// <summary>
/// Represents the transparency behavior of a texel type.
/// </summary>
public enum TexelAlphaRepresentation
{
/// <summary>
/// The texel type does not contain alpha.
/// </summary>
None,
/// <summary>
/// Color components are stored after being multiplied by alpha.
/// </summary>
Associated,
/// <summary>
/// Color components are stored independently from alpha.
/// </summary>
Unassociated,
/// <summary>
/// Alpha behavior is not specified by the texture metadata.
/// </summary>
Unknown
}This information exists to convert between texture formats. A DDS Container-specific metadata types should convert to and from the shared metadata: /// <summary>
/// Provides metadata for a specific texture format.
/// </summary>
public interface ITextureFormatMetadata : IDeepCloneable
{
/// <summary>
/// Converts the metadata to a <see cref="TextureConnectingMetadata"/> instance.
/// </summary>
/// <returns>The connecting texture metadata.</returns>
public TextureConnectingMetadata ToTextureConnectingMetadata();
}
/// <summary>
/// Provides metadata for a specific texture format.
/// </summary>
/// <typeparam name="TSelf">The metadata type implementing this interface.</typeparam>
public interface ITextureFormatMetadata<TSelf> : ITextureFormatMetadata, IDeepCloneable<TSelf>
where TSelf : class, ITextureFormatMetadata
{
/// <summary>
/// Creates a new instance of the <typeparamref name="TSelf"/> class from the given connecting metadata.
/// </summary>
/// <param name="metadata">The connecting texture metadata.</param>
/// <returns>The format metadata.</returns>
public static abstract TSelf FromTextureConnectingMetadata(TextureConnectingMetadata metadata);
}
/// <summary>
/// Stores DDS-specific texture metadata.
/// </summary>
public sealed class DdsTextureMetadata : ITextureFormatMetadata<DdsTextureMetadata>
{
/// <summary>
/// Converts the DDS metadata to connecting texture metadata.
/// </summary>
/// <returns>The connecting texture metadata.</returns>
public TextureConnectingMetadata ToTextureConnectingMetadata();
/// <summary>
/// Creates DDS metadata from connecting texture metadata.
/// </summary>
/// <param name="metadata">The connecting texture metadata.</param>
/// <returns>The DDS metadata.</returns>
public static DdsTextureMetadata FromTextureConnectingMetadata(TextureConnectingMetadata metadata);
}KTX and KTX2 metadata should follow the same pattern. The point is not that DDS/KTX/KTX2 metadata disappear. The point is that each container has a clear conversion path into the connecting metadata model, so saving to a different container does not require every encoder to understand every other container's headers. Public ModelThe primary public object should be a single using Texture texture = Texture.Load("asset.dds");
TextureSurface surface = texture.MipLevels[0].Surfaces[0];
using Image<Rgba32> image = surface.ToImage<Rgba32>();The texture should expose the layout it actually contains. Ordinal position should come from ordered collections, following the same pattern as ImageSharp frames. A mip level does not need to store its own mip index, and a depth slice does not need to store its own slice index. The index is the position in the relevant collection. using Texture texture = Texture.Load("asset.dds");
foreach (TextureMipLevel mipLevel in texture.MipLevels)
{
foreach (TextureSurface surface in mipLevel.Surfaces)
{
using Image<Rgba32> image = surface.ToImage<Rgba32>();
}
}For a cubemap, TextureMipLevel baseLevel = texture.MipLevels[0];
foreach (TextureSurface face in baseLevel.Surfaces)
{
using Image<Rgba32> image = face.ToImage<Rgba32>();
}For a 2D array, TextureMipLevel baseLevel = texture.MipLevels[0];
foreach (TextureSurface layer in baseLevel.Surfaces)
{
using Image<Rgba32> image = layer.ToImage<Rgba32>();
}For a 3D texture, TextureMipLevel baseLevel = texture.MipLevels[0];
TextureSurface slice3 = baseLevel.Surfaces[3];
/// <summary>
/// Represents a complete texture resource with its metadata, mip levels, surfaces, and encoded data.
/// </summary>
public sealed class Texture : IDisposable
{
/// <summary>
/// Loads a texture from the specified path.
/// </summary>
/// <param name="path">The source path.</param>
/// <returns>The decoded texture.</returns>
public static Texture Load(string path);
/// <summary>
/// Loads a texture from the specified path.
/// </summary>
/// <param name="path">The source path.</param>
/// <param name="decoder">The decoder to use.</param>
/// <returns>The decoded texture.</returns>
public static Texture Load(string path, ITextureDecoder decoder);
/// <summary>
/// Loads a texture from the specified stream.
/// </summary>
/// <param name="stream">The source stream.</param>
/// <returns>The decoded texture.</returns>
public static Texture Load(Stream stream);
/// <summary>
/// Loads a texture from the specified stream.
/// </summary>
/// <param name="stream">The source stream.</param>
/// <param name="decoder">The decoder to use.</param>
/// <returns>The decoded texture.</returns>
public static Texture Load(Stream stream, ITextureDecoder decoder);
/// <summary>
/// Loads a texture from the specified path.
/// </summary>
/// <param name="path">The source path.</param>
/// <param name="cancellationToken">The token to monitor for cancellation requests.</param>
/// <returns>The decoded texture.</returns>
public static Task<Texture> LoadAsync(string path, CancellationToken cancellationToken = default);
/// <summary>
/// Loads a texture from the specified path.
/// </summary>
/// <param name="path">The source path.</param>
/// <param name="decoder">The decoder to use.</param>
/// <param name="cancellationToken">The token to monitor for cancellation requests.</param>
/// <returns>The decoded texture.</returns>
public static Task<Texture> LoadAsync(
string path,
ITextureDecoder decoder,
CancellationToken cancellationToken = default);
/// <summary>
/// Loads a texture from the specified stream.
/// </summary>
/// <param name="stream">The source stream.</param>
/// <param name="cancellationToken">The token to monitor for cancellation requests.</param>
/// <returns>The decoded texture.</returns>
public static Task<Texture> LoadAsync(Stream stream, CancellationToken cancellationToken = default);
/// <summary>
/// Loads a texture from the specified stream.
/// </summary>
/// <param name="stream">The source stream.</param>
/// <param name="decoder">The decoder to use.</param>
/// <param name="cancellationToken">The token to monitor for cancellation requests.</param>
/// <returns>The decoded texture.</returns>
public static Task<Texture> LoadAsync(
Stream stream,
ITextureDecoder decoder,
CancellationToken cancellationToken = default);
/// <summary>
/// Gets the texture metadata.
/// </summary>
public TextureMetadata Metadata { get; }
/// <summary>
/// Gets the ordered mip levels in the texture.
/// </summary>
public TextureMipLevelCollection MipLevels { get; }
/// <summary>
/// Saves the current texture resource to the specified path.
/// </summary>
/// <param name="path">The destination path.</param>
public void Save(string path);
/// <summary>
/// Saves the current texture resource to the specified path.
/// </summary>
/// <param name="path">The destination path.</param>
/// <param name="encoder">The encoder to use.</param>
public void Save(string path, ITextureEncoder encoder);
/// <summary>
/// Saves the current texture resource to the specified stream.
/// </summary>
/// <param name="stream">The destination stream.</param>
/// <param name="format">The texture format to use.</param>
public void Save(Stream stream, ITextureFormat format);
/// <summary>
/// Saves the current texture resource to the specified stream.
/// </summary>
/// <param name="stream">The destination stream.</param>
/// <param name="encoder">The encoder to use.</param>
public void Save(Stream stream, ITextureEncoder encoder);
/// <summary>
/// Saves the current texture resource to the specified path.
/// </summary>
/// <param name="path">The destination path.</param>
/// <param name="cancellationToken">The token to monitor for cancellation requests.</param>
/// <returns>A task representing the asynchronous save operation.</returns>
public Task SaveAsync(string path, CancellationToken cancellationToken = default);
/// <summary>
/// Saves the current texture resource to the specified path.
/// </summary>
/// <param name="path">The destination path.</param>
/// <param name="encoder">The encoder to use.</param>
/// <param name="cancellationToken">The token to monitor for cancellation requests.</param>
/// <returns>A task representing the asynchronous save operation.</returns>
public Task SaveAsync(
string path,
ITextureEncoder encoder,
CancellationToken cancellationToken = default);
/// <summary>
/// Saves the current texture resource to the specified stream.
/// </summary>
/// <param name="stream">The destination stream.</param>
/// <param name="format">The texture format to use.</param>
/// <param name="cancellationToken">The token to monitor for cancellation requests.</param>
/// <returns>A task representing the asynchronous save operation.</returns>
public Task SaveAsync(
Stream stream,
ITextureFormat format,
CancellationToken cancellationToken = default);
/// <summary>
/// Saves the current texture resource to the specified stream.
/// </summary>
/// <param name="stream">The destination stream.</param>
/// <param name="encoder">The encoder to use.</param>
/// <param name="cancellationToken">The token to monitor for cancellation requests.</param>
/// <returns>A task representing the asynchronous save operation.</returns>
public Task SaveAsync(
Stream stream,
ITextureEncoder encoder,
CancellationToken cancellationToken = default);
/// <summary>
/// Releases resources owned by the texture.
/// </summary>
public void Dispose();
}
/// <summary>
/// Represents the surfaces stored at one mip level of a texture.
/// </summary>
public sealed class TextureMipLevel : IDisposable
{
/// <summary>
/// Gets the width of surfaces at this mip level.
/// </summary>
public int Width { get; }
/// <summary>
/// Gets the height of surfaces at this mip level.
/// </summary>
public int Height { get; }
/// <summary>
/// Gets the ordered two-dimensional surfaces stored at this mip level.
/// </summary>
public TextureSurfaceCollection Surfaces { get; }
/// <summary>
/// Releases resources owned by the mip level.
/// </summary>
public void Dispose();
}This removes shape guessing from the API. The loader parses the container once and builds the mip-level/surface graph that actually exists. A mip level has one ordered surface collection. For a simple 2D texture, the collection contains one surface. For a 2D array, it contains the array layers. For a cubemap, it contains the six faces. For a 3D texture, it contains the depth slices for that mip level.
/// <summary>
/// Represents one editable two-dimensional texture surface.
/// </summary>
public sealed class TextureSurface : IDisposable
{
/// <summary>
/// Gets the surface width.
/// </summary>
public int Width { get; }
/// <summary>
/// Gets the surface height.
/// </summary>
public int Height { get; }
/// <summary>
/// Converts the surface to an ImageSharp image using the requested pixel type.
/// </summary>
/// <typeparam name="TPixel">The ImageSharp pixel type to use for the returned image.</typeparam>
/// <returns>An ImageSharp image containing a copy of the surface pixels.</returns>
public Image<TPixel> ToImage<TPixel>()
where TPixel : unmanaged, IPixel<TPixel>;
/// <summary>
/// Replaces the surface pixels with pixels from the specified ImageSharp image.
/// </summary>
/// <typeparam name="TPixel">The ImageSharp pixel type used by the source image.</typeparam>
/// <param name="image">The image containing the replacement pixels.</param>
/// <exception cref="ArgumentException">
/// The image dimensions do not match the texture surface dimensions.
/// </exception>
public void CopyFrom<TPixel>(Image<TPixel> image)
where TPixel : unmanaged, IPixel<TPixel>;
/// <summary>
/// Releases resources owned by the surface.
/// </summary>
public void Dispose();
}
Surface and mip dimensions are established when the surface or mip level is created. Resizing should create or import a new surface through the owning collection instead of mutating
/// <summary>
/// Converts one texel type between encoded payload bytes and ImageSharp images.
/// </summary>
public abstract class TexelOperations
{
/// <summary>
/// Gets information about the texel type handled by this instance.
/// </summary>
/// <returns>The texel type information.</returns>
public abstract TexelTypeInfo GetTexelTypeInfo();
/// <summary>
/// Gets the encoded payload size for a surface with the specified dimensions.
/// </summary>
/// <param name="width">The surface width.</param>
/// <param name="height">The surface height.</param>
/// <returns>The encoded payload size, in bytes.</returns>
public abstract int GetEncodedByteCount(int width, int height);
/// <summary>
/// Decodes a texture payload into an ImageSharp image.
/// </summary>
/// <param name="source">The encoded texture payload.</param>
/// <param name="configuration">The ImageSharp configuration to use when allocating the decoded image.</param>
/// <param name="width">The decoded image width.</param>
/// <param name="height">The decoded image height.</param>
/// <returns>The decoded ImageSharp image.</returns>
public abstract Image<TPixel> Decode<TPixel>(
ReadOnlySpan<byte> source,
Configuration configuration,
int width,
int height)
where TPixel : unmanaged, IPixel<TPixel>;
/// <summary>
/// Encodes an ImageSharp image into a texture payload.
/// </summary>
/// <param name="source">The source ImageSharp image.</param>
/// <param name="destination">The encoded texture payload.</param>
public abstract void Encode<TPixel>(
Image<TPixel> source,
Span<byte> destination)
where TPixel : unmanaged, IPixel<TPixel>;
}The container decoder determines the texel type from the file and builds each The container encoder serializes the current texture surface data during save. Like ImageSharp encoders, it owns the target representation and drives conversion from the source resource into the requested output.
Layout edits should happen through owning collections. This keeps ordinal position as collection order while still letting callers add, replace, insert, and remove mip levels or surfaces. /// <summary>
/// Represents the ordered mip levels in a texture.
/// </summary>
public sealed class TextureMipLevelCollection : IDisposable, IEnumerable<TextureMipLevel>
{
/// <summary>
/// Gets the number of mip levels.
/// </summary>
public int Count { get; }
/// <summary>
/// Gets the base mip level.
/// </summary>
public TextureMipLevel BaseLevel { get; }
/// <summary>
/// Gets the mip level at the specified index.
/// </summary>
/// <param name="index">The mip level index.</param>
/// <returns>The mip level at the specified index.</returns>
public TextureMipLevel this[int index] { get; }
/// <summary>
/// Determines the index of a mip level in the collection.
/// </summary>
/// <param name="mipLevel">The mip level to locate.</param>
/// <returns>The index of the mip level if found; otherwise, -1.</returns>
public int IndexOf(TextureMipLevel mipLevel);
/// <summary>
/// Determines whether the collection contains the specified mip level.
/// </summary>
/// <param name="mipLevel">The mip level to locate.</param>
/// <returns><see langword="true"/> if the collection contains the mip level; otherwise, <see langword="false"/>.</returns>
public bool Contains(TextureMipLevel mipLevel);
/// <summary>
/// Clones and inserts a mip level at the specified index.
/// </summary>
/// <param name="index">The mip level index.</param>
/// <param name="source">The mip level to clone and insert.</param>
/// <returns>The inserted mip level.</returns>
public TextureMipLevel InsertMipLevel(int index, TextureMipLevel source);
/// <summary>
/// Clones and appends a mip level to the collection.
/// </summary>
/// <param name="source">The mip level to clone and append.</param>
/// <returns>The appended mip level.</returns>
public TextureMipLevel AddMipLevel(TextureMipLevel source);
/// <summary>
/// Removes the mip level at the specified index and releases its resources.
/// </summary>
/// <param name="index">The mip level index.</param>
public void RemoveMipLevel(int index);
/// <summary>
/// Moves a mip level from one index to another.
/// </summary>
/// <param name="sourceIndex">The source mip level index.</param>
/// <param name="destinationIndex">The destination mip level index.</param>
public void MoveMipLevel(int sourceIndex, int destinationIndex);
/// <summary>
/// Removes the mip level at the specified index and transfers ownership to the caller.
/// </summary>
/// <param name="index">The mip level index.</param>
/// <returns>The removed mip level.</returns>
public TextureMipLevel ExportMipLevel(int index);
/// <summary>
/// Creates a new mip level containing clones of the surfaces at the specified index.
/// </summary>
/// <param name="index">The mip level index.</param>
/// <returns>The cloned mip level.</returns>
public TextureMipLevel CloneMipLevel(int index);
/// <summary>
/// Releases resources owned by the collection.
/// </summary>
public void Dispose();
}
/// <summary>
/// Represents the ordered surfaces in a mip level.
/// </summary>
public sealed class TextureSurfaceCollection : IDisposable, IEnumerable<TextureSurface>
{
/// <summary>
/// Gets the number of surfaces.
/// </summary>
public int Count { get; }
/// <summary>
/// Gets the surface at the specified index.
/// </summary>
/// <param name="index">The surface index.</param>
/// <returns>The surface at the specified index.</returns>
public TextureSurface this[int index] { get; }
/// <summary>
/// Determines the index of a surface in the collection.
/// </summary>
/// <param name="surface">The surface to locate.</param>
/// <returns>The index of the surface if found; otherwise, -1.</returns>
public int IndexOf(TextureSurface surface);
/// <summary>
/// Determines whether the collection contains the specified surface.
/// </summary>
/// <param name="surface">The surface to locate.</param>
/// <returns><see langword="true"/> if the collection contains the surface; otherwise, <see langword="false"/>.</returns>
public bool Contains(TextureSurface surface);
/// <summary>
/// Clones and inserts a surface at the specified index.
/// </summary>
/// <param name="index">The surface index.</param>
/// <param name="source">The surface to clone and insert.</param>
/// <returns>The inserted surface.</returns>
/// <exception cref="ArgumentException">
/// The source surface dimensions do not match the mip level dimensions.
/// </exception>
public TextureSurface InsertSurface(int index, TextureSurface source);
/// <summary>
/// Clones and appends a surface to the collection.
/// </summary>
/// <param name="source">The surface to clone and append.</param>
/// <returns>The appended surface.</returns>
/// <exception cref="ArgumentException">
/// The source surface dimensions do not match the mip level dimensions.
/// </exception>
public TextureSurface AddSurface(TextureSurface source);
/// <summary>
/// Removes the surface at the specified index and releases its resources.
/// </summary>
/// <param name="index">The surface index.</param>
public void RemoveSurface(int index);
/// <summary>
/// Moves a surface from one index to another.
/// </summary>
/// <param name="sourceIndex">The source surface index.</param>
/// <param name="destinationIndex">The destination surface index.</param>
public void MoveSurface(int sourceIndex, int destinationIndex);
/// <summary>
/// Removes the surface at the specified index and transfers ownership to the caller.
/// </summary>
/// <param name="index">The surface index.</param>
/// <returns>The removed surface.</returns>
public TextureSurface ExportSurface(int index);
/// <summary>
/// Creates a clone of the surface at the specified index.
/// </summary>
/// <param name="index">The surface index.</param>
/// <returns>The cloned surface.</returns>
public TextureSurface CloneSurface(int index);
/// <summary>
/// Releases resources owned by the collection.
/// </summary>
public void Dispose();
}The editing API should be deliberately boring: TextureMipLevel mipLevel = texture.MipLevels[0];
// CopyFrom imports edited ImageSharp pixels into the texture surface.
mipLevel.Surfaces[2].CopyFrom(image);
// Layout edits use the owned collection API.
mipLevel.Surfaces.RemoveSurface(2);
mipLevel.Surfaces.InsertSurface(2, replacementSurface);Loading and SavingLoading should parse the container and produce a texture resource. Decoder selection can be inferred from the path or supplied explicitly: using Texture texture = Texture.Load("asset.ktx2");
using Texture explicitTexture = Texture.Load("asset.dds", new DdsTextureDecoder());Streams should be supported when the caller already owns I/O: using Texture texture = Texture.Load(stream, new Ktx2TextureDecoder());
using Texture asyncTexture = await Texture.LoadAsync(stream, new DdsTextureDecoder(), cancellationToken);Saving should encode the current texture resource into a container. Encoder selection can be inferred from the path or supplied explicitly: texture.Save("edited.dds");
texture.Save("edited.ktx2", new Ktx2TextureEncoder());Streams require an explicit target format or encoder because there is no path extension to inspect: texture.Save(stream, Ktx2Format.Instance);
await texture.SaveAsync(stream, new DdsTextureEncoder(), cancellationToken);The save path must be part of the core design. If the library exposes editable texture surfaces, the texture must retain enough state to encode those changes back to disk. LayeringThe rewrite should separate four responsibilities. Container CodecsContainer codecs parse and write file structures:
They should not decode BC, ASTC, YUV, or packed pixels themselves. Their job is to map file bytes to texture surfaces and metadata. Texture Resource ModelThe texture model owns the texture structure:
This is the center of the library. Texel OperationsTexel operations decode and encode texture surface data:
These operations should work with payload spans and bounded scratch buffers. They should not create detached Operation implementations can stream internally through the smallest vertical unit the texel type can convert independently. For uncompressed formats this is usually one image row. For packed formats it is the row span required by the packing pattern. For block-compressed formats it is usually one block row. For example, a 4x4 block-compressed operation can process ImageSharp InteropImageSharp interop should live at the boundary:
Interop should use ImageSharp v4 APIs:
ConfigurationTexture configuration should not pretend that DDS, KTX, and KTX2 are normal ImageSharp raster formats. ImageSharp v4 The rewrite should introduce texture-specific configuration: public sealed class TextureConfiguration
{
public Configuration ImageSharpConfiguration { get; }
public TextureFormatManager Formats { get; }
}The default texture configuration can use MetadataTexture metadata should preserve texture-domain information that ImageSharp metadata does not model:
Container decoders should populate ImageSharp Editing ModelEditing should be explicit. using Texture texture = Texture.Load("albedo.dds");
TextureSurface surface = texture.MipLevels.BaseLevel.Surfaces[0];
// ToImage exports a copy; mutating the image does not modify the texture.
using Image<Rgba32> image = surface.ToImage<Rgba32>();
image.Mutate(x => x.Grayscale());
texture.Save("unchanged.dds");
// CopyFrom imports the edited pixels into the texture surface data.
surface.CopyFrom(image);
// Save always invokes the encoder for the current texture state.
texture.Save("edited.dds");Each The public editing boundary is copy out with Encoding StrategySaving should follow the ImageSharp model. The texture remains the source resource, and the selected encoder writes the requested file representation. Texture format conversion is mandatory. Saving a texture to another supported container should translate metadata through If the output texel type differs from the texture's current texel type, the encoder performs that conversion through Saving always goes through the container encoder. The encoder serializes the current texture surface data into the target container. ASTC and Current Open PRsThe existing ASTC work should be treated as reference material for the rewrite, not merged into the current architecture. ASTC is a texel operation concern. KTX and KTX2 support for ASTC is a routing/container concern. Benchmarks, fuzzing, and reference comparisons are useful, but they should attach to the rewritten operation boundaries. Apple KTX support should wait until the new container and codec split exists. It combines an Apple-specific container, LZFSE supercompression, and ASTC payload decoding, so adding it before the architecture is rebuilt would make the current design harder to recover. Initial Milestones
Implementation ScopeThe rewrite should prove the design with a narrow, complete vertical slice:
Design Principles
|
Beta Was this translation helpful? Give feedback.
-
|
Is there crossover with other container image types? not just textures, that this could possibly merge with in terms of api surface? (though I do note that might go against the texture domain terminology, unless that a public api thing rather than an internals thing). It feels like this shape would mesh nicely with Tiffs for example, and possibly ico's too. As they are image formats that support multiple 'pages'/'frames' with each one having the option of being sized independently of each other. It might just be this is a nice test bed for API dev that could back feed into tiffs, but it could also just be API sugar on top of the same common codec core infrastructure with a more of a texture flavour on this side. I also think exposing the |
Beta Was this translation helpful? Give feedback.
-
|
It's great to see the library getting more attention 😄 I think I largely agree on your analysis, but I am a bit skeptical about trying to flatten faces, arrays and slices into a Surfaces collection. That seems quite tricky to do without losing information or relying too much on implicit connections (e.g. the ordering in your cubemap example). There are also at least a couple of technical issues that could make life difficult
I agree with @tocsoft that being able to mutate in place would make the API more useable, as well as saving the Texture->Image->Texture round trip. On the other hand, the proposal does make a nice boundary and saves duplicating a lot ImageSharp.Image functionality which saves on effort. Also, would TextureMipLevelCollection enforce a mip pyramid, or do we not care if the user wants to insert an arbitrary sized image at mip level x? |
Beta Was this translation helpful? Give feedback.
-
|
Regarding ASTC, I agree that it's huge and difficult to review. I tried to break it down into sections that build on each other and would need rebasing at each stage, but there wasn't any good way to break it into nice independent reviewable pieces. What do you think of just #48 as a single pull request for ASTC decoding? That is the full backend implementation, but not tied in to KTX1/2. I can just update the description to reframe the PR. I would really really like to get the ASTC decoder merged in before everything is rewritten. This nicely self-contained piece (it's all in I can accept that the KTX1/2 implementations can wait for now. Obviously I'd like for ASTC to be usable, but that's much less of a stretch to add later on. |
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
-
I've been reviewing the codebase properly for the first time.
The API usability is currently far too narrow for it to be worth maintenance effort in its current shape. It requires a full rewrite, which I am going to make a start on.
The rewrite should target ImageSharp v4 from the start. There is little value in rebuilding this library on top of older ImageSharp assumptions when v4 gives us the foundation we actually want to design against. That means we can align the texture API with the current ImageSharp memory, pixel, metadata, and processing model instead of carrying forward compatibility decisions from this abandoned codebase.
This is not a criticism of the effort that has gone into the existing code or recent contributions. In particular, there is a large amount of ASTC work open across #37 and the split PRs #45-#51. That work clearly represents significant time and research. However, reviewing or merging it into the current architecture would compound the underlying problems rather than solve them.
The current PR stack is also not practically reviewable as-is. The original ASTC PR is hundreds of files, and the later split PRs still appear to be cumulative against
main, so each successive PR includes much of the previous work again. That gives us a sequence of increasingly large diffs rather than isolated review units. Even if the implementation is technically sound, I cannot responsibly review it while the library has no clear texture model, no coherent edit/save path, and no separation between container parsing, pixel decoding, and ImageSharp image materialization.There is also an open request for Apple KTX support (#39), which depends on an undocumented Apple container variant, LZFSE decompression, and ASTC payload decoding. That is exactly the kind of feature this library should eventually be able to support, but adding it on top of the current abstractions would make the design harder to recover.
The rewrite needs to start by defining the core texture model: container format, storage/pixel format, dimensions, mip levels, array layers, cube faces, depth slices, and planes. ImageSharp interop should be an adapter over that model, not the model itself. Low-level decoders should decode texture data; they should not be responsible for constructing detached
Imageinstances.Until that foundation exists, I do not think we should merge large feature work into the current design. Small fixes may still be worth considering, but new format support should either wait for the rewrite or be preserved as reference material to port into the new architecture.
CC @brianpopow @Erik-White
Beta Was this translation helpful? Give feedback.
All reactions