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

[Merged by Bors] - Add globals struct to mesh2d #6222

Closed

Conversation

torsteingrindvik
Copy link
Contributor

@torsteingrindvik torsteingrindvik commented Oct 10, 2022

See commit message.
I noticed I couldn't use globals.time when using Material2d.

I copied the solution from 8073362 , and now Material2d works for me.

Perhaps some of these struct definitions could be shared in the future, but for now I've just copy pasted it (it looked like the View struct was done that way).

Ping @IceSentry , I saw a comment on the linked commit that you intended to do this work at some point in the future.

- When using `Material2d` for shaders, allow accessing the `globals`
struct.
This work is based on commit 8073362,
whichs adds the struct to the 3D counterpart.

- Copy the struct from `bevy_pbr` as-is into `bevy_sprite`.
Update the bind group layout and the bind group creation
to match.

- Binds the `globals` struct as a uniform in group 0 binding 1 for the
mesh 2D view.

Signed-off-by: Torstein Grindvik <torstein.grindvik@nordicsemi.no>
@IceSentry
Copy link
Contributor

Yep, that was an omission on my part. I think just copying it is fine for now. I'll have a look at this in a few hours.

@IceSentry IceSentry added A-Rendering Drawing game state to the screen C-Usability A simple quality-of-life change that makes Bevy easier to use labels Oct 10, 2022
@torsteingrindvik
Copy link
Contributor Author

Note: I searched for uses of Material2d. The only example using this right now is the shader/post_processing example.
I tested it with these changes and it still works (I didn't modify the example, I just checked that nothing suddenly broke).

Copy link
Contributor

@IceSentry IceSentry left a comment

Choose a reason for hiding this comment

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

LGTM, I haven't tested it locally, but if CI passes I think it should be fine.

@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Oct 10, 2022
@alice-i-cecile
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Oct 10, 2022
See commit message.
I noticed I couldn't use `globals.time` when using `Material2d`.

I copied the solution from 8073362 , and now `Material2d` works for me.

Perhaps some of these struct definitions could be shared in the future, but for now I've just copy pasted it (it looked like the `View` struct was done that way).

Ping @IceSentry , I saw a comment on the linked commit that you intended to do this work at some point in the future.
@bors bors bot changed the title Add globals struct to mesh2d [Merged by Bors] - Add globals struct to mesh2d Oct 10, 2022
@bors bors bot closed this Oct 10, 2022
@mockersf mockersf added the hacktoberfest-accepted A PR that was accepted for Hacktoberfest, an annual open source event label Oct 10, 2022
james7132 pushed a commit to james7132/bevy that referenced this pull request Oct 19, 2022
See commit message.
I noticed I couldn't use `globals.time` when using `Material2d`.

I copied the solution from 8073362 , and now `Material2d` works for me.

Perhaps some of these struct definitions could be shared in the future, but for now I've just copy pasted it (it looked like the `View` struct was done that way).

Ping @IceSentry , I saw a comment on the linked commit that you intended to do this work at some point in the future.
james7132 pushed a commit to james7132/bevy that referenced this pull request Oct 28, 2022
See commit message.
I noticed I couldn't use `globals.time` when using `Material2d`.

I copied the solution from 8073362 , and now `Material2d` works for me.

Perhaps some of these struct definitions could be shared in the future, but for now I've just copy pasted it (it looked like the `View` struct was done that way).

Ping @IceSentry , I saw a comment on the linked commit that you intended to do this work at some point in the future.
Pietrek14 pushed a commit to Pietrek14/bevy that referenced this pull request Dec 17, 2022
See commit message.
I noticed I couldn't use `globals.time` when using `Material2d`.

I copied the solution from 8073362 , and now `Material2d` works for me.

Perhaps some of these struct definitions could be shared in the future, but for now I've just copy pasted it (it looked like the `View` struct was done that way).

Ping @IceSentry , I saw a comment on the linked commit that you intended to do this work at some point in the future.
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
See commit message.
I noticed I couldn't use `globals.time` when using `Material2d`.

I copied the solution from 8073362 , and now `Material2d` works for me.

Perhaps some of these struct definitions could be shared in the future, but for now I've just copy pasted it (it looked like the `View` struct was done that way).

Ping @IceSentry , I saw a comment on the linked commit that you intended to do this work at some point in the future.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Usability A simple quality-of-life change that makes Bevy easier to use hacktoberfest-accepted A PR that was accepted for Hacktoberfest, an annual open source event S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants