[CR] Add low light and night vision filters to tilesets

[DanmakuDan]

(Updated Oct 11) This adds an extra set of tile textures that have been modified to be in grayscale, night vision, washed-out night vision, usable for low light situations ingame. The extra tiles are generated during tileset loading by copying the main tile surface and applying a color filter to it. The light level that was provided previously per tile is used in determining the tile texture to use. This should allow the tiles display to match closer to what ASCII was already showing based on light levels.

Note: characters will not show up with night vision currently, since the player tile is always considered lit, causing the player to appear very bright when using night vision goggles.

Outside view during daylight:

Near dusk:

Flashlight on:

Standing inside a furniture store during dusk:

Night vision goggles at dusk:

Washed out color caused by using a flashlight with goggles:

Washed out color caused by standing in daylight with goggles on:

[chaosvolt]

Ah, so this doesn't require tileset authors to add anything, then?

[DanmakuDan]

It's like a filter being applied to an image, and the post-processed image is stored as the textures used for rendering.

[chaosvolt]

Ah, interesting. o.o

[BevapDin]

Creating the surface requires different logic based on the endianness of the platform (see create_tile_surface), does accessing the pixels not need this?

Lots of (C-style) casting going on there. Would be easier to read if the pointer is stored:

const auto base_array = reinterpret_cast<unsigned char*>(surf->pixels);
const auto pixel_ptr = base_array + (y * w + x) * 4;
pix.r = pixel_ptr[0];
pix.g = pixel_ptr[1];
...

After all, the only thing that differs for the pix.(r|g|b|a) = lines is the color index 0...3.

[DanmakuDan]

I was wondering about endianness earlier, but the create_tile_surface function already handles that using SDL's endian macro already. The resulting pixel format is always in the order R G B A. Alternatively, if that's not actually true, then I think it currently would affect a Mac build?

[DanmakuDan]

Updated pointer logic, the endianness is already handled here:

SDL_Surface *cata_tiles::create_tile_surface()
{
    SDL_Surface *surface;
    #if SDL_BYTEORDER == SDL_BIG_ENDIAN
        surface = SDL_CreateRGBSurface(0, tile_width, tile_height, 32, 0xFF000000, 0x00FF0000, 0x0000FF00, 0x000000FF);
    #else
        surface = SDL_CreateRGBSurface(0, tile_width, tile_height, 32, 0x000000FF, 0x0000FF00, 0x00FF0000, 0xFF000000);
    #endif
    if( surface == nullptr ) {
        dbg( D_ERROR ) << "Failed to create surface: " << SDL_GetError();
    }
    return surface;
}

[DanmakuDan]

Currently adding night vision filters, setting back to WIP.

[DanmakuDan]

Night vision goggle filters have been added, and first post was updated.

[chaosvolt]

Wow that is impressive.

[kevingranade]

These two pixel adaptors (translating from a SDL pixel to your idea of a pixel IIUC) don't need to be part of the cata_tiles class, you can make them static helper functions, which will limit their level of privledge and not expose them to callers of cata_tiles. This would also let you move the pixel struct definition into the .cpp file so it isn't shared outside the class either.

[kevingranade]

These three methods seem like good candidates to be deduplicated, you can extract the code for iterating over the pixel values, and just have the logic that is applied to each pixel be declared seperately.

[kevingranade]

As none of these new methods are called from outside cata_tiles, they should be private instead of protected.

[kevingranade]

These new member variables should get the same treatment as the new methods, default to private instead of protected.

[kevingranade]

I'll have to poke at the SDL documentation a bit, but usually it provides a cleaner interface than this to manipulate data, there are probably some setter/getter macros or something that would encapsulate all this.

[DanmakuDan]

I noticed some missing logic related to ascii sets loading, will be updating soon.

[BevapDin]

Technically, you don't need a COLOR_FILTER enum at all, you can just forward a reference to the function that should be applied to each pixel. And apply_color_filter can be a free function.

void apply_color_filter( SDL_Surface *surf, void (&pixel_converter)(pixel &) )
{
    for( ... ) {
        pixel pix = get_pixel_color( ... );
        pixel_converter( pix );
        set_pixel_color( ... );
    }
}

void init() {
    ...
    apply_color_filter( shadow_tile_atlas, color_pixel_grayscale );
    apply_color_filter( shadow_tile_atlas, color_pixel_overexposed );
    apply_color_filter( shadow_tile_atlas, color_pixel_nightvision );
    ...
}

If you keep the enum, please remove the default: case in the switch. It disables a very useful warning of the compiler: if the switch statement is over an enum type (as in this case), the compiler will warn you if some entries of the enum are not handled. This comes in handy when another entry is added to the enum, but the switch is not updated.

[BevapDin]

Why this separate counter? It is only incremented when stuff is pushed to shadow_tile_values, so why not use shadow_tile_values.size() instead? Also the code that resets that counter is a bit inconsistent: the vector is cleared in clear(), but the counter is reset in reinit (right after calling clear). It's technically correct, but it's quite confusing.