(cherry picked from commit 348b45266ac2c2c65dcc26705dc7cf144f5e0051)
Details
Details
- Reviewers
gw - Commits
- rMOZILLACENTRAL269ab6499d32: Bug 1599327 - Use cluster scrollbar flags to select picture cache tile size.
rMOZILLACENTRAL2b3093f69e87: Bug 1599327 - Use cluster scrollbar flags to select picture cache tile size.
rMOZILLACENTRAL1bd9d4f431c4: Bug 1599327 - Use cluster scrollbar flags to select picture cache tile size. - Bugzilla Bug ID
- 1599327
Diff Detail
Diff Detail
- Repository
- rMOZILLACENTRAL mozilla-central
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Event Timeline
Comment Actions
Tested by confirming that the scrollbars currently don't seem to go into small tiles on a high-dpi laptop (the scrollbar is 34 pixels wide), and therefor when scrolling the 1024x512 tiles visibly subdivide into quadtrees. After the change, the scrollbar is clearly made up of 16-wide strips, and the large tiles no longer quad-divide.
Also added print statements and verified with a yaml file that adding scrollbar-container:true will trigger the new code path on the tile cache for the cluster containing the scrollbar item (after fixing https://bugzilla.mozilla.org/show_bug.cgi?id=1600712).
So overall it seems to do the job, but I'm not sure this is the best way to pass the flags around, or if it can be cleaner :)
Thanks!
| gfx/wr/webrender/src/picture.rs | ||
|---|---|---|
| 2095 | I'm keeping the old code to make sure that we don't regress on existing small tiles for small elements -- just make it smarter about scrollbars and nothing else. | |
| gfx/wr/webrender/src/scene_building.rs | ||
| 251 | There's a TODO here -- do I need this attribute? The other flags seem to have it, so... | |
Comment Actions
Generally looks good, nice work!
| gfx/wr/webrender/src/picture.rs | ||
|---|---|---|
| 2095 | I think we can remove those other branches. One of the goals of this change is that we only create small tiles for scrollbars, and all content gets a consistent tile size :) | |
| gfx/wr/webrender/src/scene_building.rs | ||
| 251 | It shouldn't be needed here. That annotation is only needed if it's part of a structure that also contains that annotation (which is used for auto serializing WR internal state as capture files). If you remove it and it is needed, it will show up as a compile error. | |
| 253 | For scrollbar pictures caches, we create them such that there are only scrollbar primitive(s) in it - so we could reword this to something like "is a scrollbar container" and IS_SCROLLBAR or similar. | |
| 656 | I think this might be tidier if we implement it in the if create_slice { branch above in this method - directly initializing the Slice type with the right value. I think that should still work. | |
Comment Actions
Thanks for the feedback Glen! I wasn't too sure if we could count on the fact that a scrollbar cluster is the only thing in a slice, but if it is then yes, this version is a bit cleaner :)
Cheers.
Comment Actions
I've confirmed with Glen that failing the test as a result of the change is harmless; changing the size of the tiles will produce slightly different UVs and might rasterize slightly differently.
The 7 failing pixels are off by 1 in their RGB values.
Generated a new reference image using headless.py on Linux.
Comment Actions
Re-landing after a fix by Glenn should now make this a legit optimization.
Compilation and perf try: https://bugzilla.mozilla.org/show_bug.cgi?id=1599327#c15