Skip to content

apps/cdogs: PicPixelSize round-trips HD pic dimensions through size/2*2 (diagnostic-only, low priority, submodule) #15

Description

@jeffory

Problem

In apps/cdogs/src/src/cdogs/pic.c, an HD pic's stored size is halved at load time via svec2i_scale_divide(p->size, 2) (integer division), and PicPixelSize() doubles it back when the true pixel size is needed elsewhere:

static struct vec2i PicPixelSize(const Pic *p)
{
    if (p->isHD)
    {
        return svec2i_scale(p->size, 2);
    }
    return p->size;
}

PicLoad increments the diagnostic byte counter (g_picos_pic_data_bytes) using the original, un-halved size parameter (the true pixel count):

g_picos_pic_data_bytes += (size_t)size.x * size.y * sizeof *p->Data;

But PicFree decrements using PicPixelSize(pic) — the round-tripped value:

const struct vec2i dataSize = PicPixelSize(pic);
g_picos_pic_data_bytes -= (size_t)dataSize.x * dataSize.y * sizeof *pic->Data;

If an HD pic has an odd stored dimension, size / 2 truncates, and (size / 2) * 2 != size — so the free-side decrement undercounts relative to what the load-side increment added, by one row/column's worth of pixels.

Impact

Diagnostic-only: this only affects g_picos_pic_data_bytes, the resident-graphics byte counter used for reporting/telemetry. It does not affect any actual allocation, and cannot underflow (the counter would just accumulate a small permanent over-count per odd-dimensioned HD pic freed). It's unknown whether any HD asset in the current data set actually has an odd pixel dimension — worth checking before deciding this is worth fixing.

Suggested direction

If it turns out any HD asset has an odd dimension, have PicFree compute the decrement from the same true-pixel-count basis PicLoad used (e.g. store the true byte count on the Pic at load time rather than recomputing it via the lossy round-trip), rather than relying on PicPixelSize().

Context

Surfaced while implementing Stage 0/1 of the C-Dogs asset-memory redesign (local specs, not tracked in this repo) — this diagnostic counter was added as part of that work (submodule commits ef0e2a5d/3e968854/4d5308bd). Submodule commits 2b28ead7..8e1226ce.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions