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

Stop using Either in render pass node #207

Open
ishitatsuyuki opened this issue Sep 20, 2019 · 5 comments
Open

Stop using Either in render pass node #207

ishitatsuyuki opened this issue Sep 20, 2019 · 5 comments

Comments

@ishitatsuyuki
Copy link
Contributor

Either is used in render pass node for handling both images and surfaces:

type Attachment = Either<ImageId, RenderPassSurface>;

However, this is simply confusing and does not provide much benefit considering it's an additional dependency as well. We should use a custom enum instead.

@Frizi
Copy link
Member

Frizi commented Sep 21, 2019

I'm planning to get rid of special surface handling soon anyway, which means we would stick to using ImageId exclusively here. We can change that for short term benefit, but this code will get removed on next api rendergraph iteration.

@ishitatsuyuki
Copy link
Contributor Author

Sure, removing the need of enum is also fine.

@hansihe
Copy link
Collaborator

hansihe commented Jan 28, 2020

Would removing special surface handling involve using the present node and thus require copying the image data, or did you have some other approach in mind? I guess the final solution to this is making the graph more dynamic/flexible and removing the need for this stuff at all?

@zakarumych
Copy link
Member

zakarumych commented Jan 31, 2020

the final solution to this is making the graph more dynamic/flexible and removing the need for this stuff at all

Exactly that. But I haven't heard from @Frizi (who was working on it) for a while now.

@hansihe
Copy link
Collaborator

hansihe commented Jan 31, 2020

I have been working on a graph implementation based on the snippets he described in #240 for a little bit now. Mainly because I either need to patch rendy to add another case to the render pass node in order to do what I want, or to implement a more dynamic graph myself.

I presume there is interest in merging that into the upstream rendy if I ever get it done?

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

4 participants