Skip to content

Move initializing the ScreenshotToScreenPipeline to the ScreenshotPlugin. #18524

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 6, 2025
Merged

Move initializing the ScreenshotToScreenPipeline to the ScreenshotPlugin. #18524

merged 1 commit into from
May 6, 2025

Conversation

@andriyDev
Copy link
Contributor

Objective

  • Minor cleanup.
  • This seems to have been introduced in Webgpu support #8336. There is no discussion about it I can see, there's no comment explaining why this is here and not in ScreenshotPlugin. This seems to have just been misplaced.

Solution

  • Move this to the ScreenshotPlugin!

Testing

  • The screenshot example still works at least on desktop.

@andriyDev andriyDev marked this pull request as ready for review March 25, 2025 00:55
@andriyDev andriyDev added D-Trivial Nice and easy! A great choice to get started with Bevy A-Windowing Platform-agnostic interface layer to run your app in S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Mar 25, 2025
@andriyDev andriyDev requested a review from mockersf March 25, 2025 00:55
greeble-dev
greeble-dev approved these changes May 1, 2025
View reviewed changes
Copy link
Contributor

@greeble-dev greeble-dev left a comment

Choose a reason for hiding this comment

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

Clicking approve as changes seem sensible and I've tested the screenshot example (Win10, Chrome/WebGL, Desktop/Vulkan).

@andriyDev andriyDev added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels May 1, 2025
@alice-i-cecile alice-i-cecile added this pull request to the merge queue May 5, 2025
Merged via the queue into bevyengine:main with commit 798e1c5 May 6, 2025
38 checks passed
andrewzhurov pushed a commit to andrewzhurov/bevy that referenced this pull request May 17, 2025
@andriyDev @andrewzhurov
Move initializing the ScreenshotToScreenPipeline to the `Screenshot…
52b27ed 
…Plugin`. (bevyengine#18524)

# Objective

- Minor cleanup.
- This seems to have been introduced in bevyengine#8336. There is no discussion
about it I can see, there's no comment explaining why this is here and
not in `ScreenshotPlugin`. This seems to have just been misplaced.

## Solution

- Move this to the ScreenshotPlugin!

## Testing

- The screenshot example still works at least on desktop.
@andriyDev andriyDev deleted the screenshot branch June 14, 2025 00:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alice-i-cecile alice-i-cecile alice-i-cecile approved these changes

@mockersf mockersf Awaiting requested review from mockersf

+1 more reviewer

@greeble-dev greeble-dev greeble-dev approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

A-Windowing Platform-agnostic interface layer to run your app in D-Trivial Nice and easy! A great choice to get started with Bevy 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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@andriyDev @alice-i-cecile @greeble-dev