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

Update SpriteEffect.cs to honor the Viewport settings #8093

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

stromkos
Copy link
Contributor

@stromkos stromkos commented Nov 16, 2023

Added position and min/max depth values from the GraphicsDevice.Viewport.
See #8074

Added position and min/max depth values from the GraphicsDevice.Viewport.
Negated MinDepth as well.
@mrhelmut
Copy link
Contributor

This seems correct, though it'd be best to wait for graphical unit tests to come back online before we merge just to make sure.

@Mindfulplays
Copy link
Contributor

Are we sure this is correct? glViewport (or any platform Viewport call such as Metal/DX) should apply vp.X/Y etc already. And hence, the original code is correct. Note that the backbuffer that the sprite effect renders onto has the correct width/height within the viewport (not the screen). So the shader correctly multiplies normalized coords with the matrix W/H.

(Viewport X/Y is generally 0,0 so it's hard to test this. But, on Android, GraphicsDevice::ResetClientBounds explicitly sets viewport with a x/y offset due to safe-area/cutouts - which is a common enough use case so you can verify this quickly)

I recommend not touching position.

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

3 participants