Attempt to remove performance drops on very long single lines - #945
Attempt to remove performance drops on very long single lines#945foxnne wants to merge 4 commits into
Conversation
|
Generally the code looks good! I like the recent font shortcut and the ascii kerning table.
Not sure I'm totally understanding how to use |
Sorry, I had implemented using this in my local copy of TextEntryWidget as it also has code I had added for highlighting culling and other things, which used these ranges to avoid highlighting non-visible ranges. So currently I hadn't added it to DVUI's own TextEntryWidget. Perhaps a more complete PR here would be adding this not just for myself but trying to fold in relevant changes to DVUI's TextEntryWidget rather than maintaining my own copy, I just haven't diligently been tracking exactly what I've changed in local copies of widgets.
It's one byte span with holes punched in it, rather than a list of visible lines. We've already vertically culled using Please let me know if you'd like to see fizzy's code for this, its in a PR branch currently and not on main. |
Yes please point me to that, maybe seeing the usage code will help me.
The test to add: if (self.insert_pt.x <= clip_logical.x + clip_logical.w and self.insert_pt.x + s.w >= clip_logical.x) {
self.visible_ranges.add(.{ .start = self.bytes_seen, .end = self.bytes_seen + end });
}If I'm reading that right it means "if any of the current span of text is visible, add it". If there are 10 medium length lines (meaning each one runs off the visible area a decent amount), then we'd get a visible range added for each line but a gap between each. That defeats the coalescing into the previous visible range? |
|
Thinking about this - we could try something like "the client code asks if the current insert point is off the right end of the clipping rect". If it is, then the client code can skip the rest of the line. I don't think that's a full solution, but an easy thing to do today that solves at least the performance when the user is looking at the start of the very long line. |
|
Sorry I wasn't around for a bit, I want to try to find a solution for this that can work for everyone, or at least solve the immediate issues right now. One thing I was thinking is that it would be common for exported JSON to be a single very long line, and editing that line (moving the caret to the right) would then be back to paying full cost for this line. I added a couple tests to try to catch the main issues this is trying to solve:
Please let me know if you'd rather scrap this! |
This is a great point. Sorry I'm running behind on this, and unfortunately I'm unlikely to have time to dig into it for another week. Hang in there, this is certainly something I want to fix! Sorry if I missed it, can you point me to code that uses the VisibleRanges? I'm still not understanding the whole problem. Thanks! |
No worries or rush at all! And I'm totally open to any other ideas or fixes, as I would defer to you or others with more text knowledge than I have. I purely just wanted to get something in to handle the performance issues with long lines in the interim.
fn highlightRanges(self: *TextEntryWidget, buf: *[max_highlight_ranges]ByteRange) []const ByteRange {
const range = self.highlightByteRange() orelse return &.{};
const visible = self.textLayout.visibleBytesLastFrame();
if (visible.len == 0) {
buf[0] = range;
return buf[0..1];
}
var n: usize = 0;
for (visible) |vis| {
const headroom = @max(2 * (vis.end -| vis.start), 4096);
const start = @max(range.start, vis.start -| headroom);
const end = @min(range.end, vis.end +| headroom);
if (end <= start) continue;
// Padding can make neighbouring runs meet; two passes over one span would emit the same
// captures twice, which the emit loop reads as overlapping matches and drops.
if (n > 0 and start <= buf[n - 1].end) {
buf[n - 1].end = @max(buf[n - 1].end, end);
} else {
buf[n] = .{ .start = start, .end = end };
n += 1;
}
}
return buf[0..n];
}pub fn visibleBytesLastFrame(self: *const TextLayoutWidget) []const ByteRange {
if (self.visible_ranges_last) |*vr| return vr.slice();
return &.{};
}So we just track these so we can ask "what bytes were visible last frame?" |
Hi!
This PR exists to try to tackle performance issues using TextLayoutWidget with very long single lines. Until now, it seems the performance work was handled per-line, so very long single lines (such as unformatted JSON, etc) end up causing performance issues quickly.
This PR implements two attempts to increase performance:
Below are some benchmark results through fizzy:
ReleaseFast,
zig build bench-text(on fizzy), ms/frame (lower is better).200,000-char line (204 KB file)
Scaling with line length (idle, ms/frame)
Please let me know if you'd like me to remove comments or make any other changes, or if this is a wrong approach, I initially just tried to make things clearer to understand.