Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

text selection rendering glitch #4

Closed
lunixbochs opened this issue Jan 12, 2023 · 9 comments
Closed

text selection rendering glitch #4

lunixbochs opened this issue Jan 12, 2023 · 9 comments

Comments

@lunixbochs
Copy link

lunixbochs commented Jan 12, 2023

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.

Screen.Recording.2023-01-11.at.9.44.49.PM.mov
@lucasmerlin
Copy link
Owner

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
Copy link
Owner

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

@lunixbochs
Copy link
Author

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
Copy link
Owner

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
Copy link
Author

lunixbochs commented Jan 12, 2023

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
Copy link
Owner

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
Copy link
Author

lunixbochs commented Jan 12, 2023

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
Copy link
Owner

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
Copy link
Owner

Closing this since egui 0.21 is released now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants