OutpostUniverse / OP2Utility

C++ library for working with Outpost 2 related files and tasks.
MIT License
4 stars 0 forks source link

Remove non padded pixel write function #200

Closed Brett208 closed 5 years ago

Brett208 commented 5 years ago

This push is bittersweet. It removes very large chunks of the IndexedBmpWriter code base. Some of that code I was proud of, but I do not believe it is needed in the final product.

I rewrote the test suite to be more specific in what is being tested. It still doesn't test if the BMP file is well formed, only if exceptions are properly raised or not raised based on data input. Perhaps this would be a good effort for another branch.

Closes #194

Brett208 commented 5 years ago

@DanRStevens and @ldicker83 , what do you think about the following lines:

const auto pitch = CalculatePitch(bitCount, width);
const auto bytesOfPixelsPerRow = CalcPixelByteWidth(bitCount, width);

It can prevent future code changes if the type is updated, but I feel like it hides the underlying type. It is hard to know what type is returned without inspecting the function's signature.

-Brett

DanRStevens commented 5 years ago

I like it. The underlying type is actually quite irrelevant from a high level perspective. You have a quantity that represents the size or spacing between scanlines. The number of bytes and signedness of that quantity is an implementation detail. If the compiler can handle those details, let it.

Brett208 commented 5 years ago

Made the last suggested change. I'm going to merge and move on.

Thanks, Brett