-
Notifications
You must be signed in to change notification settings - Fork 7
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
Run Gecko with gfx-rs #198
Comments
Aaaaaaand, gecko with vulkan on linux: ( If you want to try it: also this is VERY experimental ;) |
Nice! I have some suggestions for the points you raised in the initial comment: it basically comes down to drilling backend-specific holes in the API.
I think you'll want Gecko to manage as much of this as possible, and only give WR the lowest-level, backend-specific object that you need. E.g. with GL, let Gecko make the context current, with Metal, let Gecko pass the Metal context to WR, etc.
There is no need to support arbitrary API combinations here. You can expect Gecko to give you external texture handles for the "native" API that you're targeting on that platform, and not support other types of handles. For example, on Windows, Gecko has a |
The result of
|
And also run the same revision with the original WR and got this:
I have no idea why it differ in 8834 tests.
So not sure what is going on, but will look into this. |
I have updated our WR and tried it with a newer version of gecko and started to take a look at the layout tests (the repo: https://github.com/zakorgy/gecko-dev/tree/gecko_wr_linux_vulkan), and compared it to the same version with the original WR. So here are the results: Original WR:
Our WR with Vulkan:
The sums of the numbers are equal, so this is good news. But the number of tests is less compared to the results from #198 (comment), which is weird. Maybe it's because we use the Here are the tests which differ: These fail with the original and passes with our WR, but I think it's worth to take a look at these:
These fail with our WR and passes with original WR:
This one crashes, because we have a maximum limit of
And there are some unexpected passes too:
|
I have checked the unexpected passes, and they are actual passes. |
I have checked the first section of different tests (fail with the original and passes with our WR), and found that the first two passes if I run them one by one. The third one (layout/reftests/bugs/383883-2.html) is a flaky test with both OpenGL and Vulkan. Since it has a minor pixel difference, and I didn't found any suspicious in the test result images, I will mark them as resolved. |
I'd also suggest doing a Gecko try push rather than running them locally. It's not uncommon for certain tests to behavior differently on local-vs-automation, and automation is the canonical environment as far as the expectations are concerned. |
It's both building and testing infrastructure. Of course we'd want to eventually run some of the graphics tests on them, but for now at least having Gecko built would be great. |
I wouldn't be surprised if our windows test machines had Vulkan drivers already. Windows Firefox currently runs on top of D3D, and @jdashg tells me that Vulkan drivers have been bundled with D3D drivers for a while now. |
... and if we had Windows10 build bots, we could even test consistently on D3D12 WARP device |
We have some news (good and bad) about the Mac/metal gecko. We finally managed to build it. @mstange thanks for the help. So this looks promising and from the logs it seems like its actually ours, but we are in the dark until the "black screen" issue is not resolved. (pun intended) |
The result of reftest-sanity:
But with the "black screen", this is just a preliminary result. |
It sounds like your GPU process crashed (after four crashes it falls back to basic compositor). You should be able to find the crash report in about:crashes. Unrelated FYI - I'm switching WR to use all immutable storage for textures, which may be relevant to the gfx-rs port. |
@bholley The about:crashes is an empty page with a line |
It could be related to popups(?). If I scroll a wikipedia page (without hitting anything with the cursor) it works fine. But if I hover a link, which will produce a "popup" (not a real window) on wikipedia, it just froze the page, restarts the WR and then draws that popup without a problem. Then if i scroll the page and try to hover another link, it will do the same: froze, restart, show the popup. (max 4 times ofc.) But even the original start page restarts our WR while loads it. So no cursor movement needed. |
@dati91 good guess! IIRC popups create separate GL contexts (and WR renderer/backend) that share resources (shaders for the most part atm) with the main context. We should make it work so that the |
@kvark @dati91 So I think there's actually three levels of "popups". Some "popups" (like the GitHub emoji selector that appears when you type ":") are just out-of-flow html elements. These are just part of the page, and shouldn't need any special handling for what you're doing here. There are also certain overlays drawn by the browser that get their own OS-level window. The smart location bar (AwesomeBar) is one of them. Tool-tips are another, which I think is what @dati91 is talking about when hovering links on wikipedia. You can prove that something gets its own OS-level window by resizing your browser and getting the popup to be rendered outside of the main browser window (you can see this for both the examples I mentioned above). In those cases, Firefox doesn't use WebRender at all, it just uses BasicCompositor/Skia. Finally, there's the case of top-level browser windows. Each one of these gets a separate WebRender instance, like @kvark describes. I believe we don't currently share GLContexts across WR instances on Mac, but per [1] I believe we are hoping to in [2]. [1] https://bugzilla.mozilla.org/show_bug.cgi?id=1495977#c9 |
Quick update on Windows/Vulkan:
Turns out this is the case, but the crash is in Vulkan's validation layer. In the VL, it runs into a nullptr for an image_view in a draw call, but I didn't find out why. I checked our image management, and we didn't free any image which was not submitted properly. And after turning it off everything looks fine. So maybe the problem is in the VL? But its not ready for reftesting just yet, because we are not quite drawing correctly on the second window: |
We finally managed to run the reftests on Windows with Vulkan with the following results:
A more verbose result is here. We should definitely double check these results, but if everything is alright, this looks very promising. Also I couldn't check with the original WR/gecko, because it doesn't draw correctly, not sure why. Maybe a gecko and/or WR update will solve it and we can compare the two versions. |
Finally got reftest results on MacOS with Metal:
Also we had to skip 5 tests which crash at the moment. The number of failing tests is pretty high, but when I run them one-by-one, in most cases they pass. This problem is probably related the sync issue from #189 which we still have to figure out. |
Just a note for the above comment: The 5 crashing test is related to external textures, and #230 fixes those crashes. |
Are you still getting 493 unexpected failures with the original WR? |
@kvark We are currently working on a wr/gfx/gecko update, because our current version is more than a month old. After that we can re-run every platform/backend and have the results updated.
Of course, we can make that happen.
I don't think that number is still accurate, after the update we can re-check that as well. |
In the meantime, we managed to run gecko with the DX12 backend. And here are the results:
Note: the pipeline creation is noticeably slower than any other backend, but after it's done everything works fine. |
Done -> #198 (comment) |
An update on what we are up to these days. Our current focus is to get gecko working on the treeherder. Here is a short summary: Some notes on the local builds: |
A quick update: About the MacOS/Metal build on treeherder: We managed to build it. And it seems the server doesn't have Metal framework for the reftests. (try job) |
What is the current status of this effort? It would be really useful for Firefox to drop OpenGL renderer and to switch to Vulkan, to avoid nasty bugs like this one. |
Are these benchmarks published anywhere? Would be interesting to see the progress. |
@shmerl |
Just wondering, have you tried the OpenGL backend of |
@lnicola We haven't tried the OpenGL backend of |
@lnicola fyi, running on gfx-rs GL backend is planned but not in a shape where it would tell you much yet. Moreover, even if it was ready today, it wouldn't give you the answers you are looking for. It would tell you how bad OpenGL matches Vulkan API, not how much overhead gfx-rs itself has. |
Is this due to how gfx-rs is using Vulkan, or due to anv issues? |
This is due to the fact gfx-rs exposes a Vulkan-ish API, and WebRender is getting re-shaped to take advantage of it. Trying to shove it back to the ancient OpenGL is not expected to be as good as the current GL path, it will be purely a fallback in case other APIs aren't available. |
How is the progress of this? Some quite annoying bugs in radeonsi that cause GPU hangs with OpenGL on Navi could really be nice to avoid if gfx-rs with Vulkan was an option for Firefox. |
@shmerl unfortunately, nobody has been able to work on this for the last 6 months. We are quite short on resources atm... The "szeged" group (who owns this repository/fork) is no longer contracted by Mozilla for WebRender work. Please reach out to me (on Matrix as "@kvark:matrix.org") if you want to help! |
I would also like to help work on this and find resources to supply. could we possibly chat on a gfx-rs Discord server? I'm naturallymitchell 3561 on the Rust Discord server. |
@naturallymitchell would you mind joining us in #gfx:matrix.org ? I tried to look for either gfx-rs Discord, or you on Rust Discord, and failed. |
Firefox runs on gecko right? Is this part of firefox already via some config option or is it still not considered stable enough to be added to config? |
This issue is for tracking the progress with gecko's reftests.
We have some reftest results to show (
layout/reftests
):Note: these are local results, not from the treeherder.
Some pictures with gecko:
https://drive.google.com/open?id=1rMilZCCpqRRHzN_D2L0VkXXn0ia8_KQ0
The text was updated successfully, but these errors were encountered: