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

CPU rendering is a little off #1

Closed
daniel5151 opened this issue Aug 27, 2022 · 8 comments
Closed

CPU rendering is a little off #1

daniel5151 opened this issue Aug 27, 2022 · 8 comments

Comments

@daniel5151
Copy link

daniel5151 commented Aug 27, 2022

Hey there!

I know you're already aware of this issue (it's even mentioned in the README), but I thought I'd open a tracking issue regardless so I could express my interest in seeing this fixed.

I just managed to integrate egui_skia in a project of mine (https://github.com/daniel5151/wide-libretro), and I'm seeing the same rendering issues as in examples/cpu.rs.

Thanks again for implementing this! Even in its current somewhat broken state, it should be sufficient to start doing what I wanted to do :)

Capture

@lucasmerlin
Copy link
Owner

Hi, thanks for opening this issue! I think this might actually be a problem with skia itself, since it works perfectly fine with a gpu backend.

I'll try to make a minimal reproduction of this and open an issue with skia once I get back from holidays.

@lucasmerlin
Copy link
Owner

I think I nailed down where the issue is caused:
So egui is using the following font texture:
Managed(0)
In the top left corner is a tiny 1px white dot. When drawing a shape without texture (e.g. a window background), egui uses the texture coordinates 0.0,0.0 which should select the color from the white dot. For some reason that's not working correctly on the cpu (maybe a floating point error or something like that?).

I was able to create a skia fiddle that shows this issue: https://fiddle.skia.org/c/114a0ba43dadecad43a6630105d7b5c8

And I created a skia issue: https://bugs.chromium.org/p/skia/issues/detail?id=13706

@lucasmerlin
Copy link
Owner

Hi! I made a workaround and published version 0.1.1. This adds a cpu_fix feature that adds a workaround for the skia problem. Can you confirm this also fixes it for you @daniel5151?

@daniel5151
Copy link
Author

Wow, would you look at that! Awesome!

Capture

Admittedly, it does seem like there is quite a perf hit to enabling the feature (especially after opening a bunch of panels). That's totally fine for use-cases though.

Thanks again!

@lucasmerlin
Copy link
Owner

Nice! I tried to improve the performance of the splitting function I wrote for the fix, but I'm not sure if it made a huge difference. I think the main problem is that skia has to render a lot more single Meshes now, not sure if there is a workaround that does not need so many new Meshes. You can try the improvements on the improve-raster-performance branch, let me know if this improves it for you!

@daniel5151
Copy link
Author

Hmm, i'll be honest, I don't notice too much of an improvement with that new code...

I wonder if you can eek out more perf by reducing the number of allocations you're doing? Maybe by recycling allocated Vecs between frames when possible?

@lucasmerlin
Copy link
Owner

I implemented a different workaround now, and it turns out, it's not the workaround that slows things down. Rather the bug itself probably causes skia to skip some expensive rendering work. In the improve-raster-performance branch, you can try changing line 291 to 293 in painter.rs from 0.5 to 0.0 and the bug should reappear and things should speed up significantly, while nothing else changed.

So unfortunately I don't think there is an easy way to speed this up on the cpu, and also I don't expect it to speed up even once the underlying skia bug has been fixed.

I'll close this issue now since the graphical problems have been fixed.

@daniel5151
Copy link
Author

Makes sense. Thanks for digging into this!

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