dandri/pyxis
1
0 Fork 0
mirror of https://github.com/torlando-tech/pyxis.git synced 2026-10-22 22:11:26 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

fix(nomadnet): partial refresh keeps real alt text for region images

Browse Source 
Round-3 Greptile P1 (Partial Images Lose Alt Text): the round-2 fix
dropped fragment image records, so a refreshed-region image could only
render its fixed '[Image: loading]' run instead of its real alt text.

Reference 1.4.3 semantics: a partial refresh never fetches images
(detect_images runs only at full page load), but the refreshed region is
re-parsed and its images show their real alt text. Implementation:
  - region-owned base records (referenced by the region's image blocks)
    are discarded each refresh, keeping the record list bounded;
  - all other base records are copied verbatim (stable compact indices,
    so decoded images keep rendering);
  - fragment records are appended with an offset remap, so a
    refreshed-region image renders its real alt text and is never
    fetched (no loader entry exists for it).

Also fixes a latent kept-offset bug the new two-refresh test exposed:
the fragment offset used the base record total instead of the kept
count (wrong whenever a region-owned record was discarded).

Test: native partial-core now asserts real alt text after a refresh,
a bounded count across two refreshes (1 retained + 2 region), and stable
retained indices; verified failing against the prior build.
This commit is contained in:
Torlando committed 2026-09-22 00:26:24 +00:00
1 parent 2d26df32ff
commit 705358dcc6
2 files changed
+125 -39
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

lib/tdeck_ui/UI/LXMF/NomadNetCompactPage.cpp
+92 -19
View File
@@ -4,6 +4,7 @@
#include <algorithm>
#include <cstring>
#include <limits>
#include <vector>
namespace UI::LXMF::NomadNet {
@@ -463,19 +464,46 @@ PartialReplaceResult CompactPage::assign_replacing_partial(
// Image records are stable identity: the screen's decode slots and
// the loader entries are both keyed by the compact image index, and
// the loader is configured once at full page load. The reference
// detects images only at full page load — a partial refresh never
// scans (or loads) images, neither in the retained body nor in the
// refreshed region — so the record list is copied verbatim from the
// base: same records, same order, same indices, and a constant
// count across repeated refreshes (no accumulation, no remap).
// Fragment image blocks therefore carry no record (image_index -1)
// and render as alt-text placeholders until the next full load.
for (const auto& source : base._images) {
// the loader is configured once at full page load. A partial refresh
// never scans for or fetches images (reference: detect_images runs
// only at full page load; a refreshed region re-parses but is not
// image-detected), but the reference DOES show the region's real alt
// text for such images. So:
// - base records owned by the replaced region (referenced by its
// image blocks) are discarded — each refresh replaces them, so
// the list stays bounded (no accumulation across refreshes);
// - all other base records are copied verbatim (same order and
// indices, so a decoded image keeps rendering);
// - the fragment's records are appended with an offset remap, so
// a refreshed-region image renders its real alt text (and a
// decoded buffer, had one been stored) instead of a fixed
// "loading" string. They are never fetched (no loader entry).
auto image_in_region = [&](const CompactPage& page,
std::size_t source_index) {
if (source_index >= page._blocks.size()) return false;
const BlockRecord& b = page._blocks[source_index];
return b.type == BlockType::IMAGE && b.image_index >= 0 &&
b.partial_region_index == static_cast<int16_t>(partial_index);
};
std::vector<bool> region_owned;
{
bool bad = false;
try {
region_owned.assign(base._images.size(), false);
for (std::size_t i = 0; i < base._blocks.size(); ++i)
if (image_in_region(base, i))
region_owned[static_cast<std::size_t>(
base._blocks[i].image_index)] = true;
} catch (const std::bad_alloc&) { bad = true; }
if (bad) { clear(); return PartialReplaceResult::ALLOCATION_FAILED; }
}
for (std::size_t i = 0; i < base._images.size(); ++i) {
if (region_owned[i]) continue;
if (_images.size() >= MAX_IMAGES) {
clear();
return PartialReplaceResult::LIMIT_EXCEEDED;
}
const auto& source = base._images[i];
ImageRecord image;
if (!append_view(base.image_alt(source), image.alt_offset,
image.alt_length) ||
@@ -491,6 +519,43 @@ PartialReplaceResult CompactPage::assign_replacing_partial(
image.align = source.align;
_images.push_back(image);
}
for (const auto& source : fragment_page._images) {
if (_images.size() >= MAX_IMAGES) break; // the rest render via
// their run fallback
ImageRecord image;
if (!append_view(fragment_page.image_alt(source), image.alt_offset,
image.alt_length) ||
!append_view(fragment_page.image_url(source), image.url_offset,
image.url_length)) {
clear();
return PartialReplaceResult::LIMIT_EXCEEDED;
}
image.width_kind = source.width_kind;
image.height_kind = source.height_kind;
image.width_value = source.width_value;
image.height_value = source.height_value;
image.align = source.align;
_images.push_back(image);
}
// Image map: base record i -> its new index, or -1 when the record
// was region-owned and discarded. Fragment records follow the kept
// base records (offset = number kept, NOT the base total).
ExternalVector<int16_t> base_image_map;
base_image_map.assign(base._images.size(), -1);
std::size_t kept_base_images = 0;
{
bool bad = false;
try {
std::size_t next = 0;
for (std::size_t i = 0; i < base._images.size(); ++i) {
if (region_owned[i]) continue;
base_image_map[i] = static_cast<int16_t>(next);
++next;
}
kept_base_images = next;
} catch (const std::bad_alloc&) { bad = true; }
if (bad) { clear(); return PartialReplaceResult::ALLOCATION_FAILED; }
}
struct CopyMaps {
ExternalVector<int16_t> links;
@@ -660,16 +725,24 @@ PartialReplaceResult CompactPage::assign_replacing_partial(
block.partial_region_index = region_override >= 0
? region_override : old.partial_region_index;
if (old.image_index >= 0) {
// Base records were copied verbatim (identity), so a
// retained image keeps its compact index. Fragment images
// have no record in the new page (a partial refresh never
// loads images) and fall back to -1.
const bool from_base = (&source == &base);
const std::size_t mapped = from_base
? static_cast<std::size_t>(old.image_index)
: _images.size(); // out of range -> placeholder
block.image_index = (from_base && mapped < _images.size())
? static_cast<int16_t>(mapped) : -1;
// Base records survive verbatim (remapped through the kept
// base map; region-owned records were discarded). Fragment
// records follow the kept base records (offset remap), so a
// refreshed-region image renders its real alt text and is
// still never fetched (no loader entry exists for it).
int16_t mapped_index = -1;
if (&source == &base) {
mapped_index =
base_image_map[static_cast<std::size_t>(old.image_index)];
} else if (static_cast<std::size_t>(old.image_index) <
fragment_page._images.size() &&
kept_base_images +
static_cast<std::size_t>(old.image_index) <
_images.size()) {
mapped_index = static_cast<int16_t>(
kept_base_images + static_cast<std::size_t>(old.image_index));
}
block.image_index = mapped_index;
}
if (old.table_index >= 0) {
block.table_index = copy_table(source, maps, old.table_index);
tests/native/test_nomadnet_partial_core.cpp
+33 -20
View File
@@ -259,10 +259,13 @@ int main() {
std::string(preserved_form.fields()[0].value.data(),
preserved_form.fields()[0].value_length) == "user value");
// Image records must survive a partial refresh with stable identity:
// the base list is copied verbatim (same records, same indices), the
// count stays constant across repeated refreshes (no accumulation), and
// fragment images render as placeholders (image_index -1).
// Image records across a partial refresh (reference 1.4.3):
// - a partial refresh never fetches images, but the refreshed region's
// images keep their real alt text (records are replaced, not dropped);
// - region-owned records are discarded each refresh, so the list stays
// bounded (no accumulation across refreshes);
// - retained (non-region) images keep their compact index, so a
// decoded image keeps rendering.
const auto img_source = parser.parse(
R"(before
`(base img`a.webp)
@@ -279,12 +282,12 @@ after)");
CompactPage img_candidate;
const auto img_replace = img_candidate.assign_replacing_partial(
img_base, 0, img_fragment, CompactPage::MAX_ARENA_BYTES);
check("partial refresh preserves base image records and identity",
check("partial refresh replaces region image records (no accumulation)",
img_replace == PartialReplaceResult::APPLIED &&
img_candidate.images().size() == 1 &&
img_candidate.images().size() == 2 &&
img_candidate.has_image_blocks());
if (img_replace == PartialReplaceResult::APPLIED) {
// Base image: same compact index, same alt/url.
if (img_replace == PartialReplaceResult::APPLIED &&
img_candidate.images().size() == 2) {
const auto& base_image = img_candidate.blocks()[1];
check("retained image block keeps its compact image index",
base_image.type == BlockType::IMAGE &&
@@ -292,29 +295,39 @@ after)");
check("retained image record keeps its alt and url",
img_candidate.image_alt(img_candidate.images()[0]) == "base img" &&
img_candidate.image_url(img_candidate.images()[0]) == "a.webp");
// Fragment image: placeholder (no record, no fetch).
// Fragment image: region records replaced; real alt text present.
const auto& frag_image = img_candidate.blocks()[3];
check("fragment image renders as a placeholder block",
check("fragment image keeps its real alt text (no loading string)",
frag_image.type == BlockType::IMAGE &&
frag_image.image_index == -1);
// Second refresh: the candidate is the new base; the record list
// must not accumulate (constant count), and the base image keeps
// its index.
frag_image.image_index == 1 &&
img_candidate.image_alt(img_candidate.images()[1]) == "frag img" &&
img_candidate.image_url(img_candidate.images()[1]) == "b.webp");
// Second refresh (region now holds two images): the count reflects
// the region's NEW content, not the accumulated history, and the
// retained image still has index 0.
const auto img_fragment2 = parser.parse(
R"(v2
`(frag2 img`c.webp))");
`(frag2a`c.webp)
`(frag2b`d.webp))");
CompactPage img_candidate2;
const auto img_replace2 = img_candidate2.assign_replacing_partial(
img_candidate, 0, img_fragment2, CompactPage::MAX_ARENA_BYTES);
check("repeated partial refresh does not accumulate image records",
check("repeated partial refresh is bounded (1 retained + 2 region)",
img_replace2 == PartialReplaceResult::APPLIED &&
img_candidate2.images().size() == 1);
img_candidate2.images().size() == 3);
if (img_replace2 == PartialReplaceResult::APPLIED &&
img_candidate2.images().size() == 1) {
check("base image keeps its index after the second refresh",
img_candidate2.has_image_blocks() &&
img_candidate2.images().size() == 3) {
check("retained image keeps its index after the second refresh",
img_candidate2.blocks()[1].image_index == 0 &&
img_candidate2.image_alt(img_candidate2.images()[0]) ==
"base img");
check("second-refresh region images keep their real alt text",
img_candidate2.blocks()[3].image_index == 1 &&
img_candidate2.blocks()[4].image_index == 2 &&
img_candidate2.image_alt(img_candidate2.images()[1]) ==
"frag2a" &&
img_candidate2.image_alt(img_candidate2.images()[2]) ==
"frag2b");
}
}
const auto region_source = parser.parse(