RazrFalcon / tiny-skia

A tiny Skia subset ported to Rust
BSD 3-Clause "New" or "Revised" License
1.12k stars 69 forks source link

Optimize clipping #9

Open RazrFalcon opened 3 years ago

RazrFalcon commented 3 years ago

Currently, we're using just a simple alpha mask, unlike Skia, which has a very complicated clipping algorithm. It's like 5000 LOC. But this algorithms is always faster that just a mask and uses less memory.

rvolgers commented 3 years ago

These blog posts have some interesting details on Skia's complex clipping, and what can go wrong with it:

https://googleprojectzero.blogspot.com/2018/07/drawing-outside-box-precision-issues-in.html

https://googleprojectzero.blogspot.com/2019/02/the-curious-case-of-convexity-confusion.html

The main focus of these posts is on in-depth discussion of vulnerabilities. But the general discussion of the algorithms and awareness of past problems might be useful. May need to Ctrl+F "skia" and/or "clipping" to get to the good bits.

RazrFalcon commented 3 years ago

Since this is mostly a port, we would get all those fixes for free. But yes, it was an interesting insight. I didn't know there are blog posts about Skia by the developers itself.

As for the security, it shouldn't be an issue, since in tiny-skia pixels access is checked.

rvolgers commented 3 years ago

As for the security, it shouldn't be an issue, since in tiny-skia pixels access is checked.

My main reason for posting these links is so the risk can be taken into account in future development decisions, such as keeping pixel access checked or not, or implementing complex clipping.

If you were already planning to keep all pixel access checked, perhaps it will help others understand that direction.

RazrFalcon commented 3 years ago

Sure. I just wanted to clarify that I'm trying to stick to Skia as close as possible, because basically each line is an edge-case.

notgull commented 9 months ago

I just ran into this in https://src.notgull.net/notgull/theo/issues/1. At the moment I'm using the clip mask in order to compute the clip mask for my hardware rendered backends as well (as it was simpler to implement). However, using the mask slows the application to a crawl; from 60 FPS to 20 FPS or lower.

RazrFalcon commented 9 months ago

Yes, we should at least add the rectangle clip shortcut.