Spritebatch best_fit failed, app crash

[krupitskas]

Hey! Im trying to understand right now how does spritebatch handle atlas overflow.

but check this

        spritebatch_internal_atlas_node_t *best_fit = spritebatch_best_fit(sp, width, height, nodes);

        image->min = best_fit->min;

static spritebatch_internal_atlas_node_t* spritebatch_best_fit(int sp, int w, int h, spritebatch_internal_atlas_node_t* nodes)
{
    int best_volume = INT_MAX;
    spritebatch_internal_atlas_node_t *best_node = 0;
    int img_volume = w * h;

    for ( int i = 0; i < sp; ++i )
    {
        spritebatch_internal_atlas_node_t *node = nodes + i;
        int can_contain = node->size.x >= w && node->size.y >= h;
        if ( can_contain )
        {
            int node_volume = node->size.x * node->size.y;
            if ( node_volume == img_volume ) return node;
            if ( node_volume < best_volume )
            {
                best_volume = node_volume;
                best_node = node;
            }
        }
    }

    return best_node;
}

best fit easily can be 0 and no return value checks.

[RandyGaul]

I’m not sure what you’re trying to say. I wrote that code, so just copy pasting it here doesn’t really help me understand anything

[krupitskas]

Alright..... I think you missed the bottom comment, but once again. First part is calculating best_fit value and INSTANTLY use it without any checks. Bottom part is best_fit calculation logic, now look

spritebatch_internal_atlas_node_t *best_node = 0;

and only if

if ( can_contain )

pass, then best_node being initialized. If not then you dereferencing 0. What happens when you derefer 0?

[RandyGaul]

Oh gotcha, I get it now. So you noticed a crash from NULL pointer dereferencing? Can you try adding this right after line 1561?

if (!best_fit) {
    image->fit = 0;
    continue;
}

The best fit code came from cute_png.h, where overflow worked properly. I believe I have accidentally deleted the above if-statement, as seen here https://github.com/RandyGaul/cute_headers/blob/master/cute_png.h#L1496-L1500

Please try that out and report back if it crashes or works :)

[krupitskas]

Please try that out and report back if it crashes or works :)

It's still crashing, I've tried to fix all, but when the 7th crash appeared, I've left my attempts. It start to provide null pointers everywhere, tried to put as much as possible == NULL comparation, still was a new places where it appeared.

How to reproduce - config:

spritebatch_config_t get_demo_config()
{
    spritebatch_config_t config;
    spritebatch_set_default_config(&config);
    config.pixel_stride = sizeof(uint8_t) * 4; // RGBA uint8_t from cute_png.h
    config.atlas_width_in_pixels = 512;
    config.atlas_height_in_pixels = 512;
    config.atlas_use_border_pixels = 0;
    config.ticks_to_decay_texture = 0;
    config.lonely_buffer_count_till_flush = 16;
    config.ratio_to_decay_atlas = 1.0f;
    config.ratio_to_merge_atlases = 0.0;
    config.allocator_context = 0;
    return config;
}

btw, this comment doesn't make any sense

float ratio_to_decay_atlas;         // from 0 to 1, once ratio is less than `ratio_to_decay_atlas`, flush active textures in atlas to lonely buffer

Textures:

// example file/asset i/o system
const char* image_names[] = {
    "smile.png",
    "bat.png",
    "behemoth.png",
    "crow.png",
    "dragon_zombie.png",
    "fire_whirl.png",
    "giant_pignon.png",
    "night_spirit.png",
    "orangebell.png",
    "petit.png",
    "tree.png",
    "power_critter.png",
};

[RandyGaul]

Your tree png is larger than 512x512. You've setup spritebatch to use atlases of dimension 512x512, so it's impossible for it to fit anywhere. I will try thinking of a way to report this error, or make it more obvious what is happening behind the scenes.

[RandyGaul]

I went ahead and added some asserts very early on to detect if an impossible atlas occurs.

https://github.com/RandyGaul/cute_headers/commit/79997497da6674055a6bba5611f89b1884dcb496

https://github.com/RandyGaul/cute_headers/commit/87a7bd2806069750c8bede38a9e34510216c1390

Please feel free to reopen this if you still have any problems!