Fix definition and use of non-indexed sprite color channels.

Our SpriteFrameType names refer to the byte channel order rather than
the bit order, meaning that SpriteFrameType.BGRA corresponds to the
standard Color.ToArgb() etc byte order when the (little-endian) integer
is read as 4 individual bytes.

The previous code did not account for the fact that non-indexed Png
uses big-endian storage for its RGBA colours, and that SheetBuilder
had the color channels incorrectly swapped to match and cancel this out.

New SpriteFrameType enums are introduced to distinguish between BGRA
(little-endian) and RGBA (big-endian) formats, and also for 24bit data
without alpha. The channel swizzling / alpha creation is now handled
when copying into the texture atlas, removing the need for non-png
ISpriteLoader implementations to allocate an additional temporary array
and reorder the channels during load.

OpenRA.Game/FileFormats/Png.cs

@@ -17,6 +17,7 @@ using System.Net;
 using System.Text;
 using ICSharpCode.SharpZipLib.Checksum;
 using ICSharpCode.SharpZipLib.Zip.Compression.Streams;
+using OpenRA.Graphics;
 using OpenRA.Primitives;
 
 namespace OpenRA.FileFormats
@@ -25,12 +26,15 @@ namespace OpenRA.FileFormats
 	{
 		static readonly byte[] Signature = { 0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a };
 
-		public int Width { get; set; }
+		public int Width { get; private set; }
-		public int Height { get; set; }
+		public int Height { get; private set; }
-		public Color[] Palette { get; set; }
+		public Color[] Palette { get; private set; }
-		public byte[] Data { get; set; }
+		public byte[] Data { get; private set; }
+		public SpriteFrameType Type { get; private set; }
 		public Dictionary<string, string> EmbeddedData = new Dictionary<string, string>();
 
+		public int PixelStride { get { return Type == SpriteFrameType.Indexed ? 1 : Type == SpriteFrameType.RGB ? 3 : 4; } }
+
 		public Png(Stream s)
 		{
 			if (!Verify(s))
@@ -38,9 +42,8 @@ namespace OpenRA.FileFormats
 
 			s.Position += 8;
 			var headerParsed = false;
-			var isPaletted = false;
-			var is24Bit = false;
 			var data = new List<byte>();
+			Type = SpriteFrameType.RGBA;
 
 			while (true)
 			{
@@ -65,14 +68,12 @@ namespace OpenRA.FileFormats
 
 							var bitDepth = ms.ReadUInt8();
 							var colorType = (PngColorType)ms.ReadByte();
-							isPaletted = IsPaletted(bitDepth, colorType);
+							if (IsPaletted(bitDepth, colorType))
-							is24Bit = colorType == PngColorType.Color;
+								Type = SpriteFrameType.Indexed;
+							else if (colorType == PngColorType.Color)
+								Type = SpriteFrameType.RGB;
 
-							var dataLength = Width * Height;
+							Data = new byte[Width * Height * PixelStride];
-							if (!isPaletted)
-								dataLength *= 4;
-
-							Data = new byte[dataLength];
 
 							var compression = ms.ReadByte();
 							/*var filter = */ms.ReadByte();
@@ -133,39 +134,28 @@ namespace OpenRA.FileFormats
 							{
 								using (var ds = new InflaterInputStream(ns))
 								{
-									var pxStride = isPaletted ? 1 : is24Bit ? 3 : 4;
+									var pxStride = PixelStride;
-									var srcStride = Width * pxStride;
+									var rowStride = Width * pxStride;
-									var destStride = Width * (isPaletted ? 1 : 4);
 
-									var prevLine = new byte[srcStride];
+									var prevLine = new byte[rowStride];
 									for (var y = 0; y < Height; y++)
 									{
 										var filter = (PngFilter)ds.ReadByte();
-										var line = ds.ReadBytes(srcStride);
+										var line = ds.ReadBytes(rowStride);
 
-										for (var i = 0; i < srcStride; i++)
+										for (var i = 0; i < rowStride; i++)
 											line[i] = i < pxStride
 												? UnapplyFilter(filter, line[i], 0, prevLine[i], 0)
 												: UnapplyFilter(filter, line[i], line[i - pxStride], prevLine[i], prevLine[i - pxStride]);
 
-										if (is24Bit)
+										Array.Copy(line, 0, Data, y * rowStride, rowStride);
-										{
-											// Fold alpha channel into RGB data
-											for (var i = 0; i < line.Length / 3; i++)
-											{
-												Array.Copy(line, 3 * i, Data, y * destStride + 4 * i, 3);
-												Data[y * destStride + 4 * i + 3] = 255;
-											}
-										}
-										else
-											Array.Copy(line, 0, Data, y * destStride, line.Length);
 
 										prevLine = line;
 									}
 								}
 							}
 
-							if (isPaletted && Palette == null)
+							if (Type == SpriteFrameType.Indexed && Palette == null)
 								throw new InvalidDataException("Non-Palette indexed PNG are not supported.");
 
 							return;
@@ -175,7 +165,7 @@ namespace OpenRA.FileFormats
 			}
 		}
 
-		public Png(byte[] data, int width, int height, Color[] palette = null,
+		public Png(byte[] data, SpriteFrameType type, int width, int height, Color[] palette = null,
 			Dictionary<string, string> embeddedData = null)
 		{
 			var expectLength = width * height;
@@ -185,11 +175,46 @@ namespace OpenRA.FileFormats
 			if (data.Length != expectLength)
 				throw new InvalidDataException("Input data does not match expected length");
 
+			Type = type;
 			Width = width;
 			Height = height;
 
-			Palette = palette;
+			switch (type)
+			{
+				case SpriteFrameType.Indexed:
+				case SpriteFrameType.RGBA:
+				case SpriteFrameType.RGB:
+				{
+					// Data is already in a compatible format
 					Data = data;
+					if (type == SpriteFrameType.Indexed)
+						Palette = palette;
+
+					break;
+				}
+
+				case SpriteFrameType.BGRA:
+				case SpriteFrameType.BGR:
+				{
+					// Convert to big endian
+					Data = new byte[data.Length];
+					var stride = PixelStride;
+					for (var i = 0; i < width * height; i++)
+					{
+						Data[stride * i] = data[stride * i + 2];
+						Data[stride * i + 1] = data[stride * i + 1];
+						Data[stride * i + 2] = data[stride * i + 0];
+
+						if (type == SpriteFrameType.BGRA)
+							Data[stride * i + 3] = data[stride * i + 3];
+					}
+
+					break;
+				}
+
+				default:
+					throw new InvalidDataException("Unhandled SpriteFrameType {0}".F(type));
+			}
 
 			if (embeddedData != null)
 				EmbeddedData = embeddedData;
@@ -274,9 +299,8 @@ namespace OpenRA.FileFormats
 					header.Write(IPAddress.HostToNetworkOrder(Height));
 					header.WriteByte(8); // Bit depth
 
-					var colorType = Palette != null
+					var colorType = Type == SpriteFrameType.Indexed ? PngColorType.Indexed | PngColorType.Color :
-						? PngColorType.Indexed | PngColorType.Color
+						Type == SpriteFrameType.RGB ? PngColorType.Color : PngColorType.Color | PngColorType.Alpha;
-						: PngColorType.Color | PngColorType.Alpha;
 					header.WriteByte((byte)colorType);
 
 					header.WriteByte(0); // Compression
@@ -286,7 +310,7 @@ namespace OpenRA.FileFormats
 					WritePngChunk(output, "IHDR", header);
 				}
 
-				bool alphaPalette = false;
+				var alphaPalette = false;
 				if (Palette != null)
 				{
 					using (var palette = new MemoryStream())
@@ -318,12 +342,12 @@ namespace OpenRA.FileFormats
 				{
 					using (var compressed = new DeflaterOutputStream(data))
 					{
-						var stride = Width * (Palette != null ? 1 : 4);
+						var rowStride = Width * PixelStride;
 						for (var y = 0; y < Height; y++)
 						{
 							// Write uncompressed scanlines for simplicity
 							compressed.WriteByte(0);
-							compressed.Write(Data, y * stride, stride);
+							compressed.Write(Data, y * rowStride, rowStride);
 						}
 
 						compressed.Flush();

OpenRA.Game/Graphics/CursorManager.cs

@@ -69,9 +69,16 @@ namespace OpenRA.Graphics
 					// Hotspot is specified relative to the center of the frame
 					var hotspot = f.Offset.ToInt2() - kv.Value.Hotspot - new int2(f.Size) / 2;
 
-					// SheetBuilder expects data in BGRA
+					// Resolve indexed data to real colours
-					var data = FrameToBGRA(kv.Key, f, palette);
+					var data = f.Data;
-					c.Sprites[c.Length++] = sheetBuilder.Add(data, f.Size, 0, hotspot);
+					var type = f.Type;
+					if (type == SpriteFrameType.Indexed)
+					{
+						data = ConvertIndexedToBgra(kv.Key, f, palette);
+						type = SpriteFrameType.BGRA;
+					}
+
+					c.Sprites[c.Length++] = sheetBuilder.Add(data, type, f.Size, 0, hotspot);
 
 					// Bounds relative to the hotspot
 					c.Bounds = Rectangle.Union(c.Bounds, new Rectangle(hotspot, f.Size));
@@ -217,33 +224,27 @@ namespace OpenRA.Graphics
 			Update();
 		}
 
-		public static byte[] FrameToBGRA(string name, ISpriteFrame frame, ImmutablePalette palette)
+		public static byte[] ConvertIndexedToBgra(string name, ISpriteFrame frame, ImmutablePalette palette)
 		{
-			// Data is already in BGRA format
+			if (frame.Type != SpriteFrameType.Indexed)
-			if (frame.Type == SpriteFrameType.BGRA)
+				throw new ArgumentException("ConvertIndexedToBgra requires input frames to be indexed.", nameof(frame));
-				return frame.Data;
 
-			// Cursors may be either native BGRA or Indexed.
-			// Indexed sprites are converted to BGRA using the referenced palette.
 			// All palettes must be explicitly referenced, even if they are embedded in the sprite.
-			if (frame.Type == SpriteFrameType.Indexed && palette == null)
+			if (palette == null)
 				throw new InvalidOperationException("Cursor sequence `{0}` attempted to load an indexed sprite but does not define Palette".F(name));
 
 			var width = frame.Size.Width;
 			var height = frame.Size.Height;
 			var data = new byte[4 * width * height];
+			unsafe
+			{
+				// Cast the data to an int array so we can copy the src data directly
+				fixed (byte* bd = &data[0])
+				{
+					var rgba = (uint*)bd;
 					for (var j = 0; j < height; j++)
-			{
 						for (var i = 0; i < width; i++)
-				{
+							rgba[j * width + i] = palette[frame.Data[j * width + i]];
-					var rgba = palette[frame.Data[j * width + i]];
-					var k = 4 * (j * width + i);
-
-					// Convert RGBA to BGRA
-					data[k] = (byte)(rgba >> 16);
-					data[k + 1] = (byte)(rgba >> 8);
-					data[k + 2] = (byte)(rgba >> 0);
-					data[k + 3] = (byte)(rgba >> 24);
 				}
 			}
 

OpenRA.Game/Graphics/Sheet.cs

@@ -79,21 +79,17 @@ namespace OpenRA.Graphics
 
 		public Png AsPng()
 		{
-			var data = GetData();
+			if (Type == SheetType.Indexed)
+				throw new InvalidOperationException("AsPng() cannot be called on Indexed sheets.");
 
-			// Convert BGRA to RGBA
+			return new Png(GetData(), SpriteFrameType.BGRA, Size.Width, Size.Height);
-			for (var i = 0; i < Size.Width * Size.Height; i++)
-			{
-				var temp = data[i * 4];
-				data[i * 4] = data[i * 4 + 2];
-				data[i * 4 + 2] = temp;
-			}
-
-			return new Png(data, Size.Width, Size.Height);
 		}
 
 		public Png AsPng(TextureChannel channel, IPalette pal)
 		{
+			if (Type != SheetType.Indexed)
+				throw new InvalidOperationException("AsPng(TextureChannel, IPalette) can only be called on Indexed sheets.");
+
 			var d = GetData();
 			var plane = new byte[Size.Width * Size.Height];
 			var dataStride = 4 * Size.Width;
@@ -107,7 +103,7 @@ namespace OpenRA.Graphics
 			for (var i = 0; i < Palette.Size; i++)
 				palColors[i] = pal.GetColor(i);
 
-			return new Png(plane, Size.Width, Size.Height, palColors);
+			return new Png(plane, SpriteFrameType.BGRA, Size.Width, Size.Height, palColors);
 		}
 
 		public void CreateBuffer()

OpenRA.Game/Graphics/SheetBuilder.cs

@@ -52,8 +52,15 @@ namespace OpenRA.Graphics
 		{
 			switch (t)
 			{
-				case SpriteFrameType.Indexed: return SheetType.Indexed;
+				case SpriteFrameType.Indexed:
-				case SpriteFrameType.BGRA: return SheetType.BGRA;
+					return SheetType.Indexed;
+
+				// Util.FastCopyIntoChannel will automatically convert these to BGRA
+				case SpriteFrameType.BGRA:
+				case SpriteFrameType.BGR:
+				case SpriteFrameType.RGBA:
+				case SpriteFrameType.RGB:
+					return SheetType.BGRA;
 				default: throw new NotImplementedException("Unknown SpriteFrameType {0}".F(t));
 			}
 		}
@@ -74,16 +81,16 @@ namespace OpenRA.Graphics
 			this.margin = margin;
 		}
 
-		public Sprite Add(ISpriteFrame frame) { return Add(frame.Data, frame.Size, 0, frame.Offset); }
+		public Sprite Add(ISpriteFrame frame) { return Add(frame.Data, frame.Type, frame.Size, 0, frame.Offset); }
-		public Sprite Add(byte[] src, Size size) { return Add(src, size, 0, float3.Zero); }
+		public Sprite Add(byte[] src, SpriteFrameType type, Size size) { return Add(src, type, size, 0, float3.Zero); }
-		public Sprite Add(byte[] src, Size size, float zRamp, in float3 spriteOffset)
+		public Sprite Add(byte[] src, SpriteFrameType type, Size size, float zRamp, in float3 spriteOffset)
 		{
 			// Don't bother allocating empty sprites
 			if (size.Width == 0 || size.Height == 0)
 				return new Sprite(current, Rectangle.Empty, 0, spriteOffset, channel, BlendMode.Alpha);
 
 			var rect = Allocate(size, zRamp, spriteOffset);
-			Util.FastCopyIntoChannel(rect, src);
+			Util.FastCopyIntoChannel(rect, src, type);
 			current.CommitBufferedData();
 			return rect;
 		}
@@ -96,15 +103,6 @@ namespace OpenRA.Graphics
 			return rect;
 		}
 
-		public Sprite Add(Size size, byte paletteIndex)
-		{
-			var data = new byte[size.Width * size.Height];
-			for (var i = 0; i < data.Length; i++)
-				data[i] = paletteIndex;
-
-			return Add(data, size);
-		}
-
 		TextureChannel? NextChannel(TextureChannel t)
 		{
 			var nextChannel = (int)t + (int)Type;

OpenRA.Game/Graphics/SpriteLoader.cs

@@ -18,7 +18,29 @@ using OpenRA.Primitives;
 
 namespace OpenRA.Graphics
 {
-	public enum SpriteFrameType { Indexed, BGRA }
+	/// <summary>
+	/// Describes the format of the pixel data in a ISpriteFrame.
+	/// Note that the channel order is defined for little-endian bytes, so BGRA corresponds
+	/// to a 32bit ARGB value, such as that returned by Color.ToArgb()!
+	/// </summary>
+	public enum SpriteFrameType
+	{
+		// 8 bit index into an external palette
+		Indexed,
+
+		// 32 bit color such as returned by Color.ToArgb() or the bmp file format
+		// (remember that little-endian systems place the little bits in the first byte!)
+		BGRA,
+
+		// Like BGRA, but without an alpha channel
+		BGR,
+
+		// 32 bit color in big-endian format, like png
+		RGBA,
+
+		// Like RGBA, but without an alpha channel
+		RGB
+	}
 
 	public interface ISpriteLoader
 	{
@@ -47,7 +69,7 @@ namespace OpenRA.Graphics
 
 	public class SpriteCache
 	{
-		public readonly Cache<SpriteFrameType, SheetBuilder> SheetBuilders;
+		public readonly Cache<SheetType, SheetBuilder> SheetBuilders;
 		readonly ISpriteLoader[] loaders;
 		readonly IReadOnlyFileSystem fileSystem;
 
@@ -57,7 +79,7 @@ namespace OpenRA.Graphics
 
 		public SpriteCache(IReadOnlyFileSystem fileSystem, ISpriteLoader[] loaders)
 		{
-			SheetBuilders = new Cache<SpriteFrameType, SheetBuilder>(t => new SheetBuilder(SheetBuilder.FrameTypeToSheetType(t)));
+			SheetBuilders = new Cache<SheetType, SheetBuilder>(t => new SheetBuilder(t));
 
 			this.fileSystem = fileSystem;
 			this.loaders = loaders;
@@ -103,7 +125,7 @@ namespace OpenRA.Graphics
 					{
 						if (unloaded[i] != null)
 						{
-							sprite[i] = SheetBuilders[unloaded[i].Type].Add(unloaded[i]);
+							sprite[i] = SheetBuilders[SheetBuilder.FrameTypeToSheetType(unloaded[i].Type)].Add(unloaded[i]);
 							unloaded[i] = null;
 						}
 					}

OpenRA.Game/Graphics/Theater.cs

@@ -107,12 +107,13 @@ namespace OpenRA.Graphics
 							throw new YamlException("Sprite type mismatch. Terrain sprites must all be either Indexed or RGBA.");
 
 						var s = sheetBuilder.Allocate(f.Size, zRamp, offset);
-						Util.FastCopyIntoChannel(s, f.Data);
+						Util.FastCopyIntoChannel(s, f.Data, f.Type);
 
 						if (tileset.EnableDepth)
 						{
 							var ss = sheetBuilder.Allocate(f.Size, zRamp, offset);
-							Util.FastCopyIntoChannel(ss, allFrames[j + frameCount].Data);
+							var depthFrame = allFrames[j + frameCount];
+							Util.FastCopyIntoChannel(ss, depthFrame.Data, depthFrame.Type);
 
 							// s and ss are guaranteed to use the same sheet
 							// because of the custom terrain sheet allocation
@@ -136,7 +137,10 @@ namespace OpenRA.Graphics
 			}
 
 			// 1x1px transparent tile
-			missingTile = sheetBuilder.Add(new byte[sheetBuilder.Type == SheetType.BGRA ? 4 : 1], new Size(1, 1));
+			if (sheetBuilder.Type == SheetType.BGRA)
+				missingTile = sheetBuilder.Add(new byte[4], SpriteFrameType.BGRA, new Size(1, 1));
+			else
+				missingTile = sheetBuilder.Add(new byte[1], SpriteFrameType.Indexed, new Size(1, 1));
 
 			Sheet.ReleaseBuffer();
 		}

OpenRA.Game/Graphics/Util.cs

@@ -62,7 +62,7 @@ namespace OpenRA.Graphics
 			vertices[nv + 5] = new Vertex(a, r.Left, r.Top, sl, st, paletteTextureIndex, fAttribC, tint);
 		}
 
-		public static void FastCopyIntoChannel(Sprite dest, byte[] src)
+		public static void FastCopyIntoChannel(Sprite dest, byte[] src, SpriteFrameType srcType)
 		{
 			var destData = dest.Sheet.GetData();
 			var width = dest.Bounds.Width;
@@ -85,12 +85,34 @@ namespace OpenRA.Graphics
 						{
 							for (var i = 0; i < width; i++)
 							{
-								var r = src[k++];
+								byte r, g, b, a;
-								var g = src[k++];
+								switch (srcType)
-								var b = src[k++];
+								{
-								var a = src[k++];
+									case SpriteFrameType.BGRA:
-								var cc = Color.FromArgb(a, r, g, b);
+									case SpriteFrameType.BGR:
+									{
+										b = src[k++];
+										g = src[k++];
+										r = src[k++];
+										a = srcType == SpriteFrameType.BGRA ? src[k++] : (byte)255;
+										break;
+									}
 
+									case SpriteFrameType.RGBA:
+									case SpriteFrameType.RGB:
+									{
+										r = src[k++];
+										g = src[k++];
+										b = src[k++];
+										a = srcType == SpriteFrameType.RGBA ? src[k++] : (byte)255;
+										break;
+									}
+
+									default:
+										throw new InvalidOperationException("Unknown SpriteFrameType {0}".F(srcType));
+								}
+
+								var cc = Color.FromArgb(a, r, g, b);
 								data[(y + j) * destStride + x + i] = PremultiplyAlpha(cc).ToArgb();
 							}
 						}
@@ -139,16 +161,29 @@ namespace OpenRA.Graphics
 						for (var i = 0; i < width; i++)
 						{
 							Color cc;
-							if (src.Palette == null)
+							switch (src.Type)
+							{
+								case SpriteFrameType.Indexed:
+								{
+									cc = src.Palette[src.Data[k++]];
+									break;
+								}
+
+								case SpriteFrameType.RGBA:
+								case SpriteFrameType.RGB:
 								{
 									var r = src.Data[k++];
 									var g = src.Data[k++];
 									var b = src.Data[k++];
-								var a = src.Data[k++];
+									var a = src.Type == SpriteFrameType.RGBA ? src.Data[k++] : (byte)255;
 									cc = Color.FromArgb(a, r, g, b);
+									break;
+								}
+
+								// Pngs don't support BGR[A], so no need to include them here
+								default:
+									throw new InvalidOperationException("Unknown SpriteFrameType {0}".F(src.Type));
 							}
-							else
-								cc = src.Palette[src.Data[k++]];
 
 							data[(y + j) * destStride + x + i] = PremultiplyAlpha(cc).ToArgb();
 						}

OpenRA.Game/Map/Map.cs

@@ -17,6 +17,7 @@ using System.Reflection;
 using System.Text;
 using OpenRA.FileFormats;
 using OpenRA.FileSystem;
+using OpenRA.Graphics;
 using OpenRA.Primitives;
 using OpenRA.Support;
 using OpenRA.Traits;
@@ -778,7 +779,7 @@ namespace OpenRA
 				}
 			}
 
-			var png = new Png(minimapData, bitmapWidth, height);
+			var png = new Png(minimapData, SpriteFrameType.BGRA, bitmapWidth, height);
 			return png.Save();
 		}
 

OpenRA.Game/Renderer.cs

@@ -431,24 +431,15 @@ namespace OpenRA
 			var srcWidth = screenSprite.Sheet.Size.Width;
 			var destWidth = screenSprite.Bounds.Width;
 			var destHeight = -screenSprite.Bounds.Height;
-			var channelOrder = new[] { 2, 1, 0, 3 };
 
 			ThreadPool.QueueUserWorkItem(_ =>
 			{
-				// Convert BGRA to RGBA
+				// Extract the screen rect from the (larger) backing surface
 				var dest = new byte[4 * destWidth * destHeight];
 				for (var y = 0; y < destHeight; y++)
-				{
+					Array.Copy(src, 4 * y * srcWidth, dest, 4 * y * destWidth, 4 * destWidth);
-					for (var x = 0; x < destWidth; x++)
-					{
-						var destOffset = 4 * (y * destWidth + x);
-						var srcOffset = 4 * (y * srcWidth + x);
-						for (var i = 0; i < 4; i++)
-							dest[destOffset + i] = src[srcOffset + channelOrder[i]];
-					}
-				}
 
-				new Png(dest, destWidth, destHeight).Save(path);
+				new Png(dest, SpriteFrameType.BGRA, destWidth, destHeight).Save(path);
 			});
 		}
 

OpenRA.Mods.Cnc/Graphics/VoxelLoader.cs

@@ -77,8 +77,8 @@ namespace OpenRA.Mods.Cnc.Graphics
 			var size = new Size(su, sv);
 			var s = sheetBuilder.Allocate(size);
 			var t = sheetBuilder.Allocate(size);
-			OpenRA.Graphics.Util.FastCopyIntoChannel(s, colors);
+			OpenRA.Graphics.Util.FastCopyIntoChannel(s, colors, SpriteFrameType.Indexed);
-			OpenRA.Graphics.Util.FastCopyIntoChannel(t, normals);
+			OpenRA.Graphics.Util.FastCopyIntoChannel(t, normals, SpriteFrameType.Indexed);
 
 			// s and t are guaranteed to use the same sheet because
 			// of the custom voxel sheet allocation implementation

OpenRA.Mods.Cnc/UtilityCommands/ConvertPngToShpCommand.cs

@@ -14,6 +14,7 @@ using System.Collections.Generic;
 using System.IO;
 using System.Linq;
 using OpenRA.FileFormats;
+using OpenRA.Graphics;
 using OpenRA.Mods.Cnc.SpriteLoaders;
 using OpenRA.Primitives;
 
@@ -35,7 +36,7 @@ namespace OpenRA.Mods.Cnc.UtilityCommands
 			var dest = inputFiles[0].Split('-').First() + ".shp";
 
 			var frames = inputFiles.Select(a => new Png(File.OpenRead(a))).ToList();
-			if (frames.Any(f => f.Palette == null))
+			if (frames.Any(f => f.Type != SpriteFrameType.Indexed))
 				throw new InvalidOperationException("All frames must be paletted");
 
 			var size = new Size(frames[0].Width, frames[0].Height);

OpenRA.Mods.Common/SpriteLoaders/PngSheetLoader.cs

@@ -61,24 +61,22 @@ namespace OpenRA.Mods.Common.SpriteLoaders
 				RegionsFromSlices(png, out frameRegions, out frameOffsets);
 
 			frames = new ISpriteFrame[frameRegions.Count];
-
+			var stride = png.PixelStride;
 			for (var i = 0; i < frames.Length; i++)
 			{
 				var frameStart = frameRegions[i].X + frameRegions[i].Y * png.Width;
 				var frameSize = new Size(frameRegions[i].Width, frameRegions[i].Height);
-				var pixelLength = png.Palette == null ? 4 : 1;
-
 				frames[i] = new PngSheetFrame()
 				{
 					Size = frameSize,
 					FrameSize = frameSize,
 					Offset = frameOffsets[i],
-					Data = new byte[frameRegions[i].Width * frameRegions[i].Height * pixelLength],
+					Data = new byte[frameRegions[i].Width * frameRegions[i].Height * stride],
-					Type = png.Palette == null ? SpriteFrameType.BGRA : SpriteFrameType.Indexed
+					Type = png.Type
 				};
 
 				for (var y = 0; y < frames[i].Size.Height; y++)
-					Array.Copy(png.Data, (frameStart + y * png.Width) * pixelLength, frames[i].Data, y * frames[i].Size.Width * pixelLength, frames[i].Size.Width * pixelLength);
+					Array.Copy(png.Data, (frameStart + y * png.Width) * stride, frames[i].Data, y * frames[i].Size.Width * stride, frames[i].Size.Width * stride);
 			}
 
 			metadata = new TypeDictionary

OpenRA.Mods.Common/UtilityCommands/ConvertSpriteToPngCommand.cs

@@ -78,17 +78,9 @@ namespace OpenRA.Mods.Common.UtilityCommands
 							frame.Size.Width);
 				}
 
-				if (frame.Type == SpriteFrameType.BGRA)
+				var png = new Png(pngData, SpriteFrameType.Indexed, frameSize.Width, frameSize.Height, palColors);
-				{
-					var png = new Png(pngData, frameSize.Width, frameSize.Height);
 				png.Save("{0}-{1:D4}.png".F(prefix, count++));
 			}
-				else
-				{
-					var png = new Png(pngData, frameSize.Width, frameSize.Height, palColors);
-					png.Save("{0}-{1:D4}.png".F(prefix, count++));
-				}
-			}
 
 			Console.WriteLine("Saved {0}-[0..{1}].png", prefix, count - 1);
 		}

OpenRA.Mods.Common/UtilityCommands/DumpSequenceSheetsCommand.cs

@@ -45,7 +45,7 @@ namespace OpenRA.Mods.Common.UtilityCommands
 
 			var count = 0;
 
-			var sb = sequences.SpriteCache.SheetBuilders[SpriteFrameType.Indexed];
+			var sb = sequences.SpriteCache.SheetBuilders[SheetType.Indexed];
 			foreach (var s in sb.AllSheets)
 			{
 				var max = s == sb.Current ? (int)sb.CurrentChannel + 1 : 4;
@@ -53,7 +53,7 @@ namespace OpenRA.Mods.Common.UtilityCommands
 					s.AsPng((TextureChannel)ChannelMasks[i], palette).Save("{0}.png".F(count++));
 			}
 
-			sb = sequences.SpriteCache.SheetBuilders[SpriteFrameType.BGRA];
+			sb = sequences.SpriteCache.SheetBuilders[SheetType.BGRA];
 			foreach (var s in sb.AllSheets)
 				s.AsPng().Save("{0}.png".F(count++));
 

OpenRA.Mods.D2k/SpriteLoaders/R8Loader.cs

@@ -22,7 +22,7 @@ namespace OpenRA.Mods.D2k.SpriteLoaders
 	{
 		class R8Frame : ISpriteFrame
 		{
-			public SpriteFrameType Type { get; set; }
+			public SpriteFrameType Type { get { return SpriteFrameType.Indexed; } }
 			public Size Size { get; private set; }
 			public Size FrameSize { get; private set; }
 			public float2 Offset { get; private set; }