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

Reduce the number of layers our commands go through #9

Closed
kvark opened this issue Jun 21, 2014 · 7 comments
Closed

Reduce the number of layers our commands go through #9

kvark opened this issue Jun 21, 2014 · 7 comments

Comments

@kvark
Copy link
Member

kvark commented Jun 21, 2014

Here are the current stops in a one-way trip of a client command:

  1. render::Client::do_something() creates a Cast or Call message and sends it through the pipe
  2. render::Server receives the message and calls some the device::Client methods
  3. device::Client:::do_something() creates a message and sends it down the pipe
  4. device::Server receives the message and calls some of the back-end methods
  5. the back-end actually executes them

You can see there are 5 layers of execution... While we certainly need abstraction, we'd be fine with just 2 - 3 layers. I propose to remove device::Client methods in favor of render just sending messages directly. We can also remove the render::Client in favor of pure messages.

(+) less code for us to maintain, less layers to loose yourself in
(+) exposing asynchronous nature actually allows to benefit from it, i.e. send a bunch of commands and then receive some (instead of send-receive cycles)
(-) less convenient for the client (the problem does not apply to device <-> render interaction)

@kvark
Copy link
Member Author

kvark commented Jun 22, 2014

One negative aspect of the current system is that some logic is happening in Server update loops (both render::Server and device::Server), inside the match arms of the loop. That is in contrast with the plain pass-through logic that we do in separate methods of *::Client.

@brendanzab
Copy link
Contributor

I agree that the current way of doing things is easy to lose yourself in. Could you try to explain again what design would you prefer? I didn't quite understand what you meant.

@kvark
Copy link
Member Author

kvark commented Jun 22, 2014

I'd prefer us to get rid of both render::Client and device::Client in favor of direct message passing. So instead of that:

let h_vs = self.device.new_shader('v', vs);

We'd do something like this:

self.device.stream.send(device::CallNewShader('v', vs));
let h_vs = match self.device.stream.recv() {
 device::ReplyNewShader(name) => name,
 _ => fail("...")
};

I know, that might not be the hottest piece of code in the town, but at least it can do multiple requests before asking for results.

@kvark
Copy link
Member Author

kvark commented Jun 22, 2014

Casts are gonna be fine, but Calls syntax is a bit too heavy. I'll think if I can find a good solution that would still reduce the number of layers but would not complicate the syntax too much.

@kvark
Copy link
Member Author

kvark commented Jul 1, 2014

Reasons why we need to keep render::Client:

  • we could have an alternative client that doesn't spawn the render task but has the same interface (for easy switching)
  • it can instantly return new handles, which is required for Asynchronous interface for the client #21
  • it is more convenient to deal with by the end-user, comparing to message passing
  • it can accept generics, while channels need concrete types

@kvark
Copy link
Member Author

kvark commented Jul 2, 2014

Thus, since render::Client should live, and everything related to the device is supposed to be covered by #48, I'm closing this.

@kvark kvark closed this as completed Jul 2, 2014
@kvark
Copy link
Member Author

kvark commented Jul 3, 2014

Solving #60 will help with layers.

kvark pushed a commit to kvark/gfx that referenced this issue Jun 14, 2015
Updated to implement Factory
kvark pushed a commit to kvark/gfx that referenced this issue Jun 14, 2015
kvark added a commit to kvark/gfx that referenced this issue Jun 14, 2015
Remove use of removed handle typedefs
adamnemecek pushed a commit to adamnemecek/gfx that referenced this issue Apr 1, 2021
9: Restructure the repo to host multiple crates r=grovesNL a=kvark

As a follow-up, I'll rename this repository to just `wgpu`.

Co-authored-by: Dzmitry Malyshau <kvark@mozilla.com>
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

2 participants