Skip to content

TextureAtlas texture sampling can sample from the wrong atlas index #4162

Closed

Description

@mdenchev

Bevy version

0.6.1, also main

Operating system & version

Windows 10

What you did

  • Created a tilemap with 16x16 px tiles using a TextureAtlas
  • Map was of size ~25x25 tiles
  • Ortho camera
  • MSAA set to 1
  • Filtering set to Nearest
  • Panned the camera and changed the camera scaling while experimenting

What you expected to happen

I expected the grid to render "correctly", i.e. no gaps between tiles, proper textures applied, etc.

What actually happened

Under certain zooms and camera positions 1px wide "tears" occurred that would span a row or a column (image attached [0]). Those tears use texture data from an adjacent TextureAtlas index.

Additional information

I believe the issue is with this line here https://github.com/bevyengine/bevy/blob/main/crates/bevy_sprite/src/render/mod.rs#L478 and I tested a fix which seems to work but I want to confirm it makes sense before I create a PR.

To explain the issue (as I understand it), imagine you have a texture atlas with two sub-textures placed horizontally. Computing the uv rectangle for the second "sub-texture" (index 1) results in the uv verts [(0.5, 0.0), (1.0, 0.0), (1.0, 1.0), (0.5, 1.0)]. The problem arises with 0.5 being right on the border of the two textures. Due to numerical inaccuracies with floats I believe we can end up sampling with a value like 0.49999 even though index 1 was specified, resulting in the wrong texel being selected in the shader.

Proposed fix: change the computed rectangle size of sub-textures by shrinking them by the extent of a texel on all sides. In the above example if each sub-texture is a 1x1 px image, the texel size would be (0.5, 1.0) - and the mid point of the first texel would be (0.25, 0.5) and the midpoint of the second texel would be (0.75, 0.5). So instead of starting from 0.5 for the sampling u value we should start from 0.75 and also end there as well since we would subtract 0.25 from the high u values as well. This would prevent any small numerical errors from causing a sampling from the wrong sub-texture.

In code, the proposed solution would change:

                        *uv = (rect.min + *uv * rect_size) / current_image_size;

to

                        *uv = (rect.min + *uv * rect_size
                            + *uv * Vec2::new(-1.0, -1.0)
                            + Vec2::new(0.5, 0.5)) / current_image_size;

I think this would work for Nearest filtering but I'm not sure of the implications it might have on other modes.

Apologies if the terms above were not used correctly, not really an expert in graphics :)

[0] Here's an image of the "tearing"
image

After some experimenting, the colors in the tear come directly from the texture line right above the first line of the current index being specified.

Activity

  1. added
    C-BugAn unexpected or incorrect behavior
    S-Needs-TriageThis issue needs to be labelled
    on Mar 9, 2022
  2. added
    A-RenderingDrawing game state to the screen
    and removed
    S-Needs-TriageThis issue needs to be labelled
    on Mar 9, 2022
  3. alice-i-cecile commented on Mar 9, 2022

    @alice-i-cecile
    Member

    Ping @StarArawn; IIRC I've seen you give advice on similar challenges for users of bevy_ecs_tilemap.

  4. alice-i-cecile commented on Mar 9, 2022

    @alice-i-cecile
    Member

    This explanation and fix makes sense to me, but I'm also not a rendering expert. @mdenchev I think this is ready for a PR; letting others easily test and examine the code in context will probably make it easier to detect any problems.

  5. mdenchev commented on Mar 9, 2022

    @mdenchev
    Author

    Thinking about it a little more, maybe the shrink should be of a minimal size, say 1e-6, to take into account numerical issues but no affect other sampling methods.

  6. alice-i-cecile commented on Mar 9, 2022

    @alice-i-cecile
    Member

    Thinking about it a little more, maybe the shrink should be of a minimal size, say 1e-6, to take into account numerical issues but no affect other sampling methods.

    If we can do this while still fixing the issue, that makes sense to me.

  7. wilk10 commented on Apr 9, 2022

    @wilk10
    Contributor

    i think this is the same issue as #1949 , which has more information there. should we close this and continue the discussion there?

  8. alice-i-cecile commented on Apr 9, 2022

    @alice-i-cecile
    Member

    Agreed. Consolidating into #1949.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-RenderingDrawing game state to the screenC-BugAn unexpected or incorrect behavior

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions