fix(contacts): review fixes for the avatar library grid

From the adversarial review of the image-library grid:

- MED: the delete badge was an InvisibleButton positioned via SetCursorScreenPos,
  which left CursorPosPrevLine at the badge corner — the next cell's SameLine
  reads that, so a hovered/selected row's trailing cells jittered ~3px and their
  hit-rects overlapped. Hit-test the badge MANUALLY (no layout item, no cursor
  moves); it still takes click priority over selecting the thumbnail.
- MED: avatar textures were uploaded at full native resolution and cached for the
  session with no cap — a library of large photos could cost GBs of VRAM, and the
  contact list decoded every image avatar at once on tab open. Box-downscale to
  <=256px (an avatar renders at most ~112px) and budget decodes to a few per
  frame, so large libraries fill in progressively instead of stalling.

(A third finding — a symlink edge case in the delete guard — was reviewed and
judged below the bar; the weakly_canonical + parent-path guard already holds.)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-07-15 10:33:51 -05:00
parent 62fc202557
commit 6ac8f66644

View File

@@ -68,13 +68,49 @@ static bool matchesSearch(const data::AddressBookEntry& e, const std::string& ne
} }
// Lazily-loaded, path-keyed cache of custom avatar image textures (loaded once, kept for the session). // Lazily-loaded, path-keyed cache of custom avatar image textures (loaded once, kept for the session).
// Textures are box-downscaled to <=kAvatarTexMax px so a big source photo can't cost tens of MB of
// VRAM — an avatar renders at most ~112px (the edit-dialog preview), so 256 is ample. Decoding is
// budgeted per frame (s_avatarLoads*) so opening a large library/contact list doesn't stall the UI —
// callers show a placeholder and the image fills in over the next frames.
struct AvatarTex { ImTextureID tex = 0; int w = 0, h = 0; }; struct AvatarTex { ImTextureID tex = 0; int w = 0, h = 0; };
static std::unordered_map<std::string, AvatarTex> s_avatarTexCache; static std::unordered_map<std::string, AvatarTex> s_avatarTexCache;
static int s_avatarLoadsThisFrame = 0;
static constexpr int kAvatarTexMax = 256;
static constexpr int kAvatarLoadsPerFrame = 3;
static const AvatarTex* getAvatarTexture(const std::string& path) { static const AvatarTex* getAvatarTexture(const std::string& path) {
auto it = s_avatarTexCache.find(path); auto it = s_avatarTexCache.find(path);
if (it != s_avatarTexCache.end()) return it->second.tex ? &it->second : nullptr; if (it != s_avatarTexCache.end()) return it->second.tex ? &it->second : nullptr;
if (s_avatarLoadsThisFrame >= kAvatarLoadsPerFrame) return nullptr; // defer to a later frame
s_avatarLoadsThisFrame++;
AvatarTex at; AvatarTex at;
util::LoadTextureFromFile(path.c_str(), &at.tex, &at.w, &at.h); // tex stays 0 on failure int sw = 0, sh = 0;
unsigned char* raw = util::LoadRawPixelsFromFile(path.c_str(), &sw, &sh);
if (raw && sw > 0 && sh > 0) {
int dw = sw, dh = sh;
if (sw > kAvatarTexMax || sh > kAvatarTexMax) {
float s = (float)kAvatarTexMax / (float)std::max(sw, sh);
dw = std::max(1, (int)(sw * s));
dh = std::max(1, (int)(sh * s));
}
std::vector<unsigned char> small((size_t)dw * dh * 4);
for (int y = 0; y < dh; y++) {
int sy0 = y * sh / dh, sy1 = std::max(sy0 + 1, (y + 1) * sh / dh);
for (int x = 0; x < dw; x++) {
int sx0 = x * sw / dw, sx1 = std::max(sx0 + 1, (x + 1) * sw / dw);
uint32_t r=0,g=0,b=0,a=0,cnt=0;
for (int yy = sy0; yy < sy1; yy++)
for (int xx = sx0; xx < sx1; xx++) {
const unsigned char* p = raw + ((size_t)yy * sw + xx) * 4;
r+=p[0]; g+=p[1]; b+=p[2]; a+=p[3]; ++cnt;
}
unsigned char* d = small.data() + ((size_t)y * dw + x) * 4;
d[0]=(unsigned char)(r/cnt); d[1]=(unsigned char)(g/cnt); d[2]=(unsigned char)(b/cnt); d[3]=(unsigned char)(a/cnt);
}
}
if (util::CreateRawTexture(small.data(), dw, dh, false, &at.tex)) { at.w = dw; at.h = dh; }
}
if (raw) util::FreeRawPixels(raw);
auto& slot = (s_avatarTexCache[path] = at); auto& slot = (s_avatarTexCache[path] = at);
return slot.tex ? &slot : nullptr; return slot.tex ? &slot : nullptr;
} }
@@ -205,6 +241,7 @@ static void deleteAvatarLibraryFile(const std::string& path) {
void RenderContactsTab(App* app) void RenderContactsTab(App* app)
{ {
s_avatarLoadsThisFrame = 0; // reset the per-frame avatar-decode budget (see getAvatarTexture)
auto& S = schema::UI(); auto& S = schema::UI();
// Reuse the existing address-book schema/column config for the table + add/edit form. // Reuse the existing address-book schema/column config for the table + add/edit form.
auto addrTable = S.table("dialogs.address-book", "address-table"); auto addrTable = S.table("dialogs.address-book", "address-table");
@@ -636,27 +673,31 @@ void RenderContactsTab(App* app)
} }
if (sel) gdl->AddRect(mn, mx, material::WithAlpha(material::Primary(), 230), 6.0f * dp, 0, 2.5f * dp); if (sel) gdl->AddRect(mn, mx, material::WithAlpha(material::Primary(), 230), 6.0f * dp, 0, 2.5f * dp);
else if (hov) gdl->AddRect(mn, mx, material::WithAlpha(material::OnSurface(), 100), 6.0f * dp, 0, 1.5f * dp); else if (hov) gdl->AddRect(mn, mx, material::WithAlpha(material::OnSurface(), 100), 6.0f * dp, 0, 1.5f * dp);
// The delete badge is hit-tested MANUALLY (not an ImGui item) so it never moves the
// layout cursor — an InvisibleButton here would leave CursorPosPrevLine at the badge
// corner and jitter the rest of the row's cells via the next SameLine.
float dr = std::max(7.0f * dp, cell * 0.15f);
ImVec2 dcc(mx.x - dr - 3.0f * dp, mn.y + dr + 3.0f * dp);
bool dhov = ImGui::IsMouseHoveringRect(ImVec2(dcc.x-dr, dcc.y-dr), ImVec2(dcc.x+dr, dcc.y+dr));
ImGui::PushID(n); ImGui::PushID(n);
ImGui::SetNextItemAllowOverlap(); // let the delete badge take clicks over the thumb bool thumbClicked = ImGui::InvisibleButton("##avthumb", ImVec2(cell, cell));
if (ImGui::InvisibleButton("##avthumb", ImVec2(cell, cell))) s_edit_avatar = "img:" + path;
bool thumbHov = ImGui::IsItemHovered(); bool thumbHov = ImGui::IsItemHovered();
if (thumbHov) ImGui::SetMouseCursor(ImGuiMouseCursor_Hand); ImGui::PopID();
ImVec2 afterCell = ImGui::GetCursorScreenPos(); if (thumbHov || sel || hov) { // draw the delete badge
if (thumbHov || sel || hov) {
float dr = std::max(7.0f * dp, cell * 0.15f);
ImVec2 dcc(mx.x - dr - 3.0f * dp, mn.y + dr + 3.0f * dp);
bool dhov = ImGui::IsMouseHoveringRect(ImVec2(dcc.x-dr,dcc.y-dr), ImVec2(dcc.x+dr,dcc.y+dr));
gdl->AddCircleFilled(dcc, dr, dhov ? material::ReadableError() : IM_COL32(0, 0, 0, 175)); gdl->AddCircleFilled(dcc, dr, dhov ? material::ReadableError() : IM_COL32(0, 0, 0, 175));
ImFont* xf = material::Type().iconSmall(); ImFont* xf = material::Type().iconSmall();
float xsz = dr * 1.35f; float xsz = dr * 1.35f;
ImVec2 xs = xf->CalcTextSizeA(xsz, FLT_MAX, 0, ICON_MD_CLOSE); ImVec2 xs = xf->CalcTextSizeA(xsz, FLT_MAX, 0, ICON_MD_CLOSE);
gdl->AddText(xf, xsz, ImVec2(dcc.x - xs.x*0.5f, dcc.y - xs.y*0.5f), IM_COL32(255,255,255,235), ICON_MD_CLOSE); gdl->AddText(xf, xsz, ImVec2(dcc.x - xs.x*0.5f, dcc.y - xs.y*0.5f), IM_COL32(255,255,255,235), ICON_MD_CLOSE);
ImGui::SetCursorScreenPos(ImVec2(dcc.x - dr, dcc.y - dr));
if (ImGui::InvisibleButton("##avdel", ImVec2(dr*2, dr*2))) pendingDelete = n - 1;
if (ImGui::IsItemHovered()) { ImGui::SetMouseCursor(ImGuiMouseCursor_Hand); material::Tooltip("%s", TR("delete")); }
} }
ImGui::SetCursorScreenPos(afterCell); if (dhov) { // badge takes priority over selecting the thumbnail
ImGui::PopID(); ImGui::SetMouseCursor(ImGuiMouseCursor_Hand);
material::Tooltip("%s", TR("delete"));
if (ImGui::IsMouseClicked(ImGuiMouseButton_Left)) pendingDelete = n - 1;
} else {
if (thumbHov) ImGui::SetMouseCursor(ImGuiMouseCursor_Hand);
if (thumbClicked) s_edit_avatar = "img:" + path;
}
} }
col = (col + 1) % cols; col = (col + 1) % cols;
} }