I'm trying to make a brightness/contrast filter using SDL3 and ImGui but for some reason when I try to get the rgb values of each pixel using SDL_GetRGB the values are always 0,0,0. This is my function:
`void brightness_contrast_effect(SDL_Surface* surface, int brightness, int contrast)
{
if(SDL_MUSTLOCK(surface))
{
SDL_LockSurface(surface);
}
Uint8 r,g,b;
Uint32* pixels = (Uint32*) surface->pixels;
const SDL_PixelFormatDetails* pixelDetails = SDL_GetPixelFormatDetails(surface->format);
float constrast_factor = (259.0f * (contrast + 255)) / (255.0f * (255 - contrast));
std::cout << "contrast_value: " << std::fixed << std::setprecision(4) << constrast_factor << std::endl;
for(int y = 0; y < surface->h; y++)
{
for (int x = 0; x < surface->w ; x ++)
{
Uint32* pixel = pixels + (y * surface->w + x);
SDL_GetRGB(*pixel, pixelDetails, NULL, &r, &g, &b);
// std::cout << "R: " << static_cast<int>(r)
// << " G: " << static_cast<int>(g)
// << " B: " << static_cast<int>(b)
// << std::endl;
r = SDL_clamp(r + brightness, 0 , 255);
g = SDL_clamp(g + brightness, 0 , 255);
b = SDL_clamp(b + brightness, 0 , 255);
r = SDL_clamp((constrast_factor * (r - 128)) + 128.0f, 0 , 255);
g = SDL_clamp((constrast_factor * (g - 128)) + 128.0f, 0 , 255);
b = SDL_clamp((constrast_factor * (b - 128)) + 128.0f, 0 , 255);
*pixel = SDL_MapRGB(pixelDetails, NULL, r,g,b);
}
}
if(SDL_MUSTLOCK(surface))
{
SDL_UnlockSurface(surface);
}`
Since the rgb values are always 0 when I move my slider the image disappears and I just get a black to grey gradient.
There are multiple issues with your code that I will address in the following.
Your pixel access is not portable and does not work for any 3-bytes-per-pixel image, which would be the standard for a bitmap (.bmp) file. Use an 8bit-pointer to the pixel address and the pitch
member to correctly extract the pixel data you pass to SDL_GetRGB
.
Uint8 r = 0u, g = 0u, b = 0u;
const SDL_PixelFormatDetails* pixelDetails = SDL_GetPixelFormatDetails(surface->format);
const Uint8 bpp = SDL_BYTESPERPIXEL(surface->format);
for (int y = 0; y < surface->h; y++)
{
for (int x = 0; x < surface->w; x++)
{
Uint8* pixel = static_cast<Uint8*>(surface->pixels) + y * surface->pitch + x * bpp;
SDL_GetRGB(*reinterpret_cast<Uint32*>(pixel), pixelDetails, NULL, &r, &g, &b);
Also upon writing back the pixel data make sure not to overwrite the next pixel. Writing a full Uint32
onto a 3-bytes-per-pixel image thus does not work.
Uint32 res = SDL_MapRGB(pixelDetails, NULL, r, g, b);
memcpy(pixel, &res, bpp);
Converting between Uint8
and float
may have undesirable effects, e.g. over- or underflow. Make sure you cast to a wide-enough datatype prior to your operations.
r = static_cast<Uint8>(SDL_clamp(static_cast<int>(r) + brightness, 0, 255));
g = static_cast<Uint8>(SDL_clamp(static_cast<int>(g) + brightness, 0, 255));
b = static_cast<Uint8>(SDL_clamp(static_cast<int>(b) + brightness, 0, 255));
r = static_cast<Uint8>(SDL_clamp((constrast_factor * (static_cast<float>(r) - 128.0f)) + 128.0f, 0.0f, 255.0f));
g = static_cast<Uint8>(SDL_clamp((constrast_factor * (static_cast<float>(g) - 128.0f)) + 128.0f, 0.0f, 255.0f));
b = static_cast<Uint8>(SDL_clamp((constrast_factor * (static_cast<float>(b) - 128.0f)) + 128.0f, 0.0f, 255.0f));
Changing brightness and/or contrast of an image is beyond simply adding the same offset to the R,G,B channels. There are physical models of human perception and corresponding color-space transformations. If you want to dive into these, start with CIE LAB color space.