color object normalization issues

[hendrik-brower]

When you create a ColorInt8, 127 is the "max" value, but if you make black: ColorInt8.rgba(0,0,0,127), you end up with grey. If you use ColorUint8.rgba(0,0,0,255) you get black. Though for both of these, the {rgba}Normalized values show 1.0. The grey is observed when doing something of the form:
cmd.fill(white...);
cmd.fillRec(black...);

I think there is a similar issue with createImage with respect to formatting of the image data with respect to the number of channels, though I'm observing this from using a generated image in another library and I can't say for certain if its incorrectly generated or incorrect used.

[brendan-duncan]

Can you give me a specific repo example?

[hendrik-brower]

I would suggest reviewing test cases. Just peeking at test/color/color_int8_test.dart, it looks a bit sparse.

Since the class doesn't document this sort of detail, I would recommend adding some clarifying notes or maybe range checks in the code:
assert(r<128&&r>=-128, 'bad r value') or maybe an throw ArgumentError.

As it is now, users can initialize the colorInt8 with any integer, though I'm guessing it effectively truncates the value with &0xff. Even reading the code for the class, I'm not sure if negative numbers are valid. Since its an int8, I would assume so. If negatives are valid, then the normalization calculations are probably not correct.

I don't understand what the draw test is checking. It looks like it could be filled in a bit. In that area, it might be worth defining test cases to verify the various draw functions. I'm betting each could be compared to some expected value. Admittedly, the expected value would probably be generated by running the code, however, over time errors will get corrected and changes that impact the behavior unexpectedly will get flagged.

Since Command.createImage defines the format of the image, but Command.drawRect, for example, just takes the abstract color, I would assume the internal representation of the image is a normalized format and the command class is responsible for normalizing the manipulations of the internal image data. Given that, every draw type test could be run with every color type and use a common expected value to verify it. I think that would catch the bug noted in this issue. Iterating over black, white, red, green, blue might catch unintended channel swaps.

Just some thoughts.

[brendan-duncan]

You're correct that the tests aren't testing all that much. They're more of a "spit out a bunch of images and I visually look at the results to see if something is messed up" tests, than actual unit tests.

Yes, more error checking could (should) be added. The problem is Dart is awful about doing things carefully while maintaining any sense of performance. I'm constantly having to fight Dart for performance, it is extremely sensitive to performance. The language really wasn't designed to be great at image processing, and using native bindings is not something I'm interested in maintaining. Everyone wants to do image processing on 8k resolution images, and then complain that it's slow. The color/pixel abstractions don't help, but I needed them to support all of the image formats people wanted.

The draw functions do use normalized color values to compensate for the varying color formats.

If I get some time, I'll work on improving tests, particularly around colors. I am also always open to PRs.