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 UV issues in Compatibility renderer #87067

Conversation

moonpirates
Copy link
Contributor

Fixes a bug where CanvasTexture's UVs were off when using the Compatibility mode's renderer.

Fixes #86746

@moonpirates moonpirates requested a review from a team as a code owner January 11, 2024 10:46
@moonpirates moonpirates changed the title Fix issue where the UV's were off in Compatibility Fix issue where the UVs were off in Compatibility Jan 11, 2024
@AThousandShips AThousandShips added this to the 4.3 milestone Jan 11, 2024
@AThousandShips AThousandShips changed the title Fix issue where the UVs were off in Compatibility Fix UV issues in Compatibility renderer Jan 11, 2024
drivers/gles3/rasterizer_canvas_gles3.cpp Outdated Show resolved Hide resolved
drivers/gles3/rasterizer_canvas_gles3.cpp Outdated Show resolved Hide resolved
drivers/gles3/rasterizer_canvas_gles3.cpp Outdated Show resolved Hide resolved
drivers/gles3/rasterizer_canvas_gles3.cpp Outdated Show resolved Hide resolved
@AThousandShips
Copy link
Member

I'd suggest against breaking up the if/else block, it makes sense as it were, just fixing the size in the default case, like so:

if (!texture) {
	ct->diffuse = texture_storage->texture_gl_get_default(GLES3::DEFAULT_GL_TEXTURE_WHITE);
	size_cache = Size2i(1, 1);
} else {
	size_cache = Size2i(texture->width, texture->height);
}

And remove the rest

@moonpirates
Copy link
Contributor Author

@AThousandShips the problem with that is that the diffuse slot, upon a subsequent iteration, is assigned to a texture (the default white), making it end up in the else statement, which will fall back to the 4x4 size.

Thanks for your feedback, I'll fix that!

@moonpirates moonpirates force-pushed the bugfix-canvastexture-uvs-compatibility branch from 194ea24 to 1e00385 Compare January 11, 2024 11:11
Fixes a bug where CanvasTexture's UVs were off when using the Compatibility mode's renderer.

Fixes godotengine#86746
@moonpirates moonpirates force-pushed the bugfix-canvastexture-uvs-compatibility branch from 1e00385 to ac87d5f Compare January 11, 2024 12:27
Copy link
Member

@clayjohn clayjohn left a comment

Choose a reason for hiding this comment

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

Looks great!

@akien-mga akien-mga merged commit 463edd0 into godotengine:master Jan 15, 2024
15 checks passed
@akien-mga
Copy link
Member

Thanks! And congrats for your first merged Godot contribution 🎉

@moonpirates moonpirates deleted the bugfix-canvastexture-uvs-compatibility branch January 18, 2024 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inconsistant UV coordinates between Forward+ and Compatibility
4 participants