lucasmerlin / egui_skia

Render egui with skia!
MIT License
55 stars 13 forks source link

text selection rendering glitch #4

Closed lunixbochs closed 1 year ago

lunixbochs commented 1 year ago

If you start or end a multiline text selection on a blank line in a TextEdit control, the rendering of that text box glitches with this painter. I implemented my own skia painter doesn't use skia-safe, and it exhibits the same behavior, so I think it's something to work around (with the vertex layout?) rather than a bug in our respective code.

When this is triggered: On the OpenGL SDL and Metal Winit backends, both the text and the rectangle disappear. On the CPU backend, it manifests differently - the selection color disappears but the text doesn't disappear.

I printed out the vertex positions and it looks like there are NaN verts.

https://user-images.githubusercontent.com/117642/211988541-024f1e71-b097-4f47-ac4a-7388eab59155.mov

lucasmerlin commented 1 year ago

If there are NaN verts, wouldn't this be a problem with egui itself? Can you post en example?

Maybe we can open an issue in the egui repo. Or are NaN verts something that painters should be able to deal with?

lucasmerlin commented 1 year ago

I released 0.2.1, which should hopefully fix this issue. Can you try if this fixes it for you?

lunixbochs commented 1 year ago

Yep, zeroing NaN vertices seems to fix it in this case. I would be cautious about how this may potentially zero only 1-2 of the 3 verts of a triangle though, as that could produce a giant triangle with a 0,0 origin.

I attempted to fix this by iterating over the indices in chunks of 3 and removing triangles that contained a NaN, but that didn't work for some reason.

I suspect this is a bug in egui itself with 0-width rects, and maybe skia is stricter around NaN vertices than other gpu backends.

lucasmerlin commented 1 year ago

Yeah, I was also surprised that this didn't lead to any artifacts, but I assume this is similar to what happens in the other painters. I also tried to remove the NaN vertex but for me that caused a small triangular cutout to appear in the text selection.

I'll open an issue in the egui repo about this.

lunixbochs commented 1 year ago

I also tried to remove the NaN vertex but for me that caused a small triangular cutout to appear in the text selection.

This fails because this is an indexed draw call - triangles are constructed by each chunk of 3 indices, which can reference any vertex from the list they want. To remove a triangle, you need to remove all 3 indices, or the backend will read subsequent triangles from the wrong offset and have glitches. You also can't remove a vertex from the middle of the list or all of your indices pointing after that will point at the wrong vertices.


Ohhhhh! skia appears to reject the render for having a NaN in the vertices whether it is referenced by an index or not. So in theory we should prune those triangle indices and zero any NaN verts, then report this egui so epaint stops emitting NaN verts.

I wrote this to delete triangle indices with NaN verts (this needs to reference verts you haven't zeroed yet). In the empty-line text selection case, this is dropping exactly 10 triangles for me.

let indices: Vec<u16> = mesh.indices.chunks(3)
    .filter_map(|tri| {
        let isnan = tri.iter().any(|i| {
            let v = verts[*i as usize].pos;
            v.x.is_nan() || v.y.is_nan()
        });
        if isnan { None } else { Some(tri) }
    })
    .flatten()
    .cloned()
    .collect();
drop tri: [[NaN, NaN], [16.25, 280.0], [15.75, 300.0]]
drop tri: [[15.75, 300.0], [16.25, 280.0], [NaN, NaN]]
drop tri: [[16.25, 280.0], [NaN, NaN], [NaN, NaN]]
drop tri: [[NaN, NaN], [15.75, 280.0], [16.25, 280.0]]
drop tri: [[NaN, NaN], [16.25, 280.0], [15.75, 280.0]]
drop tri: [[15.75, 280.0], [NaN, NaN], [NaN, NaN]]
drop tri: [[15.75, 300.0], [NaN, NaN], [NaN, NaN]]
drop tri: [[NaN, NaN], [16.25, 300.0], [15.75, 300.0]]
drop tri: [[NaN, NaN], [15.75, 300.0], [16.25, 300.0]]
drop tri: [[16.25, 300.0], [NaN, NaN], [NaN, NaN]]
dropped 10 NaN triangles
lucasmerlin commented 1 year ago

This fails because this is an indexed draw call - triangles are constructed by each chunk of 3 indices, which can reference any vertex from the list they want. To remove a triangle, you need to remove all 3 indices, or the backend will read triangles from the wrong offset and have glitches. You also can't remove a vertex from the middle of the list or all of your indices pointing after that will point at the wrong vertices.

Makes sense!


I just tried to check if this still happens in the latest egui master version, and I couldn't reproduce it. So I assume they recently fixed this, or something about text selection changed, stopping this from happening.

lunixbochs commented 1 year ago

Yep, latest egui git seems to fix. We may want to leave a workaround in place as this seems to be a way skia deviates from egui's other renderers. Also looks like egui recently added pos.any_nan() as a shorthand for x.is_nan() || y.is_nan().

lucasmerlin commented 1 year ago

If we leave a workaround in place, the question is how the other renderers handles this (setting nan values to 0, skipping the indices referencing them, or something else). I think we can remove the workaround once the next egui version releases and maybe add a check and warning if we encounter NaN vertices in debug builds so we know what's wrong if something like this should happen again in the future.

lucasmerlin commented 1 year ago

Closing this since egui 0.21 is released now