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

Cannot clear window due to `draw_*d` being on the window now #136

Closed
TheNeikos opened this Issue Apr 20, 2016 · 3 comments

Comments

Projects
None yet
3 participants
@TheNeikos
Copy link
Contributor

TheNeikos commented Apr 20, 2016

An unforseen consequence of putting the draw on the window is that it is now borrowed mutably during draw_2d and draw_3d! Which means that one cannot now borrow anything from it immutably, like for example clearing the output.

window.draw_3d(&e, |enc| {
    enc.clear(&window.output_color, GRAY);
});

Will not compile.

(I've accidentally opened it on the wrong repo before )

@mitchmindtree

This comment has been minimized.

Copy link
Member

mitchmindtree commented Apr 20, 2016

One option could be to make new types Draw2d and Draw3d (or Draw2dArgs and Draw3dArgs) which could be passed to their respective closures as the sole argument. These types could encapsulate both the fields from the window that a user might deem necessary, along with all current arguments. I.e.

window.draw_3d(&e, |args| {
    args.encoder.clear(&args.output_color, GRAY);
});

Another nice quality of wrapping args in a struct is that you can destructure them in argument position:

window.draw_3d(&e, |Draw3d { encoder, output_color, .. }| {
    encoder.clear(output_color, GRAY);
});
@bvssvni

This comment has been minimized.

Copy link
Member

bvssvni commented Apr 22, 2016

Alternative:

  • Pass PistonWindow to draw_3d
  • Remove PistonWindow::app to use application state separately in draw_2d

bvssvni added a commit to bvssvni/piston_window that referenced this issue Apr 22, 2016

Redesign (remove `PistonWindow::app`)
Closes PistonDevelopers#136

- Bumped to 0.44.0
- Removed `PistonWindow::app` to allow using application state inside
`draw_2d`
- Pass `PistonWindow` to `draw_3d` to use other fields
- Updated docs
- Updated README
@bvssvni

This comment has been minimized.

Copy link
Member

bvssvni commented Apr 22, 2016

I've opened a PR #138 for the alternative I suggested. It does what @mitchmindtree suggested for draw_3d, and the overall design gets simpler. I prefer this to adding more complexity, and it keeps 2D rendering consistent with the other graphics backends.

@bvssvni bvssvni added easy and removed discussion draft labels Apr 24, 2016

@bvssvni bvssvni self-assigned this Apr 24, 2016

@bvssvni bvssvni closed this in #138 Apr 24, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment