bevy icon indicating copy to clipboard operation
bevy copied to clipboard

Add an example demonstrating how to repeat textures

Open alice-i-cecile opened this issue 1 year ago • 10 comments

This is a simple adoption of https://github.com/bevyengine/bevy/issues/11109.

alice-i-cecile avatar Dec 30 '23 00:12 alice-i-cecile

I was looking at that PR briefly earlier and had a few questions for @IceSentry or other rendering/assets folks. I'll post them here.

  • Is #11111 likely to be fixed, and if so, should we wait on that so we don't need to add an image to the repo that will be deleted in short order?
  • Does it make sense to build the mesh by creating and modifying a Quad? We show off this pattern in the generate_custom_mesh example, and for the sake of this example it seems like only the UV attribute needs to differ from the default. e.g.
    fn repeating_quad(times: f32) -> Mesh {
        let mut mesh: Mesh = shape::Quad::default().into();
        let uv_attribute = mesh.attribute_mut(Mesh::ATTRIBUTE_UV_0).unwrap();
        // The format of the UV coordinates should be Float32x2.
        let VertexAttributeValues::Float32x2(uv_attribute) = uv_attribute else {
            panic!("Unexpected vertex format, expected Float32x2.");
        };
        // The default `Quad`'s texture coordinates are in the range of `0..=1`. Values outside
        // this range cause the texture to repeat.
        for uv_coord in uv_attribute.iter_mut() {
            uv_coord[0] = uv_coord[0] * times;
            uv_coord[1] = uv_coord[1] * times;
        }
    
        mesh
    }
    
    

rparrett avatar Dec 31 '23 03:12 rparrett

IMO that issue is likely to be fixed quite easily: I'd prefer to wait on the fix rather than polluting the main git history.

alice-i-cecile avatar Dec 31 '23 03:12 alice-i-cecile

IMO that issue is likely to be fixed quite easily

Is it?

This code

https://github.com/bevyengine/bevy/blob/70b0eacc3b5ccf12028fce925d1efc5ae31b5699/crates/bevy_asset/src/server/info.rs#L64

maps assets by path. To work correctly, this should be mapped by a pairs: (path, Option<MetaTransform>). But there's no hash/equality for MetaTransform. Should be use pointer hash/equality in MetaTransform?

For the reference, the definition of MetaTransform:

https://github.com/bevyengine/bevy/blob/70b0eacc3b5ccf12028fce925d1efc5ae31b5699/crates/bevy_asset/src/meta.rs#L9

wait on the fix rather than polluting the main git history

The opposite might be practically more convenient: land the example, and then remove extra copy during the fix demonstrating the issue is actually fixed.

stepancheg avatar Dec 31 '23 16:12 stepancheg

wait on the fix rather than polluting the main git history

The opposite might be practically more convenient: land the example, and then remove extra copy during the fix demonstrating the issue is actually fixed.

While the new example would be nice, there is no rush, and adding it now doesn't add that much value rather than waiting a bit on how to fix the underlying issue.

I don't remember that scenario (loading the same file with different settings) being discussed in the recent asset rework. Let's wait for more people to chime in on #11111. It's an holiday season for many, so it can take a bit of time.

mockersf avatar Dec 31 '23 18:12 mockersf

@mockersf Do you think we really need to show off non-repeating textures in this example? Could we just strip that part of the example out?

rparrett avatar Jan 11 '24 22:01 rparrett

@mockersf Do you think we really need to show off non-repeating textures in this example? Could we just strip that part of the example out?

Yup that could work!

mockersf avatar Jan 12 '24 05:01 mockersf

Do you think we really need to show off non-repeating textures in this example?

If we remove non-repeating texture example, it won't be obvious what this example demonstrates: that is texture is not repeating by default, side by side (both code and visually) what it means.

In particular, what code is mandatory to just make textures work, and what is the delta to make it repeating.

stepancheg avatar Jan 12 '24 18:01 stepancheg

I think that's sort of inherent in an example where there all these extra steps to do a repeating texture.

The important part in my eyes is showing users how to accomplish this. It's not trivial to do right now, and users really want to do it.

I can see the utility in showing users the results side-by-side, especially in the example showcase. But it doesn't seem totally necessary and skipping it is a way to unblock this.

rparrett avatar Jan 12 '24 18:01 rparrett

I think that's sort of inherent in an example where there all these extra steps to do a repeating texture.

There are steps to step up texture, and there are extra steps to make it repeating. Example as is shows that setting up image sampler is extra steps.

skipping it is a way to unblock this

Example is not really blocked. Two copies of the file until the issue is fixed is a small price to pay.

Alternatively, an image can be loaded directly, without asset server.

stepancheg avatar Jan 12 '24 19:01 stepancheg

I was looking at that PR briefly earlier and had a few questions for @IceSentry or other rendering/assets folks. I'll post them here.

* Does it make sense to build the mesh by creating and modifying a `Quad`?
  We show off this pattern in the `generate_custom_mesh` example, and for the sake of this example it seems like only the UV attribute **needs** to differ from the default.
  e.g.
  ```rust
  fn repeating_quad(times: f32) -> Mesh { ... }
  ```

I was looking for a way to make a repeated texture today.

I was using the plane mesh builder and I expected a way to control UVs there.

Currently that isn't supported, and the UVs are always in the [0.0, 1.0] range on the generated mesh.

// Example, current
let mesh = Plane3d::default().mesh().size(5.0, 5.0);
commands.spawn(PbrBundle {
    mesh: meshes.add(mesh),
    ..default()
})

not exactly sure what the API should look like but it would be great if something like

// Example, with modified UVs
let mesh = Plane3d::default().mesh().uv(Vec2::new(3.0, 2.0)).size(5.0, 5.0);
commands.spawn(PbrBundle {
    mesh: meshes.add(mesh),
    ..default()
})

which would make the mesh have UVs from [0.0, 3.0] in the X dir and [0.0, 2.0] in the Y dir.

I think that would be a more user friendly approach than having to go via mesh attributes

torsteingrindvik avatar Apr 26 '24 08:04 torsteingrindvik

I think job can be done easier since uv_transform (which is actualy proportions) has been added in StandardMaterial in #11904 . Here I explained how to repeat UV, you can follow that thread. Here is result of attempt image

bugsweeper avatar May 01 '24 11:05 bugsweeper

I didn't use *.meta files for sampler configuration (I used configuration in code too), because meta files mask is in .gitignore

bugsweeper avatar May 02 '24 11:05 bugsweeper