Small changes to the main entry classes

[onepiecefreak3]

Remove code duplications in BcDecoder;
Add more detailed summary to public methods in BcDecoder and BcEncoder;
Add DecodeRaw in BcDecoder as an equivalent to EncodeToRawBytes in BcEncoder;

[Nominom]

Awesome! This looks good. I was thinking I need to add those raw decoding functions at some point too.

It looks like you added the luminanceAsRed option to the Decoder input options, when it's already in the output options. Should probably keep the current convention.

Do you want to add tests for the raw decoding? I can also do it myself later.

[onepiecefreak3]

I actually decided against a test for the raw methods, since the EncodeToRawBytes method also has no direct test. At least I couldn't find one. And since I basically copy and pasted together code from other methods to decode and create an Image, that were already tested, it may not be needed.

It seems I have missed the input option redAsLuminance. I just put this option to input, since the encoder options also took it from the input. But now that you mention it, it doesn't make much sense. I will correct that.

[Nominom]

Seems like I've indeed missed a test for the raw encoding. I'll add tests for both then.

[onepiecefreak3]

I can add those tests myself, no problem. I like tests as well.
I will add them for both, encoding and decoding. I will just create an image encoded with BC1 and raw RGBA8888 (if it doesn't exist already in your test files) and add those tests.

[Nominom]

Could combine the tests to make it easier. Just encode some existing image to raw bytes then decode the bytes and compare with the original.

[onepiecefreak3]

The test currently has a problem since the recoded image does produce some colors with a difference of 1 in one component. It's like the encoding would introduce floating point errors. So, as if the interpolation fix causes this behaviour to be seen, now due to the encoding process. But I can't say that for sure.

I'm not sure if that is supposed to happen. Can we assume that your encoder just doesn't produce the exact same endpoints and introduce a margin of error to asserting the equality of a recoded image?

I will commit the test with a disabled assert and maybe we can look into that.

[Nominom]

Do you mean the raw tests? Can you push them? I can check if i see a problem. Sounds to me like it's just normal compression loss though.

Also, does the den parameter in the interpolation need to be a float? Doesn't seem like a float is used anywhere as an input.

I think the Half and Third methods should be named InterpolateHalf and InterpolateThird for clarity. Since they are extension methods.

[Nominom]

I've been using the BCnEncoder.Shared.ImageQuality.PeakSignalToNoiseRatio() function to test for similarity. Other tests have a passing grade of > 30 for Balanced and Best quality, and > 25 for Fast / bad quality. The default quality option is Balanced, so the encoder probably does not find the exact endpoints. Could use one of the ImageLoader test images like ImageLoader.testDiffuse1 or ImageLoader.testGradient1.

[onepiecefreak3]

I originally made the den parameter a float, since the division was meant to operate as a floating point division, instead of a integer division. And instead of casting inline, I decided to already type den as a float. I can just cast it inline, but I would like to keep it a floating point division.

I will rename the methods accordingly.

Ok, I will look into the other tests and see how they compare images and do that in my raw test. Hopefully it really is just a compression artifact and not a floating point error.

[Nominom]

I think inline casting would make the internal api more clear. Since it's meant to only be used with integers.

[Nominom]

I'll compare the new interpolation with Compressonator a bit later. I probably could add a test for it as well.

[onepiecefreak3]

At least for the case you provided in issue #3 the new interpolation gives the compressonator values. Of course one single data point is not enough to test that, but at least this case goes through ^^

[Nominom]

At least for the case you provided in issue #3 the new interpolation gives the compressonator values. Of course one single data point is not enough to test that, but at least this case goes through ^^

That's good to hear. It's probably correct then.

[onepiecefreak3]

I don't know why the build checks are failing. Locally all tests run through successfully.

[Nominom]

I don't know why the build checks are failing. Locally all tests run through successfully.

Seems like they passed now.

[onepiecefreak3]

Are you ok with having a commit providing more commentary and overall code style changes?
Code style changes would include:

• Preferably use var instead of explicit type for variables and out parameters
• Removing redundant brackets
• Removing some empty lines
• Changing fields in options to properties
• Prefixing private class fields with an underscore

[ptasev]

Might be good to add an editorconfig to make the spacing consistent. Right now it seems there's a mix of tabs and spaces throughout the code.

[Nominom]

Are you ok with having a commit providing more commentary and overall code style changes?
Code style changes would include:

* Preferably use `var` instead of explicit type for variables and out parameters

* Removing redundant brackets

* Removing some empty lines

* Changing fields in options to properties

* Prefixing private class fields with an underscore

Everything else is fine but I'd rather keep extra brackets even for 1-line blocks for clarity and underscore on private members is more of a c++ thing I think. The existing code is probably not that consistent on style but making it so is definitely a good thing.

Editorconfig is a good idea too. The diff was a bit hard to read since everything that was touched changed to spaces instead of tabs.

[onepiecefreak3]

Ah, sorry about that. My VS is configured so that tabs and auto-indenting is done via spaces.
It is a common naming scheme in C# as well to have prefixing underscores on private members. But of course I can leave it as is. I just reconfgure my VS for this solution.

[Nominom]

Yeah, I'd like to keep the privates without underscores. It's just my preference in C#.