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

fix: blit should only draw one triangle #2474

Merged
merged 1 commit into from
Feb 13, 2022

Conversation

CurryPseudo
Copy link
Contributor

@CurryPseudo CurryPseudo commented Feb 11, 2022

Description
Original shader:

fn vs_main(@builtin(vertex_index) vertex_index: u32) -> VertexOutput {
var out: VertexOutput;
let x = i32(vertex_index) / 2;
let y = i32(vertex_index) & 1;
let tc = vec2<f32>(
f32(x) * 2.0,
f32(y) * 2.0
);
out.position = vec4<f32>(
tc.x * 2.0 - 1.0,
1.0 - tc.y * 2.0,
0.0, 1.0
);
out.tex_coords = tc;
return out;
}

x in [0, 1]
y in [0, 1]
out.tex_coord in [(0, 0), (2, 2)]
out.position in [(-1, -3), (3, 1)]
Clearly it is wrong, but it actually covers the whole render area([(-1, -1), (1, 1)]), and tex_coord happened to be right, so it worked.
image

Testing
Run the mipmap example, result is the same.
master branch:
image
this branch:
image

@kvark
Copy link
Member

kvark commented Feb 11, 2022

This is actually by design. The shader is written as if one large triangle is drawn.
The confusing part is that the Rust side is written to spew 2 triangles in a strip. I think that part needs to be fixed to just use one triangle as triangle list instead.

@CurryPseudo
Copy link
Contributor Author

That makes sense.
I changed to triangle list and use only 3 vertices, it works.

@CurryPseudo CurryPseudo changed the title fix: blit shader example's position and tex_coords fix: blit should only draw one triangle Feb 12, 2022
Copy link
Member

@kvark kvark left a comment

Choose a reason for hiding this comment

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

Thank you!

@kvark kvark merged commit a604535 into gfx-rs:master Feb 13, 2022
cwfitzgerald pushed a commit that referenced this pull request Oct 25, 2023
The `splice()` call takes `IntoIterator`, so calling `into_iter()` is not needed.
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.

2 participants