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

Backend of rendy #13

Closed
torkleyy opened this issue Oct 31, 2018 · 21 comments
Closed

Backend of rendy #13

torkleyy opened this issue Oct 31, 2018 · 21 comments

Comments

@torkleyy
Copy link
Member

torkleyy commented Oct 31, 2018

This issue shall discuss which backend(s) rendy is going to use.

Discussion so far:

@kvark

What's the reasoning behind "Abandon hal for now."?


@omni-viral

@kvark without reinventing the wheel (abstraction layer over ash and hal) I'm way more productive.

Instead of providing own abstractions I believe it is better to implement ash::DeviceV* etc traits for hal. I heard that someone already suggesting to do so (or even started).
Then to work over hal rendy would only need to parametrize Factory type.


@Moxinilian

To the reader: please note that "dropping hal" does not mean that hal will not have anything to do with rendy. Viral is focusing on ash right now as it is closer to Vulkan so easier to use for him, but when all of this is done rendy is made in such a way that it will be "easy" to implement the hal backend. It's just to not have to implement two backends at once.


@kvark

@Moxinilian thank you for clarification. I don't see the story the same way. gfx-hal is already pretty much Vulkan, just a bit rustier (borrowing, move semantics, and references instead of pointers and opaque handles). You could just have used gfx-hal and have access to all platforms, while benefiting gfx-rs community with your feedback.

Truth is that @omni-viral found gfx-hal inconvenient for their needs, hence the departure. I don't see it temporary. The expressed solution (in #12 (comment)) is basically - solve it on the Ash level, so that Amethyst wouldn't care about gfx-rs APIs.

Tight Ash (and Vulkano) integration is certainly one of our goals, but it's not going to be the most performant path at the end of the day, so we'd like Rust projects which really need performance (like Amethyst and WebRender) to work with gfx-hal directly.


@Moxinilian

There is enough motivation within the Amethyst team to make sure hal will be supported by rendy, most notably in the perspective of WebGL (and eventually WebGPU). From what has been discussed on Discord, it seems like first class hal is planned in the future.

If I misunderstood, then this is going to be a much larger issue considering Amethyst's next step after mobile support will be web technologies, in which gfx-hal would help in a significant way.


@kvark

@Moxinilian FYI, this is a very poor communication on the Amethyst side. Your team discussed something on discord (which we don't even have any visibility into, since we use Gitter) and decided to drop gfx-hal without any heads up to our team, without even bothering to explain this in PR description... We hope that in the future, if there is any news of that scale, we'd know about it from you.


@Moxinilian

Yes, I agree. If I recall correctly, an issue is being written to be posted on the gfx issue tracker regarding the reasons for such a decision. I still believe that supporting hal from the ground up would have been better, but I am not involved enough in rendering to have a meaningful opinion.


@Moxinilian

We handled this very poorly, and apologize to the entire gfx-hal team. It was inexcusable, and we will take steps internally to ensure it doesn't happen again. We would like to re-open a dialog on this with the gfx-hal team, if they are ok with it.


@omni-viral

@kvark Sorry I didn't explained in detail why I abandoned hal in this PR. I also sad for that I didn't state my opinion clearly in gfx-rs/gfx#2206 😭

Correct me if I'm wrong.
As I understand it the main idea of hal API to stay is that it is rustier and reduces boxing overhead over non-Vulkan backends. But I also see calling overhead in hal's API for Vulkan backend which you underestimate (I think). Hence I'd really like to see ash's API implemented for hal's backends (notably metal backend which is awesome 👍).

Sorry. It's hard for me to express sophisticated stuff in language I barely know. It took me 15 minutes to write this comment 😅


Also pinging:

(please ignore if you're not interested)

  • EDIT 1: add forgotten comments
@kvark
Copy link
Member

kvark commented Oct 31, 2018

Worth mentioning that we are using Rendy in wgpu implementation, and now forced to look for a replacement (with one of the options - to refresh gfx-memory).

@omni-viral

But I also see calling overhead in hal's API for Vulkan backend which you underestimate (I think)

Let me provide some more details. HAL API is based on iterators, and Vulkan API needs slices of low-level objects. All that the Vulkan backend needs is basically collecting the necessary data in memory before issuing the low-level calls. This is just copying a few bits of data, it's not that much of an overhead. Moreover, our expectation is that the user doesn't have the data laid out sequentially anyway: they typically have their own wrappers carrying more accompanying data, and these may be in elaborate data structures, not just vectors. In this (expected) case there isn't any more work happening when going "user -> gfx-hal -> gfx-backend-vulkan" than the user would do with "user -> vulkan".

@torkleyy
Copy link
Member Author

torkleyy commented Oct 31, 2018

Please note: Amethyst people hardly discussed this backend issue so far. We haven't agreed on anything. It's generally problematic who owns rendy.. Just @omni-viral, the gfx project, Amethyst, nobody?

@dotellie
Copy link
Contributor

dotellie commented Oct 31, 2018

(Disclaimer, I may not be the most qualified to talk about this, but regardless...)

I think that we should continue to support both backends. The reasoning being that is since we want rendy to be Vulkan based, that's simply easier to accomplish by using a "pure" Vulkan API. However, I do think hal is the way forward eventually and getting feedback from what we find in doing rendy might prove to be beneficial to the gfx team in the long run. The problem here is that it's (very understandably) difficult for @omni-viral to figure out all the abstractions and keep the implementations up to date for both backends while still trying to be productive. Amethyst is long overdue for a renderer revamp and having our main developer of that new renderer not being able to efficiently iterate on it is a pretty bad scenario as you can probably imagine.

That's where my "proposal" comes in. I'm personally completely willing to put in the work required to keep both backends alive. They will both after all be needed eventually anyway. I think missing out on the cooperation between Amethyst and the gfx team would be very unfortunate so if @omni-viral is okay with it, I would like for us to keep the abstractions we have made and continue improving on them in order to support both ash and hal. I'm not completely sure how much this will help the productivity problem, but I think that this is regardless the best way forward for all parties.

@torkleyy
Copy link
Member Author

The reasoning being that since we want rendy to be Vulkan based, that's simply easier to accomplish by using a "pure" Vulkan API.

Can you (or Viral) elaborate on that?

@anderejd
Copy link

anderejd commented Nov 1, 2018

@torkleyy I just saw that I got pinged, thank you for the ping, I will try to make a useful braindump below.

The main thing that stands out to me from the side-lines is that I think tighter collaboration, more communication, between the Amethyst and gfx-rs team would be great for both projects and possibly for the Rust ecosystem as a whole.

I'm confident that you guys can produce amazing results if you establish more and better communication. I added a confused emoji to the original thread because it looked like you were dumping gfx-rs, which would be very unfortunate and would duplicate a massive amount of work. Please reconsider and leverage gfx-rs fully by collaborating tightly with the gfx-rs team. The rust ecosystem is quite small and fragmentation hurts productivity as well as quality.

Collaborate more, reinvent fewer wheels. ❤️

@zakarumych
Copy link
Member

First of all I'd like to state that for me PR is a form of proposal, not something cemented.

@kvark Rendy's memory crate in particular benefits almost nothing from using Ash instead of original abstract API because it has next to zero trade-offs. Even less since abstract API allows mock implementation for testing.
I think that the best solution here would be to make it use abstract API while using POD from existing crate. Probably ash because it is generated from vulkan spec xml.

If someone (@magnonellie) feels ready to support both crates I have no objections.
I just don't want rendy's abstractions become basically hal-ish abstraction layer owned by another team... Abstraction layers that don't add nothing are not worth writing.

Maybe the best course of actions would be to move hal closer to ash. Make it more flexible.
Make some types Copy. Make unsafe methods marked unsafe. Provide real documentation (at least in form of links). And even generate it 💡.

Considering iterators in API. Having concrete type in signature of a function you plan to call few thousand times per second encourages you to store data in that type and call a function without any conversions. Moving bits costs when you do it a lot.
I understand that each backend has its own concrete type and abstract as iterator makes sense. But since amethyst and rendy are mostly about games then focusing on vulkan backend first should make sense too.

I totally would like to discuss it further.

@torkleyy
Copy link
Member Author

torkleyy commented Nov 1, 2018

Is there any reason for using ash other than the potential performance advantage (why would we be able to gain a significant performance improvement by doing a part of the abstraction on our own instead of focusing on one layer?) and the documentation?

The step to use pure ash seems more like a convenient one to me than anything we want mid or long term. I'm not sure if it makes sense to delay (potentially necessary, idk) documentation work on hal to get rendy done sooner, since that will just require more work later.

@zakarumych
Copy link
Member

zakarumych commented Nov 1, 2018

why would we be able to gain a significan performance improvement

By making it zero-cost for ash.

I don't want to make another abstraction layer (I was wrong thinking otherwise).
I'd like to use existing one if it doesn't compromise performance on platform it mimics (and I believe most of us care most of) and is convenient to use, yes, this is important.

we want mid or long term

What we want mid or long term?

@torkleyy
Copy link
Member Author

torkleyy commented Nov 1, 2018

What we want mid or long term?

Not to duplicate efforts. What if gfx gets low level optimizations, debugging facilities, etc?

Maintaining two backends is a lot more work, and it's hard to believe for me that a high-level rendering API can be zero-cost.

@kvark
Copy link
Member

kvark commented Nov 1, 2018

@omni-viral

I just don't want rendy's abstractions become basically hal-ish abstraction layer owned by another team... Abstraction layers that don't add nothing are not worth writing.

I don't know what to think of that, tbh. The last sentence reads half obvious and half inappropriately offensive.

Maybe the best course of actions would be to move hal closer to ash. Make it more flexible.

This general direction has been considered in full: gfx-rs/gfx#2206

Make some types Copy.

I've made an RFC about this recently - gfx-rs/gfx#2457, and got some positive feedback, but nothing constructive so far. You are welcome to express the views on the issue.

Provide real documentation (at least in form of links). And even generate it bulb.

I don't think it matters much given that you are the only one working on the rendering subsystem. I agree that generating the docs locally adds a tiny bit of friction, but it's not a showstopper.

Make unsafe methods marked unsafe

We are discussing this in gfx-rs/gfx#2453 . I don't see how this affects you, tbh. You can just treat all gfx-hal as unsafe on your side today, especially since that's what you do for Ash already.

Having concrete type in signature of a function you plan to call few thousand times per second encourages you to store data in that type and call a function without any conversions. Moving bits costs when you do it a lot.

Are you sure that you are optimizing the right thing here? Do you have a hotspot in mind that uses iterators in gfx-hal and that would be improved by you storing the data in the arrays?

I understand that each backend has its own concrete type and abstract as iterator makes sense. But since amethyst and rendy are mostly about games then focusing on vulkan backend first should make sense too.

Has Amethyst team actually sit together and put a priority on Vulkan as a backend? I still remember the old good times when Unreal came out and we had this wonderful multiple choice window at start, asking to pick D3D/GL/Glide/MeTaL/software, and I couldn't have guessed which one would work best until I benchmarked all of them, considering all of performance, quality, and stability. And these were the times when everyone could theoretically write GL-only apps (like Id software did) and run on all the platforms, much unlike today where Vulkan support is very limited (no Intel Haswell/Broadwell and below on Win10, no MacOS, poor debugging experience, very small Android user base, etc). If I were you, I'd want to give users more choice, support more platforms, and... ideally have this beauty back:

renderpick

@zakarumych
Copy link
Member

high-level rendering API can be zero-cost.

No. It can't.

gfx gets low level optimizations, debugging facilities.

Low-level optimizations are done when implementing higher level API. Debug facilities exists in Vulkan and not exposed by gfx.

@zakarumych
Copy link
Member

zakarumych commented Nov 1, 2018

half inappropriately offensive.

Didn't mean to. I was refering to rendys abstraction traits you can find in master as worthless.
Hal abstracts different backends. It worth a lot.

@MaikKlein
Copy link

I already mentioned this in discord, but I don't think that it matters. Just stick with the the library that works the best for you right now, or where you have spent to most time.

You should probably abstract over the various APIs anyway. Create a backend like rendy_hal, rendy_ash as an external library. Focus your work on just one backend until the the rendy API has stabilized a bit, then you can start to add other backends.

If you went with hal, you get the other backends for free, although I am not sure how stable all the backends are right now.

If you went wish ash, you get Vulkan and Dx12/Metal etc though the portability lib or Metal with MoltenVk. I am not sure how much overhead there will be compared to hal.

You can also still create custom backends like rendy_webgl etc. Also you should look at Unity's renderer, where they have a lightweight renderer and a high definition renderer.

I don't think maintaining multiple backends will be that much work. Remember you only need a subset of the various APIs, and rendy appears to expose at least some low level things like memory, which means a lot of optimization can happen in the higher level layer.

Also hal and ash are still very similar. So it should be straight forward to add another Vulkan backend if anyone wants to maintain it.

@zakarumych
Copy link
Member

@MaikKlein thank you for input.

I just think it's weird to have multiple Vulkan backends.
It would make sense to have custom Dx12, Metal, GL backends implemented for higher level API. Low level parts of rendy API, like memory manager, will be feature/extension gated in this case.
So rendy_ash and rendy_hal would have trivial (and similar) implementation. It can be done, no doubt. But it will require rendy to define abstractions on both low and high levels. And low-level abstraction will just mimic low-level implementations. We will have one more version of types looking exactly like hal's and ash's. Why can't we have them defined once?

@kvark
Copy link
Member

kvark commented Nov 1, 2018

It would make sense to have custom Dx12, Metal, GL backends implemented for higher level API

It could make sense, but only in the context of performance. Yes, if you have a nicely defined higher level model of interacting with GPU, you can probably take more advantage of mapping it to the low-level APIs directly.

Is this really what you want to do though? This task would require enormous effort to implement, test, and maintain. Not to mention the arguable benefit to Rust community, given that you'd be doing everything from scratch. It's also unclear if you are planning to expose the rendering model for non-Amethyst apps, and what advantage it would have versus things like wgpu-rs or even three-rs.

@Moxinilian
Copy link
Member

I am somewhat confused by the iterator performance issue. rustc is pretty good at optimizing them, do you have a specific example where it would not be equivalent?

@zakarumych
Copy link
Member

Is this really what you want to do though?

No, unless someone would need that additional performance.

You were telling that you want this nice window where one can choose from different available backends to see which is most performant on target machine. But I highly doubt if all those backends are hidden behind vulkan API that vulkan backend will be outperformed (if available ofc). Hence I think it makes sense to focus on the vulkan backend.

@Moxinilian they are optimized. Yes. But when you compare iteration with storing result in array of some sort versus having this array in the first place then the only way it can be equal is iterating through same array. Which is not the case since types are different. @kvark suggests that overhead is negligible and I have no benchmarks to prove otherwise. So let's just agree with @kvark on the matter.

@zakarumych
Copy link
Member

zakarumych commented Nov 1, 2018

I have one crazy idea.

What if instead of accepting tons of iterators to the methods hal would provide intermediate result of collected named iterators in the exact structure backend will utilize.
So instead of having Submission containing iterator and iterating each frame I would be able to create backend specific B::Submission from those iterators once. And then feed Queue with it.
Instead of collecting pipeline barriers each frame I will construct B::PipelineBarrier and record it into CommandBuffer in O(1). For metal and gl B::PipelineBarrier would be ().

Same can go with resources. hal could provide B::SharedT versions for resources that all are Copy and implemented in the backend-specific way. For vulkan backend B::Buffer and B::ShaderdBuffer will be the same thing. B::SharedT would have to implement From<B::T> + Borrow<B::T>.

That's even two crazy ideas.

@kvark
Copy link
Member

kvark commented Nov 1, 2018

@omni-viral unfortunately, this would mean even more generic types that gfx-hal would expose and the backends would have to implement. In general, we'd like to reduce this type zoo, not grow it.

I think the clone-ability discussion in gfx-rs/gfx#2457 will yield results, and we'll make sure to take Amethyst interests into account. But for iterators, I strongly believe there is no action required until we see a performance hit. Take the pipeline barriers into account: if you aren't re-using the command buffer and instead re-recording it, the time spent copying the barriers when iterating is going to be negligible comparing to the time spent on recording the command buffer. Let's not worry about it at this point.

@zakarumych
Copy link
Member

Why you'd want to reduce type zoo? Is it even possible?

@zakarumych
Copy link
Member

We were settled on gfx-hal

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

7 participants