Fix Sprite3D texture bleed when not wrapping#98122
Fix Sprite3D texture bleed when not wrapping#98122akien-mga merged 1 commit intogodotengine:masterfrom
Conversation
AThousandShips
added
bug
topic:rendering
topic:3d
cherrypick:4.2
Repiteo
added
the
cherrypick:4.4
akien-mga
removed
the
cherrypick:4.2
|
Will this be in 4.4.1? |
|
This hasn't been merged yet and won't be cherry-picked until after it has been merged, so probaly won't be in 4.4.1 as we are planning to release that pretty soon |
Repiteo
added
the
cherrypick:4.5
|
Hi, What's the status of this PR? The feature is important for my team. I see that it was pushed back to 4.5 and now to 4.6. Is it a manner of positive review, or is something else still needing work? |
kleonc
left a comment
kleonc
left a comment
There was a problem hiding this comment.
What's the status of this PR? The feature is important for my team. I see that it was pushed back to 4.5 and now to 4.6. Is it a manner of positive review, or is something else still needing work?
AFAICT it's awaiting a review from @godotengine/rendering.
But currently it's also outdated / it has conflicts with the current master branch. So would need to be rebased as well.
Seems there's also #90202 which aims to resolve the same issue, but AFAICT this PR is more aligned with what the rendering team wanted the solution to be. Hence I'm leaving my review here.
|
You used a merge commit to update your branch, please use rebase in the future instead, see here |
cadence-s
force-pushed
the
fix-sprite3d-bleed
branch
3 times, most recently
from
282c9e9 to
19f8a74
Compare
# Conflicts: # scene/resources/material.cpp
cadence-s
force-pushed
the
fix-sprite3d-bleed
branch
from
19f8a74 to
fc9ea46
Compare
|
This should be all cleaned up now |
There was a problem hiding this comment.
LGTM, fixes the issue.
As the rendering team wanted the repeat flag is being enabled/disabled automatically. However, according to my suggestion, it's done in a slightly different manner than originally suggested, as it's based on the final UV region used instead of solely based on the Sprite3D.region (this makes it work e.g. for AtlasTextures). Nevertheless, this should get an approval from the rendering team before merging.
akien-mga
removed
cherrypick:4.3
|
I am looking to test this in our custom 4.5 build. |
There shouldn't be any problems with merging it into 4.5; the relevant code hasn't changed in a long time. |
|
Thanks! |
clayjohn
removed
the
cherrypick:4.5
9 participants
Fixes #82044.
Texture wrapping is now turned off by default in 2D-in-3D, except for the case where a Sprite3D has a defined region larger than the texture size. According to @clayjohn this was discussed as a good solution in a rendering meeting (#82193 (comment)).