From 6ac8f66644508e1dda6c42a816d28a452dbf1aa6 Mon Sep 17 00:00:00 2001 From: DanS Date: Wed, 15 Jul 2026 10:33:51 -0500 Subject: [PATCH] fix(contacts): review fixes for the avatar library grid MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- src/ui/windows/contacts_tab.cpp | 69 ++++++++++++++++++++++++++------- 1 file changed, 55 insertions(+), 14 deletions(-) diff --git a/src/ui/windows/contacts_tab.cpp b/src/ui/windows/contacts_tab.cpp index 8552559..a2ba0e1 100644 --- a/src/ui/windows/contacts_tab.cpp +++ b/src/ui/windows/contacts_tab.cpp @@ -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). +// 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; }; static std::unordered_map 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) { auto it = s_avatarTexCache.find(path); 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; - 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 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); return slot.tex ? &slot : nullptr; } @@ -205,6 +241,7 @@ static void deleteAvatarLibraryFile(const std::string& path) { void RenderContactsTab(App* app) { + s_avatarLoadsThisFrame = 0; // reset the per-frame avatar-decode budget (see getAvatarTexture) auto& S = schema::UI(); // Reuse the existing address-book schema/column config for the table + add/edit form. 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); 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::SetNextItemAllowOverlap(); // let the delete badge take clicks over the thumb - if (ImGui::InvisibleButton("##avthumb", ImVec2(cell, cell))) s_edit_avatar = "img:" + path; + bool thumbClicked = ImGui::InvisibleButton("##avthumb", ImVec2(cell, cell)); bool thumbHov = ImGui::IsItemHovered(); - if (thumbHov) ImGui::SetMouseCursor(ImGuiMouseCursor_Hand); - ImVec2 afterCell = ImGui::GetCursorScreenPos(); - 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)); + ImGui::PopID(); + if (thumbHov || sel || hov) { // draw the delete badge gdl->AddCircleFilled(dcc, dr, dhov ? material::ReadableError() : IM_COL32(0, 0, 0, 175)); ImFont* xf = material::Type().iconSmall(); float xsz = dr * 1.35f; 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); - 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); - ImGui::PopID(); + if (dhov) { // badge takes priority over selecting the thumbnail + 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; }