Fixed systematic rounding errors

[RunDevelopment]

This PR fixes #88. I applied my floating point solution (with a nicer interface) to DXT1, DXT3, and DXT5.

You can really see the rounding errors in the tests. They were a lot more common than I expected. Interestingly, the largest difference is 3. I only expected them to be off by one, so this was quite surprising. I checked that all changed colors are decoded correctly by comparing them with the color values Paint.net (which uses DirectX to read and write DDS files) has.

As for the hashes, I simply copied the actual values. There's probably some cool command that updates the these inline snapshots, so it would be nice if that was in the README.

I haven't done benchmarks yet, but they will likely not show much given my previous experience. I'll do them shortly.

[RunDevelopment]

And I can't run benchmarks. It errors at some point and doesn't output anything. Maybe this has something to do with your recent project changes @nickbabcock? This is the error:

``` System.Reflection.TargetInvocationException: Ein Aufrufziel hat einen Ausnahmefehler verursacht. ---> System.DllNotFoundException: Die DLL "DevIL": Das angegebene Modul wurde nicht gefunden. (Ausnahme von HRESULT: 0x8007007E) kann nicht geladen werden. bei DevILSharp.IL.GenImage() bei DevILSharp.Image.Load(Byte[] arr, ImageType imageType) bei Pfim.Benchmarks.DdsBenchmark.DevILSharp() bei BenchmarkDotNet.Autogenerated.Runnable_3.WorkloadActionNoUnroll(Int64 invokeCount) bei BenchmarkDotNet.Engines.Engine.RunIteration(IterationData data) bei BenchmarkDotNet.Engines.EngineFactory.Jit(Engine engine, Int32 jitIndex, Int32 invokeCount, Int32 unrollFactor) bei BenchmarkDotNet.Engines.EngineFactory.CreateReadyToRun(EngineParameters engineParameters) bei BenchmarkDotNet.Autogenerated.Runnable_3.Run(IHost host, String benchmarkName) --- Ende der internen Ausnahmestapel?berwachung --- bei System.RuntimeMethodHandle.InvokeMethod(Object target, Object[] arguments, Signature sig, Boolean constructor) bei System.Reflection.RuntimeMethodInfo.UnsafeInvokeInternal(Object obj, Object[] parameters, Object[] arguments) bei System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture) bei BenchmarkDotNet.Autogenerated.UniqueProgramName.AfterAssemblyLoadingAttached(String[] args) // AfterAll // Benchmark Process 5788 has exited with code -1. Unbehandelte Ausnahme: System.InvalidOperationException: Die Sequenz enthält kein übereinstimmendes Element. bei System.Linq.Enumerable.Last[TSource](IEnumerable`1 source, Func`2 predicate) bei BenchmarkDotNet.Running.BenchmarkRunnerClean.Execute(ILogger logger, BenchmarkCase benchmarkCase, BenchmarkId benchmarkId, IToolchain toolchain, BuildResult buildResult, IResolver resolver) bei BenchmarkDotNet.Running.BenchmarkRunnerClean.RunCore(BenchmarkCase benchmarkCase, BenchmarkId benchmarkId, ILogger logger, IResolver resolver, BuildResult buildResult) bei BenchmarkDotNet.Running.BenchmarkRunnerClean.Run(BenchmarkRunInfo benchmarkRunInfo, Dictionary`2 buildResults, IResolver resolver, ILogger logger, List`1 artifactsToCleanup, String resultsFolderPath, String logFilePath, StartedClock& runChronometer) bei BenchmarkDotNet.Running.BenchmarkRunnerClean.Run(BenchmarkRunInfo[] benchmarkRunInfos) bei BenchmarkDotNet.Running.BenchmarkSwitcher.RunWithDirtyAssemblyResolveHelper(String[] args, IConfig config, Boolean askUserForInput) bei BenchmarkDotNet.Running.BenchmarkSwitcher.Run(String[] args, IConfig config) bei Pfim.Benchmarks.Program.Main(String[] args) in C:\Users\micha\Git\Pfim\src\Pfim.Benchmarks\Program.cs:Zeile 10. // * Artifacts cleanup * ```

[nickbabcock]

And I can't run benchmarks. It errors at some point and doesn't output anything. Maybe this has something to do with your recent project changes @nickbabcock? This is the error:

Yeah I'm not sure when the benchmarks broke, as I tried running the benchmarks many commits ago and they were still broken. This should now not happen again, as I've added running the benchmarks to CI.

Can you rebase off master and see if the benchmarks are fixed?

Sorry for all the dust I kicked up with the project changes. I wanted to get them in to make it easier for us to feel confident about any performance results

[RunDevelopment]

Very nice, they work now. Thank you! I'll post the results shortly.

[RunDevelopment]

Alright, the results are in and it's looking pretty good. As my initial run in #88 suggested, there is no noteable difference in performance, which is good.

DXT5 seems to have gotten faster, but that's probably just background process running and making the Before benchmark slower. The other benchmarked decoders also got faster in the After benchmark, so it's unlikely that my code changes increased performance.

Benchmarks:

BenchmarkDotNet=v0.13.1, OS=Windows 10.0.19044.1865 (21H2)
Intel Core i7-8700K CPU 3.70GHz (Coffee Lake), 1 CPU, 12 logical and 6 physical cores
.NET SDK=6.0.400
  [Host]     : .NET 6.0.8 (6.0.822.36306), X64 RyuJIT
  DefaultJob : .NET 6.0.8 (6.0.822.36306), X64 RyuJIT

Before

| Method | Payload | Mean | Error | StdDev | StdErr | Median | Gen 0 | Gen 1 | Gen 2 | Allocated | |------------ |---------- |-------------:|-----------:|-----------:|----------:|-------------:|--------:|--------:|--------:|----------:| | **Pfim** | **32bit.dds** | **2.372 μs** | **0.0177 μs** | **0.0157 μs** | **0.0042 μs** | **2.374 μs** | **0.0763** | **-** | **-** | **496 B** | | FreeImage | 32bit.dds | 269.457 μs | 2.6861 μs | 2.3812 μs | 0.6364 μs | 269.907 μs | 12.2070 | 12.2070 | 12.2070 | 5,025 B | | ImageMagick | 32bit.dds | 1,305.426 μs | 4.8699 μs | 4.5553 μs | 1.1762 μs | 1,305.282 μs | 1.9531 | - | - | 13,266 B | | DevILSharp | 32bit.dds | 115.278 μs | 0.3941 μs | 0.3686 μs | 0.0952 μs | 115.181 μs | - | - | - | 128 B | | **Pfim** | **dxt1.dds** | **1.211 μs** | **0.0197 μs** | **0.0184 μs** | **0.0047 μs** | **1.207 μs** | **0.0839** | **-** | **-** | **528 B** | | FreeImage | dxt1.dds | 40.740 μs | 0.7989 μs | 1.3348 μs | 0.2225 μs | 40.457 μs | 13.4277 | 13.4277 | 13.4277 | 2,145 B | | ImageMagick | dxt1.dds | 410.323 μs | 3.1319 μs | 2.6153 μs | 0.7253 μs | 409.281 μs | 1.9531 | - | - | 13,246 B | | DevILSharp | dxt1.dds | 48.750 μs | 0.2152 μs | 0.1908 μs | 0.0510 μs | 48.778 μs | - | - | - | 128 B | | **Pfim** | **dxt3.dds** | **1.210 μs** | **0.0129 μs** | **0.0115 μs** | **0.0031 μs** | **1.207 μs** | **0.0839** | **-** | **-** | **528 B** | | FreeImage | dxt3.dds | 53.761 μs | 1.0052 μs | 0.9873 μs | 0.2468 μs | 53.868 μs | 13.4277 | 13.4277 | 13.4277 | 2,385 B | | ImageMagick | dxt3.dds | 457.618 μs | 3.7906 μs | 3.5457 μs | 0.9155 μs | 459.159 μs | 1.9531 | - | - | 13,246 B | | DevILSharp | dxt3.dds | 76.815 μs | 1.1313 μs | 1.0582 μs | 0.2732 μs | 76.643 μs | - | - | - | 128 B | | **Pfim** | **dxt5.dds** | **1.336 μs** | **0.0266 μs** | **0.0327 μs** | **0.0070 μs** | **1.336 μs** | **0.0896** | **-** | **-** | **568 B** | | FreeImage | dxt5.dds | 73.210 μs | 0.8810 μs | 0.7809 μs | 0.2087 μs | 73.232 μs | 13.5498 | 13.5498 | 13.5498 | 2,385 B | | ImageMagick | dxt5.dds | 525.018 μs | 10.2807 μs | 12.6256 μs | 2.6918 μs | 523.311 μs | 1.9531 | - | - | 13,253 B | | DevILSharp | dxt5.dds | 74.928 μs | 1.2130 μs | 1.1347 μs | 0.2930 μs | 74.833 μs | - | - | - | 128 B |

After

| Method | Payload | Mean | Error | StdDev | StdErr | Median | Gen 0 | Gen 1 | Gen 2 | Allocated | |------------ |---------- |-------------:|-----------:|-----------:|----------:|-------------:|--------:|--------:|--------:|----------:| | **Pfim** | **32bit.dds** | **2.200 μs** | **0.0277 μs** | **0.0246 μs** | **0.0066 μs** | **2.210 μs** | **0.0763** | **-** | **-** | **496 B** | | FreeImage | 32bit.dds | 270.120 μs | 2.3818 μs | 2.2279 μs | 0.5753 μs | 269.893 μs | 12.6953 | 12.6953 | 12.6953 | 5,025 B | | ImageMagick | 32bit.dds | 1,273.669 μs | 14.9527 μs | 13.9868 μs | 3.6114 μs | 1,278.949 μs | 1.9531 | - | - | 13,266 B | | DevILSharp | 32bit.dds | 113.118 μs | 0.2183 μs | 0.1704 μs | 0.0492 μs | 113.107 μs | - | - | - | 128 B | | **Pfim** | **dxt1.dds** | **1.222 μs** | **0.0077 μs** | **0.0072 μs** | **0.0019 μs** | **1.219 μs** | **0.0839** | **-** | **-** | **528 B** | | FreeImage | dxt1.dds | 40.208 μs | 0.4495 μs | 0.3753 μs | 0.1041 μs | 40.289 μs | 13.5498 | 13.5498 | 13.5498 | 2,145 B | | ImageMagick | dxt1.dds | 405.563 μs | 4.4299 μs | 4.1437 μs | 1.0699 μs | 404.365 μs | 1.9531 | - | - | 13,246 B | | DevILSharp | dxt1.dds | 48.613 μs | 0.1528 μs | 0.1276 μs | 0.0354 μs | 48.636 μs | - | - | - | 128 B | | **Pfim** | **dxt3.dds** | **1.201 μs** | **0.0107 μs** | **0.0100 μs** | **0.0026 μs** | **1.205 μs** | **0.0839** | **-** | **-** | **528 B** | | FreeImage | dxt3.dds | 50.879 μs | 0.8502 μs | 0.7537 μs | 0.2014 μs | 50.908 μs | 13.4277 | 13.4277 | 13.4277 | 2,385 B | | ImageMagick | dxt3.dds | 459.834 μs | 5.0993 μs | 4.7699 μs | 1.2316 μs | 461.547 μs | 1.9531 | - | - | 13,246 B | | DevILSharp | dxt3.dds | 76.112 μs | 0.9255 μs | 0.8657 μs | 0.2235 μs | 76.339 μs | - | - | - | 128 B | | **Pfim** | **dxt5.dds** | **1.224 μs** | **0.0102 μs** | **0.0095 μs** | **0.0025 μs** | **1.220 μs** | **0.0896** | **-** | **-** | **568 B** | | FreeImage | dxt5.dds | 68.391 μs | 1.1327 μs | 1.0595 μs | 0.2736 μs | 68.630 μs | 13.5498 | 13.5498 | 13.5498 | 2,385 B | | ImageMagick | dxt5.dds | 500.117 μs | 4.1895 μs | 3.2709 μs | 0.9442 μs | 500.309 μs | 1.9531 | - | - | 13,253 B | | DevILSharp | dxt5.dds | 75.014 μs | 1.0198 μs | 0.9040 μs | 0.2416 μs | 75.160 μs | - | - | - | 128 B |

[nickbabcock]

In case you want to run more dds benchmarks in the future you can target them better with:

dotnet run -c Release --project .\src\Pfim.Benchmarks -- --filter "*.DdsBenchmark.Pfim"

I ran the benchmarks too and didn't notice any deviations with this change.

---

You can really see the rounding errors in the tests. They were a lot more common than I expected. Interestingly, the largest difference is 3. I only expected them to be off by one, so this was quite surprising

Wow yeah, it's a bummer this large of a correctness issue has gone unnoticed for so long. I'm unsure what the upper bound of the error is, but anything above zero is unacceptable 😄

I ran the PR through some conversion tests and everything looked good:

dotnet run -c Release --project .\src\Pfim.Skia -- .\tests\Pfim.Tests\data\bc2-simple-srgb.dds

And spot checked the output the generated png with the dds image open in Paint.NET.

I did try and find a way to compare pfim output with a reference implementation, but I haven't solved the problem yet. I was hoping that imagemagick's compare would work, but it only supports BC1 images (afaik) and even for the BC1 image I tested on, it didn't work.

---

This PR looks good. I'll mull it over a little longer before merging and then releasing a new version.