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

Introduce explicit Viewport sharing #96

Merged
merged 1 commit into from
May 8, 2024

Conversation

hecrj
Copy link
Sponsor Contributor

@hecrj hecrj commented May 8, 2024

This PR introduces a Viewport type that explicitly holds the Params of a Pipeline.

This allows sharing a Viewport with multiple TextRenderer instances, reducing pipeline context changes.

Depends on #95.

Copy link
Owner

@grovesNL grovesNL left a comment

Choose a reason for hiding this comment

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

This looks great, thank you! Sharing Viewport explicitly seems like a good approach.

I wish we could derive this internally somehow and share params on a shared type like Pipeline. I'm not sure how we could do that without requiring something like a viewport index to be specified during screen resolution updates though.

@hecrj
Copy link
Sponsor Contributor Author

hecrj commented May 8, 2024

Rebased on top of main!

I wish we could derive this internally somehow and share params on a shared type like Pipeline. I'm not sure how we could do that without requiring something like a viewport index to be specified during screen resolution updates though.

Right... It doesn't seem easy to figure out which params to invalidate internally. At least, we are mostly just trading an argument here (Resolution for Viewport).

@grovesNL grovesNL merged commit b411ea7 into grovesNL:main May 8, 2024
1 check passed
@hecrj hecrj deleted the explicit-viewport branch May 9, 2024 09:52
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

Successfully merging this pull request may close these issues.

None yet

2 participants