Stop the constant repaint that made the left-edge artefacts permanent

The artefacts went from occasional to continuous, and the cause was my own
mitigation for them.

lv_obj_set_local_style_prop() does NOT skip identical values -- it calls
lv_obj_refresh_style() unconditionally, which invalidates the object. A
comment in ui_set_stale() claimed the opposite and that assumption spread.
ui_update_thermostat() is called once a second by the control task and was
writing seven style properties every time, so the panel was redrawing
something forever, and glitched lines were being regenerated as fast as
anything could clear them.

Two full-screen invalidates made it worse still: one after every HA poll,
one 600ms after every touch. The premise was that LVGL never reinvalidates
pixels it does not know are wrong, so repainting would clean them up. But
the README already had the answer -- the corruption scales with how much is
written, because LVGL's PSRAM writes compete with the panel's DMA reads out
of the same PSRAM. A full repaint is 768KB of exactly the contention that
causes the problem. Both are removed.

What is left is the thing that actually helps: write less.

  - ui_update_thermostat() returns without taking the lock when nothing
    moved, and otherwise touches only the widgets whose values changed.
  - ui_set_stale() is guarded on the previous value.
  - Condition and trend icons only get a colour write when the glyph itself
    changed.

Also raises the bounce buffer from 16 to 20 scanlines. The comment saying
20 "fails to allocate once WiFi is up" was true when display_init() ran
after wifi_init_sta(); it does not any more. display_alloc() takes the
buffers from a virgin heap with ~200KB free, so the extra 12.8KB is
affordable, and each larger chunk buys the refill ISR more time to fetch the
next one before the LCD needs it. That attacks the mechanism rather than the
symptom. ~24KB internal heap remains once WiFi is up; the "Free internal
heap" log line is the thing to watch if it needs to come back down.

Both gotchas are now in the README so the next person does not reach for a
repaint again.

Co-Authored-By: Claude <noreply@anthropic.com>

README.md

@@ -231,9 +231,17 @@ the buffer being displayed — copying into the live framebuffer races the LCD's
 own reads, and the resulting corrupt pixels persist indefinitely because LVGL
 does not know they are wrong and never reinvalidates them.
 
-A 16-scanline bounce buffer stages each chunk in internal RAM. 20 scanlines
+A **20**-scanline bounce buffer stages each chunk in internal RAM (64KB across
-would be better but needs 64KB of internal RAM and fails to allocate once
+two buffers). This was 16 for a long time because 20 "fails to allocate once
-WiFi is up.
+WiFi is up" — which was true when `display_init()` ran after
+`wifi_init_sta()`. The two-phase split changed that: `display_alloc()` now
+takes these buffers from a virgin heap with ~200KB free, long before WiFi has
+fragmented it. Bigger chunks give the refill ISR more time to fetch the next
+one before the LCD needs it, which attacks the artefact mechanism directly.
+
+That leaves ~24KB of internal heap once everything is up. Watch the
+"Free internal heap" log line — if it approaches single digits, come back
+down to 18 or 16 scanlines.
 
 ### Known limitation: occasional left-edge artefacts
 
@@ -283,6 +291,20 @@ about PSRAM bandwidth or panel timing.
 
 ## Gotchas
 
+- **Setting a style property always invalidates, even to an identical value.**
+  `lv_obj_set_local_style_prop()` calls `lv_obj_refresh_style()`
+  unconditionally — there is no equality short-circuit, unlike the one you
+  might assume by analogy with `label_set_if_changed()`. A periodic update
+  that restyles widgets "harmlessly" is a periodic full repaint of them.
+  This cost a round of debugging: `ui_update_thermostat()` ran at 1Hz writing
+  seven styles, which turned the left-edge artefacts from occasional into
+  permanent. Every update path here is now change-guarded.
+- **Do not repaint to fix artefacts.** It is tempting, because LVGL never
+  reinvalidates pixels it does not know are wrong. But the corruption scales
+  with how much is written, so a full-screen invalidate is 768KB of exactly
+  the PSRAM contention that causes it. Two attempts at this (post-poll and a
+  post-interaction "repair" timer) both made it visibly worse and were
+  removed. Redraw *less*, not more.
 - **LVGL does not bubble events to parents.** An event handler on the screen
   object never sees taps that land on a child widget, which looks exactly like
   dead touch zones. Light tiles therefore carry their own handlers, and the

TODO.md

@@ -27,13 +27,12 @@ no errors), but eyes and fingers haven't confirmed:
 2. Trend arrows appear ~5 minutes after boot (indoor big card, outdoor and
    pressure cells) once the local ring has history. They are local-history
    based and reset on reboot — that is by design, not a bug.
-3. Artefact lifetime: interactions schedule a one-shot full repaint 600ms
+3. Artefact lifetime. Two "repaint to clean up" mitigations were tried and
-   after the last touch, and every poll repaints too. Artefacts should now
+   both made it worse -- see README Gotchas. They are gone, the 1Hz restyle
-   visibly self-clean in under a second after touching, and within a poll
+   that made the artefacts permanent is fixed, and the bounce buffer is up to
-   otherwise. If they *persist* beyond that, the repaint isn't reaching
+   20 scanlines. If lines still come and go, the remaining honest levers are
-   them; if they merely *recur* during interaction, that is the PSRAM
+   fewer redraws still, less WiFi interrupt load during rendering, or 120MHz
-   bandwidth ceiling (README) and the honest remaining levers are fewer
+   PSRAM on a newer IDF.
-   redraws or 120MHz PSRAM on a newer IDF.
 
 ## Done
 

main/drivers/display/display_rgb.c

@@ -80,9 +80,22 @@ static const char *TAG = "display_rgb";
  * displays whatever was left in the buffer, showing up as short stale lines
  * at the left edge where each chunk begins.
  */
-/* 16 is the practical ceiling: two buffers at 20 lines need 64KB of internal
+/*
- * RAM and the allocation fails once WiFi is up. */
+ * 20 lines: two buffers at 800px x 2 bytes = 64KB of internal RAM.
-#define BOUNCE_BUFFER_LINES 16
+ *
+ * This used to be 16 because 20 "fails to allocate once WiFi is up" -- true
+ * when display_init() ran after wifi_init_sta(). It no longer does: the
+ * two-phase split means display_alloc() takes these buffers from a virgin
+ * heap before WiFi has touched it (~200KB free at that point), so the extra
+ * 12.8KB is affordable now.
+ *
+ * Bigger directly attacks the artefact mechanism: each chunk buys the refill
+ * ISR that much more time to fetch the next one out of PSRAM before the LCD
+ * needs it. Going further eats the internal RAM WiFi wants at runtime --
+ * watch the "Free internal heap" line, and if it drops near single digits,
+ * come back down.
+ */
+#define BOUNCE_BUFFER_LINES 20
 
 static esp_lcd_panel_handle_t panel = NULL;
 static lv_display_t *disp = NULL;

main/main.c

@@ -289,20 +289,19 @@ static void ha_task(void *arg)
         lvgl_port_resume();
 
         /*
-         * Repaint everything after every fetch window.
+         * No full-screen invalidate here, deliberately.
          *
-         * The known left-edge artefacts (see README) happen when WiFi
+         * There used to be one, on the theory that repainting would clean up
-         * interrupt load starves the bounce-refill ISR, and once bad pixels
+         * glitched pixels LVGL does not know are wrong. It made things
-         * are on screen LVGL has no idea they are wrong -- it never
+         * materially worse, and the README already said why: the corruption
-         * reinvalidates them, so they sit there indefinitely. A full
+         * scales with how much is written, because LVGL's writes into PSRAM
-         * invalidate after each poll bounds their lifetime at one poll
+         * compete with the panel's own DMA reads out of it. A full repaint is
-         * interval. Mitigation, not cure: it cannot prevent the glitch,
+         * 768KB of exactly that contention -- the largest possible dose of
-         * only clean up after it.
+         * the cause, applied as a cure.
+         *
+         * Redraw as little as possible is the actual lever. See ui_update*(),
+         * which now write nothing when nothing changed.
          */
-        if (lvgl_port_lock(100)) {
-            lv_obj_invalidate(lv_screen_active());
-            lvgl_port_unlock();
-        }
 
         poll_count++;
 

main/ui/ui_manager.c

@@ -195,33 +195,20 @@ static bool slider_touched[HA_LIGHT_COUNT];
 static int32_t slider_sent_pct[HA_LIGHT_COUNT];
 
 /*
- * One-shot "repair" repaint, rescheduled by every interaction to fire 600ms
+ * There is deliberately no "repair" repaint here any more.
- * after the last one.
  *
- * Redrawing is itself what glitches the panel: the flush's PSRAM writes
+ * An earlier version scheduled a full-screen invalidate 600ms after each
- * contend with the LCD's scan-out, and a burst of interactive redraws is the
+ * interaction, on the theory that repainting would clear glitched pixels
- * most likely moment for the bounce ISR to lose (README, PSRAM bandwidth).
+ * LVGL does not know are wrong. It made the artefacts worse and constant
- * The glitched lines then persist because LVGL believes those pixels are
+ * rather than occasional, for the reason the README already gave: the
- * fine. One full repaint after the burst settles cleans them up within
+ * corruption scales with how much is written, because LVGL's PSRAM writes
- * ~0.6s instead of leaving them until the next poll's repaint, up to 60s
+ * compete with the panel's DMA reads out of the same PSRAM. Repainting to
- * away. It runs once per burst, not periodically -- a constant repaint loop
+ * fix corruption caused by repainting does not converge.
- * would be a constant supply of fresh glitch opportunities.
+ *
+ * The lever that actually works is writing less. Everything below is
+ * change-guarded: if a value has not moved, no style is written and nothing
+ * is invalidated.
  */
-static lv_timer_t *repair_timer;
-
-static void repair_timer_cb(lv_timer_t *t)
-{
-    lv_obj_invalidate(lv_screen_active());
-    lv_timer_pause(t);
-}
-
-static void schedule_repair(void)
-{
-    if (repair_timer != NULL) {
-        lv_timer_reset(repair_timer);
-        lv_timer_resume(repair_timer);
-    }
-}
 
 /* --------------------------------------------------------- condition icons */
 
@@ -348,18 +335,24 @@ static void trend_render(lv_obj_t *icon, ha_trend_t t)
         return;
     }
 
-    switch (t) {
+    /*
-    case HA_TREND_RISING:
+     * The colour write is skipped when the glyph did not change. Style writes
-        label_set_if_changed(icon, ICON_TREND_UP);
+     * invalidate unconditionally, so an unguarded one here would repaint
+     * three arrows on every poll to redraw the same pixels.
+     */
+    const char *glyph = (t == HA_TREND_RISING)  ? ICON_TREND_UP
+                      : (t == HA_TREND_FALLING) ? ICON_TREND_DOWN
+                                                : "";
+    const char *current = lv_label_get_text(icon);
+    if (current != NULL && strcmp(current, glyph) == 0) {
+        return;
+    }
+
+    lv_label_set_text(icon, glyph);
+    if (t == HA_TREND_RISING) {
         lv_obj_set_style_text_color(icon, lv_color_hex(COL_ACCENT_WARM), LV_PART_MAIN);
-        break;
+    } else if (t == HA_TREND_FALLING) {
-    case HA_TREND_FALLING:
-        label_set_if_changed(icon, ICON_TREND_DOWN);
         lv_obj_set_style_text_color(icon, lv_color_hex(COL_ACCENT), LV_PART_MAIN);
-        break;
-    default:
-        label_set_if_changed(icon, "");
-        break;
     }
 }
 
@@ -409,7 +402,6 @@ static void therm_button_clicked(lv_event_t *e)
     if (therm_cb != NULL) {
         therm_cb(cmd);
     }
-    schedule_repair();
 }
 
 /* ------------------------------------------------------------ light tiles */
@@ -469,7 +461,6 @@ static void light_tile_clicked(lv_event_t *e)
                           : -1,
     };
     light_apply_optimistic(idx, expect);
-    schedule_repair();
 
     /* -1: a tap is on/off only. Brightness is the slider's job, and sending
      * the current brightness here would fight a user who dimmed the light
@@ -525,7 +516,6 @@ static void light_slider_changed(lv_event_t *e)
                                 lv_color_hex(pct > 0 ? COL_ON : COL_TEXT_DIM),
                                 LV_PART_MAIN);
 
-    schedule_repair();
 
     if (light_cb == NULL || !lv_obj_has_state(slider, LV_STATE_PRESSED)) {
         return;
@@ -552,7 +542,6 @@ static void light_slider_released(lv_event_t *e)
     const bool touched = slider_touched[idx];
     slider_touched[idx] = false;
 
-    schedule_repair();
 
     if (touched && pct == slider_sent_pct[idx]) {
         return;   /* tap without drag: the press command already said this */
@@ -1070,10 +1059,13 @@ static void update_forecast(const ha_data_t *d)
 
         if (icon_font_26 != NULL) {
             const cond_icon_t *ci = cond_icon_lookup(f->condition);
-            label_set_if_changed(fc_icon[i], ci->icon);
+            const char *current = lv_label_get_text(fc_icon[i]);
+            if (current == NULL || strcmp(current, ci->icon) != 0) {
+                lv_label_set_text(fc_icon[i], ci->icon);
                 lv_obj_set_style_text_color(fc_icon[i], lv_color_hex(ci->colour),
                                             LV_PART_MAIN);
             }
+        }
 
         if (f->high_f.valid && f->low_f.valid) {
             snprintf(buf, sizeof(buf), "%d / %d",
@@ -1105,9 +1097,13 @@ void ui_update(const ha_data_t *data)
     if (icon_font_26 != NULL) {
         const cond_icon_t *ci = cond_icon_lookup(
             data->condition[0] ? data->condition : NULL);
-        label_set_if_changed(cond_icon, ci->icon);
+        /* Colour only when the glyph moved -- see trend_render(). */
+        const char *current = lv_label_get_text(cond_icon);
+        if (current == NULL || strcmp(current, ci->icon) != 0) {
+            lv_label_set_text(cond_icon, ci->icon);
             lv_obj_set_style_text_color(cond_icon, lv_color_hex(ci->colour), LV_PART_MAIN);
         }
+    }
     /* The label's width changed with its words; re-hang it off the icon. */
     lv_obj_align_to(cond_label, cond_icon, LV_ALIGN_OUT_LEFT_MID, -8, 0);
 
@@ -1138,19 +1134,34 @@ void ui_update(const ha_data_t *data)
 
 void ui_set_stale(bool stale)
 {
+    /*
+     * Guarded, because setting an identical style value is NOT free --
+     * lv_obj_set_local_style_prop() calls lv_obj_refresh_style()
+     * unconditionally and invalidates the object. An earlier comment here
+     * claimed the opposite and was wrong; called every poll, it repainted the
+     * temperature for no reason.
+     */
+    static bool last_stale;
+    static bool last_stale_valid;
+
+    if (last_stale_valid && last_stale == stale) {
+        return;
+    }
+
     if (!lvgl_port_lock(UI_LOCK_TIMEOUT_MS)) {
         ESP_LOGW(TAG, "Dropped stale-state update: LVGL lock timed out");
         return;
     }
 
     if (temp_label != NULL) {
-        /* Unlike lv_label_set_text(), setting an identical style value does
-         * not invalidate, so this is safe to call on every poll. */
         lv_obj_set_style_text_color(temp_label,
                                     lv_color_hex(stale ? COL_STALE : COL_TEXT),
                                     LV_PART_MAIN);
     }
 
+    last_stale = stale;
+    last_stale_valid = true;
+
     lvgl_port_unlock();
 }
 
@@ -1180,12 +1191,37 @@ void ui_set_therm_cb(ui_therm_cmd_cb_t cb)
     therm_cb = cb;
 }
 
+/*
+ * Previous state pushed to the thermostat card. This is not an optimisation,
+ * it is the fix for a real bug: this function is called once a second by the
+ * control task, and lv_obj_set_local_style_prop() does NOT skip identical
+ * values -- it calls lv_obj_refresh_style() unconditionally, which
+ * invalidates. Writing seven styles per second was seven redraws per second
+ * forever, which is what turned the left-edge artefacts from occasional into
+ * permanent.
+ */
+static therm_status_t last_therm;
+static bool last_therm_valid;
+
 void ui_update_thermostat(const therm_status_t *st)
 {
     if (st == NULL) {
         return;
     }
 
+    const bool first = !last_therm_valid;
+
+    /* The common case by far: nothing has moved, so touch nothing and do not
+     * even take the lock. */
+    if (!first &&
+        st->state == last_therm.state &&
+        st->mode == last_therm.mode &&
+        st->fan == last_therm.fan &&
+        st->lockout_remaining_s == last_therm.lockout_remaining_s &&
+        (int)lroundf(st->setpoint_f) == (int)lroundf(last_therm.setpoint_f)) {
+        return;
+    }
+
     if (!lvgl_port_lock(UI_LOCK_TIMEOUT_MS)) {
         ESP_LOGW(TAG, "Dropped thermostat update: LVGL lock timed out");
         return;
@@ -1193,16 +1229,35 @@ void ui_update_thermostat(const therm_status_t *st)
 
     char buf[32];
 
+    const bool heat_mode = (st->mode == THERM_MODE_HEAT);
+    const bool mode_changed = first || st->mode != last_therm.mode;
+
+    if (first || mode_changed ||
+        (int)lroundf(st->setpoint_f) != (int)lroundf(last_therm.setpoint_f)) {
         snprintf(buf, sizeof(buf), "%d°", (int)lroundf(st->setpoint_f));
         label_set_if_changed(setpoint_label, buf);
 
+        /* Greys out when the mode is off -- it still adjusts, it just is not
+         * controlling anything. */
+        lv_obj_set_style_text_color(setpoint_label,
+                                    lv_color_hex(heat_mode ? COL_ACCENT_WARM
+                                                           : COL_TEXT_FAINT),
+                                    LV_PART_MAIN);
+    }
+
+    /* Text changes every second while a lockout counts down, but it is one
+     * small label rather than the whole card. */
+    if (first || st->state != last_therm.state ||
+        st->lockout_remaining_s != last_therm.lockout_remaining_s) {
         thermostat_state_text(st, buf, sizeof(buf));
         label_set_if_changed(therm_state_label, buf);
+    }
 
+    if (first || st->state != last_therm.state) {
         /*
-     * The dot colour is the at-a-glance channel: amber means the burner is
+         * The dot colour is the at-a-glance channel: amber means the burner
-     * lit, red means something is wrong and no heat is coming, dim means
+         * is lit, warm orange means it wants to run but a protection timer
-     * everything is fine and there is nothing to do.
+         * says not yet, red means no heat is coming at all.
          */
         uint32_t dot = COL_TEXT_FAINT;
         uint32_t text = COL_TEXT_DIM;
@@ -1224,8 +1279,9 @@ void ui_update_thermostat(const therm_status_t *st)
         }
         lv_obj_set_style_bg_color(therm_state_dot, lv_color_hex(dot), LV_PART_MAIN);
         lv_obj_set_style_text_color(therm_state_label, lv_color_hex(text), LV_PART_MAIN);
+    }
 
-    const bool heat_mode = (st->mode == THERM_MODE_HEAT);
+    if (mode_changed) {
         label_set_if_changed(mode_btn_label, heat_mode ? "Heat" : "Off");
         lv_obj_set_style_bg_color(mode_btn,
                                   lv_color_hex(heat_mode ? COL_ACCENT_WARM : COL_PANEL_HI),
@@ -1233,7 +1289,9 @@ void ui_update_thermostat(const therm_status_t *st)
         lv_obj_set_style_text_color(mode_btn_label,
                                     lv_color_hex(heat_mode ? 0x2A1A05 : COL_TEXT_DIM),
                                     LV_PART_MAIN);
+    }
 
+    if (first || st->fan != last_therm.fan) {
         const bool fan_on = (st->fan == THERM_FAN_ON);
         label_set_if_changed(fan_btn_label, fan_on ? "Fan on" : "Fan auto");
         lv_obj_set_style_bg_color(fan_btn,
@@ -1242,12 +1300,10 @@ void ui_update_thermostat(const therm_status_t *st)
         lv_obj_set_style_text_color(fan_btn_label,
                                     lv_color_hex(fan_on ? 0x05131F : COL_TEXT),
                                     LV_PART_MAIN);
+    }
 
-    /* Setpoint greys out when the mode is off -- it still adjusts, it just
+    last_therm = *st;
-     * is not controlling anything. */
+    last_therm_valid = true;
-    lv_obj_set_style_text_color(setpoint_label,
-                                lv_color_hex(heat_mode ? COL_ACCENT_WARM : COL_TEXT_FAINT),
-                                LV_PART_MAIN);
 
     lvgl_port_unlock();
 }
@@ -1280,12 +1336,6 @@ void ui_init(void)
     lv_obj_set_flex_flow(scr, LV_FLEX_FLOW_COLUMN);
     lv_obj_remove_flag(scr, LV_OBJ_FLAG_SCROLLABLE);
 
-    /* Created paused; interactions reset-and-resume it. See schedule_repair. */
-    repair_timer = lv_timer_create(repair_timer_cb, 600, NULL);
-    if (repair_timer != NULL) {
-        lv_timer_pause(repair_timer);
-    }
-
     build_header(scr);
     build_body(scr);
     build_lights(scr);