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

Consider using gfx-rs for rendering #407

Closed
kvark opened this issue Sep 15, 2016 · 13 comments
Closed

Consider using gfx-rs for rendering #407

kvark opened this issue Sep 15, 2016 · 13 comments

Comments

@kvark
Copy link
Member

@kvark kvark commented Sep 15, 2016

GFX has been around for quite a long time, and is still actively developed. We have OpenGL/GLES, DX11 backends working with Metal finishing up and Vulkan in the works. Our programming model uses command buffers, allowing WebRender to parallelize. Surely, we could benefit from some cooperation and avoid re-implementing all the backand logic for different graphics APIs.

@samlh
Copy link

@samlh samlh commented Sep 16, 2016

I agree alternate backends may be useful, especially for D3D.

It looks like most of the gl specific code is in renderer.rs, device.rs, and (of course) webgl.rs. In addition, there are the shaders in the resource directory that would need to be ported.

Alas, I think adding an abstraction layer wouldn't help much, and may hurt. A lot of the perf work I've been seeing recently in Webrender has been in reducing the complexity of the code, and using the underlying apis in ways that avoid triggering driver perf or correctness bugs. Gfx-rs is comparable in size to the entirety of webrender. The less code used, the fewer places inefficiencies or bugs may hide. Webrender has so little OpenGl code in total that I think switching to gfx-rs would not be a good use of time, and would likely introduce inefficiency.

However, I am entirely unaffiliated with both projects, and thus have no say in the matter :).

Edited: added a sentence and rephrased a couple others.

@glennw
Copy link
Member

@glennw glennw commented Oct 12, 2016

gfx-rs does look pretty awesome, nice work! I haven't spent a huge amount of time looking at the exact APIs, so please forgive my ignorance on the details of gfx-rs.

I will admit to being wary of using libraries that attempt to abstract the differences between rendering APIs to a common interface. I'll try to explain why below :)

In WR, the majority of the code itself is not GL calls. In fact, the core GL rendering loop code, minus boilerplate, is only a couple of hundred lines of code. However, the rest of the code that doesn't call GL and is preparing the data for submission to GL is very tailored towards the capabilities of the rendering API. For example, the way that culling, batching, texture atlas allocation, render target management, threading etc is handled is platform independent code, but is highly specific to the capabilities of the base GL3 API.

If / when we implement a Vulkan backend, we can therefore do it one of two ways. Either (1) maintain the current platform independent code and tack on a Vulkan backend or (2) modify the culling, batching, texture atlas etc code to match the capabilities of the Vulkan API in an optimal way. To give a concrete example, the draw call overhead differences between Vulkan and GL (assuming no AZDO, see below) would have a large effect on the batching strategy that would perform best, as well as selecting things like where it makes sense to use ubershaders. Equally, the differences in texture binding capabilities would have a large effect on the best way to manage texture atlases and related functionality.

In my experience, following path (1) would work but ends up with very non-optimal performance for some APIs, whereas (2) is more work but produces a much better result in the end. This is, of course, a generalization, but it's something I've run into many times previously, when trying to provide a simple abstraction at the rendering API level rather than the application level. Again, forgive my ignorance of gfx-rs here - but I guess the question is what the design strategy is for impedance mismatches between the gfx-rs APIs and the underlying rendering APIs, and does this imply a performance impact on some rendering backends that must emulate certain functionality (e.g. threading with OpenGL)?

A second question concerns code paths within a particular rendering backend. My strategy right now is to use minimal optional features of each rendering API. For example, WR uses GL3 as a base, but doesn't use any GL extensions that aren't core functionality (which is why AZDO isn't feasible right now). The theory is that I'd rather lose a (hopefully small) amount of performance if that means drastically reducing the number of WR/driver code paths that get hit. I theorize (without data to back this up) that doing this means less QA testing is required due to having fewer code paths, and that perhaps we will hit fewer driver bugs in practice. What is the gfx-rs design philosophy on use of extensions / optional code paths within a single rendering backend?

Thanks for opening the issue, I'm certainly interesting in trying to share rendering backend code if we can arrive at a solution that is mutually beneficial for both projects :)

@kvark
Copy link
Member Author

@kvark kvark commented Oct 14, 2016

@samlh Thanks for your input! I'd be interested to see what kind of driver perf/correctness overhead WR team has been dealing with. Maybe we can address them? Maybe we already do :)

Gfx-rs is comparable in size to the entirety of webrender

This is inaccurate. Gfx-rs (core + render + GL backend) is about half the size of webrender.

@glennw Thanks for your detailed answer!

re: GL3 API capabilities, Vulkan plans, and more:

In my experience, following path (1) would work but ends up with very non-optimal performance for some APIs, whereas (2) is more work but produces a much better result in the end.
... what the design strategy is for impedance mismatches between the gfx-rs APIs and the underlying rendering APIs ...?

Absolutely! That's why GFX is not based on GL3 capabilities, and instead mimics DX12 in a lot of aspects. Please check out our programming model if you haven't already.

does this imply a performance impact on some rendering backends that must emulate certain functionality (e.g. threading with OpenGL)?

Good question! GL is the only backend with no native support for command buffers (in the ARB/core, at least), so here come the differences:

  • we emulate the command buffers, which is definitely an overhead! Some of the earlier benchmarks (from 2 years ago) showed no difference though, but I bet we could find some if we looked with more precision.
  • since we defer the actual GL calls up until the command buffer is complete, we are theoretically giving the driver less time to prepare the work for GPU. Again, this is not registered in practice (drivers don't seem to do much until swap_buffers), but may probably show up on some vendors/drivers under certain workloads.
  • the state caching is done during the c-buf construction (not submission), so it can happen on a different thread (not the main thread) and can be executed simultaneously with other c-buf constructors/submissions (re: your GL threading concern). This can be a speed-up if your main thread is a bottleneck.

WR uses GL3 as a base, but doesn't use any GL extensions that aren't core functionality
The theory is that I'd rather lose a (hopefully small) amount of performance if that means drastically reducing the number of WR/driver code paths that get hit
What is the gfx-rs design philosophy on use of extensions / optional code paths within a single rendering backend?

We actually follow the same logic in Gfx-rs. The only non-core API we use is GL_EXT_texture_filter_anisotropic, which is not needed for WR. We also use some of the ARB extensions when available, namely:

  • GL_ARB_draw_buffers_blend, when running on a pre-GL-3.2 context (like 2.1 or 3.1)
  • GL_ARB_program_interface_query for querying the fragment shader outputs properly (one of my pet peeves of GL3)

Thanks for opening the issue, I'm certainly interesting in trying to share rendering backend code if we can arrive at a solution that is mutually beneficial for both projects :)

I figured it wouldn't hurt to spark a discussion about it. If we find some common ground - awesome! If not, we'll definitely get some new ideas for the Gfx-rs improvement. Well, and increase the awareness of our project as a side-effect :)

Let me help you a bit. Here are the reasons why you should not use Gfx-rs:

  1. Programming model! It's very different (and more complex) from WR's current GL implementation.
    • counter-point: you'd otherwise need to go through an even more complex transformation for implementing the Vulkan backend.
  2. Gfx-rs is still incomplete:
    • GL backend needs caching actually implemented (gfx-rs/gfx#978, counter-point: this is rather straightforward)
    • Vulkan & Metal backends still have the sync problem of handling multiple command buffers in HW queue (gfx-rs/gfx#1051). This may need an API change.
    • Vulkan backend is not ready to even run any examples yet (counter-point: neither is WR's)
  3. Gfx-rs may not be as efficient as your own implementation (of either GL or Vulkan) due to being more generic. You may be able to cut a few corners for WR specific workloads, simplify resource management, etc.
    • counter-point: aside from the GL differences listed above, there is not much that we do. There is no GPU performance overhead. And on CPU side, there is just Arc handling of resources.
  4. It's another dependency, a community-supported library that you'd need to deal with.

So, I hope you'll find time for a closer look into Gfx-rs, and give us some feedback (much appreciated!). There is no rush here - we still have major things to do :)

@samlh
Copy link

@samlh samlh commented Oct 15, 2016

I'd be interested to see what kind of driver perf/correctness overhead WR team has been dealing with.

Just a sampling of what I was referring to - I don't know how relevant this is to the discussion, however. :)
Driver limitations: #410, #403, #393, #385
Reducing cpu usage by specializing for the api: #390

This is inaccurate. Gfx-rs (core + render + GL backend) is about half the size of webrender.

Thanks for the correction!

Edit: typo

bors-servo added a commit that referenced this issue Oct 27, 2016
Isolate GL calls to the device only

Also used by the WebGL module, but cutting that tie off would be a much bigger task.
This PR makes the code a bit cleaner, also paves the way to #407.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/480)
<!-- Reviewable:end -->
@kvark
Copy link
Member Author

@kvark kvark commented Dec 24, 2016

Related - gfx-rs/gfx#1102

@glennw
Copy link
Member

@glennw glennw commented Jan 25, 2018

@kvark should we keep this open? Is there anything actionable in here for WR?

@kvark
Copy link
Member Author

@kvark kvark commented Jan 25, 2018

This has been considered, twice - to be precise, and is happening as we speak ;)

@kvark kvark closed this Jan 25, 2018
@Kerollmops
Copy link

@Kerollmops Kerollmops commented Jan 29, 2018

You say it's happening now but where is the related issue or PR that I can track the change ? please 😄

@kvark
Copy link
Member Author

@kvark kvark commented Jan 29, 2018

@Kerollmops this is the active branch - https://github.com/szeged/webrender/tree/gfx_hal
We'll setup a Waffle board or something for easier tracking.

@Kerollmops
Copy link

@Kerollmops Kerollmops commented Jan 29, 2018

Ho ! I searched for a branch named gfx or something on the main fork and I missed it obviously ! Thank you !

@kvark
Copy link
Member Author

@kvark kvark commented Feb 1, 2018

https://waffle.io/szeged/webrender

Current progress is super promising 🤞

@erlend-sh
Copy link

@erlend-sh erlend-sh commented Nov 21, 2018

The Amethyst team is considering using the WebRender-based GUI framework Azul by @fschutt to build the Amethyst Editor, but that effort is largely blocked on this at the moment.

Looks like @dati91 and @zakorgy are the ones driving this forward, thanks a lot! 👏 Since it seems like you’re doing this work as a part of your university studies, any additional information about your mandate and roadmap would be greatly appreciated. I ask because certain university projects can come to an abrupt end (at no fault to the students of course!), so any team considering building graphics-intensive applications on top of WebRender at this point is very excited to know if the szeged fork has an end-game in mind ;)

@dati91
Copy link
Contributor

@dati91 dati91 commented Nov 22, 2018

Hi @erlend-sh!

Yes, we are - together with @zakorgy, @loki04 and @kvark - supporting this project but not as student.

About WebRender: Our fork is near completion, regarding correctness. There are still a lot of opportunity to improve performance.
Our Vulkan and DX12 backend's wrench results are matches the original WR. Our Metal backend still has some minor differences, and we are working on these as well.

About this issue (changing the backend): This one is not that trivial. Our work is focusing on keeping the original OpenGL backend and also adding gfx, making the backend optional at build time.
Merging this, or even consider to merge, is up to the webrender community.
But this would make their work a lot harder at the moment, because it is not just used in Servo as originally intended. It also ships with Gecko now, and that part can be problematic, since that one is not so experimental.

About roadmap: We are about to make this project as a complete one, and we see lots of opportunities to improve. But at first we are targeting the full support of the back-ends on the available platforms. Than we could move to the more interesting parts, e.g. optimizations. I guess the upcoming Mozilla's All Hands highlights more about the roadmap.

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

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.