Skip to content
This repository has been archived by the owner on Apr 18, 2022. It is now read-only.

[WIP] Rendy v0.5.x #2040

Closed
wants to merge 6 commits into from
Closed

[WIP] Rendy v0.5.x #2040

wants to merge 6 commits into from

Conversation

valkum
Copy link
Contributor

@valkum valkum commented Nov 15, 2019

Description

Update to rendy v0.5.x and winit v0.20.0-alpha5
Feel free to contribute.

Currently an in place upgrade without any new features added. Might allow for OpenGL

Status

Vulkan complains about the device being dropped before an object. Needs investigation.
Rest is working.
Not responding window might be an issue with our dispatcher blocking the main thread. Needs separate issue.

Blockers

Todo:

Changes

See Changelog of rendy and winit

PR Checklist

By placing an x in the boxes I certify that I have:

  • Updated the content of the book if this PR would make the book outdated.
  • Added a changelog entry if this will impact users, or modified more than 5 lines of Rust that wasn't a doc comment.
  • Added unit tests for new code added in this PR.
  • Acknowledged that by making this pull request I release this code under an MIT/Apache 2.0 dual licensing scheme.

If this modified or created any rs files:

  • Ran cargo +stable fmt --all
  • Ran cargo clippy --all --features "empty"
  • Ran cargo test --all --features "empty"

Copy link
Member

@azriel91 azriel91 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(eager review 😛, guessing i should wait)

amethyst_gltf/src/format/material.rs Outdated Show resolved Hide resolved
amethyst_rendy/Cargo.toml Outdated Show resolved Hide resolved
amethyst_rendy/src/formats/mesh.rs Outdated Show resolved Hide resolved
amethyst_rendy/src/formats/texture.rs Outdated Show resolved Hide resolved
amethyst_rendy/src/plugins.rs Outdated Show resolved Hide resolved
amethyst_rendy/src/system.rs Outdated Show resolved Hide resolved
As rendy updated to the new winit version the change is included here too.
@valkum
Copy link
Contributor Author

valkum commented Dec 11, 2019

Let's talk about the ergonomics of the new mainloop. (See updated examples)
I updated some examples (rendy, renderable, sprite_animation, sprite_camera_follow, sprites_ordered, tiles ) to work with the new API.
To be honest I don't like it.

What are your opinions?

Restructure to use EventLoop::run instead of run_return
Fix rendy by moving some system to the mainthread
Update the following examples to work with the winit and rendy upgrades: rendy, renderable, sprite_animation, sprite_camera_follow, sprites_ordered, tiles
@@ -19,9 +19,9 @@ license = "MIT/Apache-2.0"
travis-ci = { repository = "amethyst/amethyst", branch = "master" }

[features]
default = ["animation", "audio", "locale", "network", "renderer"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean to commit this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops should only exclude audio because of rust-windowing/winit#1255

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh. The hightlight on github looked like I removed all defaults. Removing audio was intended because of the issue.

examples/renderable/main.rs Outdated Show resolved Hide resolved
@distransient distransient added the pri: important Something other teams are relying on, or a low-level, critical piece of functionality. label Dec 17, 2019
@azriel91
Copy link
Member

azriel91 commented Jan 8, 2020

Current branch state:

  • Updated to winit 0.20.0
  • Resolves compilation errors for rendy

It doesn't yet compile as winit::event::Event is !Clone, which is needed for our StateEvent. rust-windowing/winit#1374 improves the situation (WindowEvent will be Clone, though Event itself will not be).

  • Rework MonitorIdent in amethyst_window may be done (amethyst_window compiles, and has fixed the logical size to physical size conversions (winit returns PhysicalSize by default now, previously logical size))

@azriel91
Copy link
Member

Something to watch (unblocks the StateEvent breakage): rust-windowing/winit#1456

@semtexzv
Copy link

semtexzv commented Feb 21, 2020

Possible way to get amethyst usecase unblocked: rust-windowing/winit#1478

// Edit: Took my fix to winit, rebased the rendy branch on top of master. Seems to compile on linux so far.
https://github.com/semtexzv/amethyst/tree/rendy_v5
& more rewritten examples
https://github.com/semtexzv/amethyst/tree/rendy-all

It seems that we'll need to make Application::run take EventLoop as an argument, current ergonomics are terrible.

@azriel91 Feel free to pull my changes

@azriel91
Copy link
Member

azriel91 commented Mar 3, 2020

@semtexzv Heya, thank you very much for your effort 🙇‍♂️! I'm on another project for 2 weeks, but after that shall be focusing a lot on getting WASM going, and your changes definitely help!

@azriel91
Copy link
Member

Closing -- best follow the WASM project, and use the wasm branch.

@azriel91 azriel91 closed this Apr 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pri: important Something other teams are relying on, or a low-level, critical piece of functionality. team: rendering
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants