Refactor palette IO and storage #545
Conversation
| Write(colour.Blue); | ||
| Write(colour.Unused); | ||
| } | ||
| } |
There was a problem hiding this comment.
Shouldn't we create an override Write(TRColour4 colour)?
TRLevelControl/IO/TRLevelReader.cs
Outdated
| Red = ReadByte(), | ||
| Green = ReadByte(), | ||
| Blue = ReadByte(), | ||
| Unused = ReadByte() |
There was a problem hiding this comment.
Is it really unused, or is it alpha? If it's unused maybe it's best to convert it to Colour3 and then write dummy values back?
There was a problem hiding this comment.
It's unused in the original TR2 and I'm fairly sure it's the same for TR3. But you are right, TR2Main treats it as the alpha channel, so I think renaming it is a good idea as it makes sense that this should be its purpose.
There was a problem hiding this comment.
Renamed now to Alpha. I've also fixed a previous hack where we were keeping track of imported colours using the Unused property. There is now a proper tracker class for 16-bit colour import, which will also quantize if the palette runs out of space - very unlikely in TR2/3, but it's good practice.
|
I've simplified the palette16 tracker as it felt overly complicated for what it does. Best to do it now while it's fresh. It now makes a queue of free indices and these are then allocated to imported colours as necessary. It no longer assumes the colours are ordered by usage and so will accept gaps (not that it occurs in OG levels, but best not to assume). I also standardised the names between it and the palette8 control class, and shared out the method to find the nearest colour. |
| } | ||
|
|
||
| return colIndex; | ||
| } |
There was a problem hiding this comment.
Maybe we could change this to use more LINQ?
var indexed = palette.Select((paletteColour, index) => new {
Index = index,
Delta = Math.Sqrt(
Math.Pow(colour.R - paletteColour.R, 2) +
Math.Pow(colour.G - paletteColour.G, 2) +
Math.Pow(colour.B - paletteColour.B, 2)
),
});
return indexed.Min(item => item.Delta).Index;Also, it's unnecessary to call Math.Sqrt here to get the accurate Euclidean distance – for the purpose of determining the closest point, comparing the squared distances is sufficient and will yield the same result.
There was a problem hiding this comment.
Thanks. Good point about Math.Sqrt - some unnecessary calculations there :) Updated now with LINQ - just had to ensure we skip over index 0 for TR1, so we honour the passed in start index.
| { | ||
| int multiplier = TRConsts.Palette8Multiplier; | ||
| return Color.FromArgb(c.Red * multiplier, c.Green * multiplier, c.Blue * multiplier); | ||
| } |
There was a problem hiding this comment.
If this is only used by TR1, I think it's worth highlighting it somehow with function names.
There was a problem hiding this comment.
Makes sense - now renamed. I'm thinking we should only have one colour class eventually and have the IO handle this complexity - so multiplied out on read and divided on write for TR1. But I'll wait until we do more with TR3/4/5 lights as they use colours in different formats. That way we can manipulate colours as we please and the complexity is left until writing back.
It would also need updates to the custom model exports we use for cross-level enemies, so I think handling it later will be best.
| @@ -139,7 +124,7 @@ private static void ReindexMeshTextures(TRMesh[] meshes, Dictionary<int, int> in | |||
|
|
|||
| private static ushort ReindexTexture(ushort value, Dictionary<int, int> indexMap, bool has16Bit) | |||
| { | |||
| int p16 = value;; | |||
| int p16 = value; | |||
This continues the level IO refactoring and conversion of arrays into lists. The palettes and lightmaps are dealt with here (applies only to TR1-3).
Part of #507 and part of #468.