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

Add an example demonstrating how to repeat textures #11136

Closed
alice-i-cecile opened this issue Dec 30, 2023 · 12 comments · Fixed by #13176
Closed

Add an example demonstrating how to repeat textures #11136

alice-i-cecile opened this issue Dec 30, 2023 · 12 comments · Fixed by #13176
Labels
A-Rendering Drawing game state to the screen C-Examples An addition or correction to our examples D-Trivial Nice and easy! A great choice to get started with Bevy

Comments

@alice-i-cecile
Copy link
Member

This is a simple adoption of #11109.

@alice-i-cecile alice-i-cecile added D-Trivial Nice and easy! A great choice to get started with Bevy A-Rendering Drawing game state to the screen C-Examples An addition or correction to our examples labels Dec 30, 2023
@rparrett
Copy link
Contributor

rparrett commented Dec 31, 2023

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 AssetServer:: load_with_settings ignores the settings if file was already loaded with default settings #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
    }

@alice-i-cecile
Copy link
Member Author

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.

@stepancheg
Copy link
Contributor

IMO that issue is likely to be fixed quite easily

Is it?

This code

path_to_id: HashMap<AssetPath<'static>, UntypedAssetId>,

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:

pub type MetaTransform = Box<dyn Fn(&mut dyn AssetMetaDyn) + Send + Sync>;

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.

@mockersf
Copy link
Member

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.

@rparrett
Copy link
Contributor

@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?

@mockersf
Copy link
Member

@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!

@stepancheg
Copy link
Contributor

stepancheg commented Jan 12, 2024

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.

@rparrett
Copy link
Contributor

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.

@stepancheg
Copy link
Contributor

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.

@torsteingrindvik
Copy link
Contributor

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

@bugsweeper
Copy link
Contributor

bugsweeper commented May 1, 2024

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
Copy link
Contributor

bugsweeper commented May 2, 2024

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

github-merge-queue bot pushed a commit that referenced this issue May 5, 2024
# Objective

Fixes #11136 .
Fixes #11161.

## Solution

- Set image sampler with repeated mode for u and v
- set uv_transform of StandardMaterial to resizing params

## Testing

Got this view on example run

![image](https://github.com/bevyengine/bevy/assets/17225606/a5f7c414-7966-4c31-97e1-320241ddc75b)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Examples An addition or correction to our examples D-Trivial Nice and easy! A great choice to get started with Bevy
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants