Add a repeating texture example #11161

IQuick143 · 2023-12-31T04:48:22Z

Objective

Make an example on how to set custom properties on samplers and achieve a repeating texture.
Fixes Add an example demonstrating how to repeat textures #11136
Progress towards Tiled/repeating textures #399

Changelog

Started with the commits from the PR #11109
Added another commit that

Re-orders functions (main -> setup -> other)
Generalises the function quad_1_2 to quad_with_custom_uv to make the sample code more useful and reusable
Added a comment explaining why we are manipulating UVs and how texture sampler tie into that

Further details

Should we build the quad mesh from scratch, or show how to change the UVs on a mesh created by bevy::render::mesh::shape::Quad?
Probably wait on AssetServer:: load_with_settings ignores the settings if file was already loaded with default settings #11111 being fixed to resolve the need for a duplicate texture.
Could/should the code for setting the sampler properties be written nicer (lines setting the actual properties are effectively duplicated for the two cases)?
Any further suggestions?

github-actions · 2023-12-31T04:48:33Z

Welcome, new contributor!

Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨

IQuick143 · 2023-12-31T04:51:51Z

@alice-i-cecile @IceSentry would you mind reviewing this?

stepancheg · 2023-12-31T16:55:53Z

examples/asset/texture_sampler.rs

+    // Instead of using a standard quad with UV coordinates in the range `0..=1` (spanning the entire texture),
+    // we create a quad with UV coordinates in the range `-1..=2`, going beyond the texture by one entire unit in each direction.
+    // Texture behaviour then depends on the selected sampler mode.
+    // By default (`ImageAddressMode::ClampToEdge`) the out-of-bounds UV coordinates will map to the edge of the texture.
+    // When `ImageAddressMode::Repeat` is specified the out-of-bounds UV coordinates will repeat the texture.


My opinion, which has very little weight in this context would be this:

Less text is better, because those who read the example, would have to spend less time and energy reading it.

standard quad with UV coordinates

There's no such thing as "standard quad with UV coordinates". For example, if you have a mesh for a sphere, it usually contains hundreds quads, and none of them use coordinates 0..1. Quads with XY and UV coordinates 0..1 are used mostly in examples (and probably in Minecraft).

Also the comments below "usual UV range 0..=1", "custom UV coordinates" are not exactly correct for the same reason.

Texture behaviour then depends on the selected sampler mode

Texture behavior always depends on the sampler more, not "then". Perhaps you meant "Texture behaviour then depends on the selected address mode", which indeed then depends, because when UV is in range 0..=1, it does not depend on address mode.

By default (ImageAddressMode::ClampToEdge) the out-of-bounds UV coordinates will map to the edge of the texture.
When ImageAddressMode::Repeat is specified the out-of-bounds UV coordinates will repeat the texture

These would be the redundant comments because they

repeat the mostly obvious enum variant name

repeat enum documentation

basically repeat what is said several lines above where the image is loaded (where this comments belongs)

Again, feel free to ignore this comment.

You make a good point in regards to the sampler vs address mode, that should be reworded.

As per what a standard quad UV means, you're right that when you're talking about quad as a graphics primitive (part of a mesh), it could have any UVs for any reason, but when one talks about a quad as a mesh itself (a geometric quadrilateral) it does occur to me only as natural to use UV coordinates as usual.
I said usual UV range 0..=1, because in that range lie UVs of most meshes people come into contact with (3D designers don't typically make their UVs sample outside of a texture from my experience).

You're right the comment does seem to give a bunch of redundant information that could be found in the docstrings for ImageAddressMode, though the purpose of the comment is mainly to explain why it takes user-handled in UV coordinates + a change in address mode to achieve the desired repeating effect, because just changing the adress mode would not actually produce any noticeable effects.
I'll consider shortening this part to a refference to the appropriate docs.

Anyway if someone gives me another opinion on this, I'll rewrite it.

stepancheg · 2023-12-31T16:59:04Z

Generalises the function quad_1_2 to quad_with_custom_uv to make the sample code more useful and reusable

In my very subjective opinion, examples are not meant to optimized for reading, not for code reuse.

rparrett · 2024-01-12T17:05:16Z

Could you edit your PR description to include that this also fixes #399?

Also, I think we can simplify and unblock this by just removing the non-repeating texture from the example. Discussion: #11136 (comment)

rparrett · 2024-01-12T17:10:27Z

examples/asset/texture_sampler.rs

+    let mut mesh = Mesh::new(PrimitiveTopology::TriangleList);
+    mesh.insert_attribute(
+        Mesh::ATTRIBUTE_POSITION,
+        vec![[0., 0., 0.], [1., 0., 0.], [1., 1., 0.], [0., 1., 0.]],


I think the vertex positions should match those of Bevy's "default quad." Users putting together code from various Bevy examples might be surprised that this quad isn't "anchored" on its center like the default one.

I personally feel like we should just modify the default Quad here, and that the example would be a bit more self-documenting with a function more like repeating_quad(times: f32). I have an example using that method here: #11136 (comment)

Modifying the default Quad might be better, I'll at least fix the mesh coordinates.

A better function name is welcome, although repeating_quad(times: f32) feels a little suggestive, because it suggest using this mesh would automatically cause repeating, whereas it only sets up a mesh that can be repeated over, but maybe this is not such a problem given the comments explaining the mechanisms.

IQuick143 · 2024-01-13T13:21:01Z

Could you edit your PR description to include that this also fixes #399?

Also, I think we can simplify and unblock this by just removing the non-repeating texture from the example. Discussion: #11136 (comment)

Issue #399 added!
I think the example is better when it shows directly the effects of different ImageAdressModes, but that's maybe just my opinion. The argument that it would unblock this is somewhat compelling but I think that since #11111 needs to be fixed anyway, this can probably wait (perhaps reconsider if this block would decide if this PR makes it into 0.13 or not).

janhohenheim · 2024-03-06T13:35:52Z

One point against this fixing #399:
On 3D, most users will want to repeat textures on GLTFs, e.g. a texture on a house that repeats its facade, or a texture on a tree that repeats the bark, etc.
As far as I know, it is still not possible to set these things to repeat after the texture has already been loaded from a GLTF. Please correct me if I'm wrong.
Right now, the best way to do this is to scale the UVs through the uv_transform GLTF extension to have enough repetitions, which does not work when a model is rescaled (10 repeats on a small house might be good enough, but a big house might need 30)
Notes that is distinct from #11111 as we are not loading a texture again, but want to change the settings after it was loaded.

alice-i-cecile

I don't have any blocking concerns: I think that the Quad swap would be nice, but should be done seperately.

bugsweeper · 2024-05-01T12:10:07Z

Please correct me if I'm wrong.

@janhohenheim StandardMaterial also has uv_transform since #11904 , which is done exactly for GLTF

janhohenheim · 2024-05-01T12:51:06Z

@bugsweeper yep, I wrote that PR 😉
Thanks for noting this though, so that others may know as well :)

# 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)

stepancheg and others added 2 commits December 31, 2023 05:29

Example how to repeat texture

ea757b1

Improve docs on texture_sampler example

2b3e218

MrGVSV added C-Examples An addition or correction to our examples A-Assets Load files from disk to use for things like images, models, and sounds labels Dec 31, 2023

MrGVSV requested review from alice-i-cecile and IceSentry December 31, 2023 05:14

stepancheg reviewed Dec 31, 2023

View reviewed changes

mockersf added the S-Blocked This cannot move forward until something else changes label Dec 31, 2023

rparrett reviewed Jan 12, 2024

View reviewed changes

alice-i-cecile approved these changes Mar 6, 2024

View reviewed changes

matiqo15 mentioned this pull request May 5, 2024

Example with repeated texture #13176

Merged

alice-i-cecile closed this in #13176 May 5, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a repeating texture example #11161

Add a repeating texture example #11161

IQuick143 commented Dec 31, 2023 •

edited by alice-i-cecile

Loading

github-actions bot commented Dec 31, 2023

IQuick143 commented Dec 31, 2023

stepancheg Dec 31, 2023

IQuick143 Dec 31, 2023

stepancheg commented Dec 31, 2023

rparrett commented Jan 12, 2024

rparrett Jan 12, 2024

IQuick143 Jan 13, 2024

IQuick143 commented Jan 13, 2024

janhohenheim commented Mar 6, 2024 •

edited

Loading

alice-i-cecile left a comment

bugsweeper commented May 1, 2024 •

edited

Loading

janhohenheim commented May 1, 2024 •

edited

Loading

Add a repeating texture example #11161

Add a repeating texture example #11161

Conversation

IQuick143 commented Dec 31, 2023 • edited by alice-i-cecile Loading

Objective

Changelog

Further details

github-actions bot commented Dec 31, 2023

IQuick143 commented Dec 31, 2023

stepancheg Dec 31, 2023

Choose a reason for hiding this comment

IQuick143 Dec 31, 2023

Choose a reason for hiding this comment

stepancheg commented Dec 31, 2023

rparrett commented Jan 12, 2024

rparrett Jan 12, 2024

Choose a reason for hiding this comment

IQuick143 Jan 13, 2024

Choose a reason for hiding this comment

IQuick143 commented Jan 13, 2024

janhohenheim commented Mar 6, 2024 • edited Loading

alice-i-cecile left a comment

Choose a reason for hiding this comment

bugsweeper commented May 1, 2024 • edited Loading

janhohenheim commented May 1, 2024 • edited Loading

IQuick143 commented Dec 31, 2023 •

edited by alice-i-cecile

Loading

janhohenheim commented Mar 6, 2024 •

edited

Loading

bugsweeper commented May 1, 2024 •

edited

Loading

janhohenheim commented May 1, 2024 •

edited

Loading