Widget.refresh Method Not Cropping To Region Start Correctly

[ddkasa]

The Bug

When supplying explicit regions with the Widget.refresh command Textual does not crop the region correctly. It will crop the end of the region correctly, but refresh everything before that instead of cropping to the start.

Video

Current

region_refresh.webm

Expected

expected_refresh.webm

MRE

import itertools

from rich.segment import Segment
from textual.app import App, ComposeResult
from textual.geometry import Region
from textual.strip import Strip
from textual.widget import Widget

CHARS = itertools.cycle(["x", "o", ".", ",", "/", "-", "|", "\\"])

class LineWidget(Widget):
    def refresh(self, *regions, repaint=True, layout=False, recompose=False):
        self._char = next(CHARS)
        return super().refresh(
            *regions, repaint=repaint, layout=layout, recompose=recompose
        )

    def render_line(self, y):
        return Strip([Segment(self._char * self.size.width)])

    def redraw_square(self) -> None:
        self.refresh(Region(40, 10, 5, 5))

class TestApp(App[None]):
    def compose(self) -> ComposeResult:
        yield LineWidget()

    def on_mount(self) -> None:
        self.set_interval(1 / 10, self.refresh_widget)

    def refresh_widget(self) -> None:
        self.query_one(LineWidget).redraw_square()


TestApp().run()

Possible Cause/Solution

Through some exploration of the _compositor.py module, I find that the ChopsUpdate.render_segments method is not cropping up to the start of the region, but to the start of the Strip instead.

textual/src/textual/_compositor.py

Lines 266 to 268 in dd36b69

	strip = strip.crop(0, min(end, x2) - x)
	append(move_to(x, y).segment.text)
	append(strip.render(console))

Changing the above to the code below leads to the expected outcome.

strip = strip.crop(x1, min(end, x2) - x)
append(move_to(x1, y).segment.text)
append(strip.render(console))

A side note is that the Rich console repr has a todo comment leftover in it which mentions it, so it might have been forgotten in the ChopsUpdate.render_segments method as well.

textual/src/textual/_compositor.py

Line 198 in 732c6b9

# TODO: crop to x extents

Accounting for the previous Compositor logic, this step makes most sense in terms of the problem source. Might be wrong though, so getting extra pairs of eyes on this would great.

Textual Diagnostics

Versions

Name	Value
Textual	2.1.2
Rich	13.9.4

Python

Name	Value
Version	3.13.1
Implementation	CPython
Compiler	Clang 19.1.6
Executable	/home/dk/dev/tmp/textual-sandbox/.venv/bin/python3

Operating System

Name	Value
System	Linux
Release	6.13.4-100.fc40.x86_64
Version	#1 SMP PREEMPT_DYNAMIC Sun Feb 23 15:15:27 UTC 2025

Terminal

Name	Value
Terminal Application	Kitty
TERM	xterm-kitty
COLORTERM	truecolor
FORCE_COLOR	Not set
NO_COLOR	Not set

Rich Console options

Name	Value
size	width=274, height=54
legacy_windows	False
min_width	1
max_width	274
is_terminal	True
encoding	utf-8
max_height	54
justify	None
overflow	None
no_wrap	False
highlight	None
markup	None
height	None

[github-actions]

Thank you for your issue. Give us a little time to review it.

PS. You might want to check the FAQ if you haven't done so already.

This is an automated reply, generated by FAQtory

[willmcgugan]

You should consider this to be an implementation detail. When you add a region to refresh, you are informing Textual which regions have changed. You aren't saying exactly which characters to update. It's up to Textual to decide which segments / characters to build in to the next update.

This means that your render_line should always return the up-to-date state of your widget. i.e. it should never make assumptions about how it is called.

The currently implementation does update more than is needed. It is generally more efficient to do this, as it requires less cropping of segments (which is expensive), and it avoids issues where double width characters may be broken in half (which doesn't render well). So updating more on the screen can be a performance win.

This may change in the future. As long as your render_line returns an up-to-date Strip it won't matter how Textual turns this in to an update.

[davidfokkema]

Hi @willmcgugan, this came up in the context of https://github.com/davidfokkema/textual-hires-canvas. It works, but is not very fast. The Line API introduction reads:

Textual offers an alternative API which reduces the amount of work required to refresh a widget, and makes it possible to update portions of a widget (as small as a single character) without a full redraw.

So we were hoping to implement that. But it appears that even in the case of Segments containing only single characters, Textual refuses to update only the Region that was passed to refresh. With the proposed change there is no change in the number of calls to crop() or the number of iterations in the loops, only the data that is passed on to the terminal is reduced. Would that still be more expensive?

Phrased another way: is it, or is it currently not, possible to update single characters in the middle of the widget?

[davidfokkema]

...but I see that in the case of textual-plot, which uses textual-hires-canvas, that particular section of code is hardly ever executed. So changing it might not get us anything it seems. I'll have to dig deeper.

[willmcgugan]

It is possible in a sense. Rather than write the segments up to the place we want to update, Textual could simply move the cursor up to the start. That would reduce the amount of data sent to the terminal, and may be something worth doing. Although reducing the amount of data sent to the terminal is unlikely to have a material impact on performance here.

Looking at your render_line, I see it is constructing a Segment per cell. Constructing many objects can be slow. Additionally with fresh instances you negate the benefits of various caches within Rich and Textual.

Let me suggest a different approach which I think will be faster. Rather than two 2D arrays, store the canvas as essentially a 2D array of Segment instances. Something like the following:

BLANK = Segment(" ", Style())

lines = [[BLANK] * width for y in range(height)]

Note that creates multiple references to the same blank segment, which will help caching.

Your render_line would essentially just return one of these lines in a Strip, doing negligible work in the process.

When you set a "pixel" you would update a single segment, and call refresh with a region that covers that segment.

Bonus point if you cache those lines. So if render_line is called for a y that has been previously calculated it returns a cached strip. This should give you much bigger wins than the more accurate cropping.

BTW I'm loving the work you are doing on textual-hires-canvas. It's a terrific addition!

[davidfokkema]

Thanks! I'll set up a proper performance testing application to work on that. Right now I find it hard to interpret pyinstrument output from run to run because I run the app for a bit and then quit it and get only wall clock times, but the app did not render the same number of frames.

It is possible in a sense. Rather than write the segments up to the place we want to update, Textual could simply move the cursor up to the start. That would reduce the amount of data sent to the terminal, and may be something worth doing.

I think the code @ddkasa posted above implements just that. After some testing, and maybe tweaking it a bit, that could be turned into a PR. So if you think it's worth doing, although it wouldn't help textual-hires-canvas much, we could proceed with a PR. If you think it's not worth it, you can close the issue, ;-).

[willmcgugan]

I think the code @ddkasa posted above implements just that. After some testing, and maybe tweaking it a bit, that could be turned into a PR. So if you think it's worth doing, although it wouldn't help textual-hires-canvas much, we could proceed with a PR. If you think it's not worth it, you can close the issue, ;-).

It's worth doing in principle. Conceptually that's exactly what we need to do, but crop is inefficient and there is a gotcha with double width characters. @ddkasa you couldn't have know this at the time!

I'll leave this issue open, to tackle when I get back. Although I'm happy to elaborate on the solution I have in mind, if either of you want to give it a go...

[davidfokkema]

Sure, if that saves you time, we can look at it in more detail! No rush, though.