PERF: FUTURE: Default default-constructors of RGBPixel and RGBAPixel

[N-Dekker]

Will make image.Allocate() and image.Allocate(false) calls much, much
faster, for RGB and RGBA images, when ITK_FUTURE_LEGACY_REMOVE is enabled.
An Allocate() call on a 1024x1024x1024 RGB image was observed to take more than
1.3 sec. before this commit, and less than 0.001 sec. after this commit, having
ITK_FUTURE_LEGACY_REMOVE=ON, using Visual C++ 2019 Release. Even
image.Allocate(true) (initializePixels = true) appears slightly faster now
with ITK_FUTURE_LEGACY_REMOVE.

Adjusted unit tests tests from itkCommonTypeTraitsGTest, because RGBPixel and
RGBAPixel become "trivial" types when they get defaulted default-constructors.

[dzenanz]

Why protect this by legacy ifdef? Does this change introduce backwards compatibility issues? Which ones?

[hjmjohnson]

I think I agree with @dzenanz. I don't immediately see a backward-compatible problem with the code change.

[N-Dekker]

Thanks for asking! With the original code, even rgbImage.Allocate(false) (with initializePixels parameter = false) zero-fills the RGB pixels. Whereas when having a defaulted default-constructor for RGBPixel, only rgbImage.Allocate(true) zero-initializes the pixels. So with this change, users have to explicitly specify the parameter initializePixels = true, if that is what they want.

Also, a declared local variable of an RGB pixel type may not be initialized, with this change. So in the code below, the behavior may change for pixel1. It will not change for pixel2, pixel3, and pixel4.

RGBPixel<> pixel1; // May not be initialized, after this change 
RGBPixel<> pixel2{}; // Properly zero-initialized.
RGBPixel<> pixel3 = RGBPixel<>{}; // Properly initialized.
RGBPixel<> pixel4 = RGBPixel<>(); // Properly initialized.

Obviously it has been like that for other pixel types already, including int, float, itk::Vector<int> etc. But it's a behavior change for users who expect RGB pixels to always be initialized by default.

Anyway, I can also directly make it LEGACY_REMOVE, instead of just FUTURE_LEGACY_REMOVE. Would you like that better?

[dzenanz]

Thanks for explaining. I am on the fence of legacy vs future legacy.

[N-Dekker]

I am on the fence of legacy vs future legacy.

Thanks @dzenanz. I am also on the fence of legacy vs future legacy. 😸 But only if someone is strongly in favor of LEGACY_REMOVE, instead of FUTURE_LEGACY_REMOVE, I would still adjust the PR. @hjmjohnson Is FUTURE_LEGACY_REMOVE now OK to you?