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

Line2D texture mapping details/behavior #10297

Open
runningopenloop opened this issue Nov 21, 2024 · 3 comments
Open

Line2D texture mapping details/behavior #10297

runningopenloop opened this issue Nov 21, 2024 · 3 comments
Labels
area:class reference Issues and PRs about the class reference, which should be addressed on the Godot engine repository enhancement topic:2d

Comments

@runningopenloop
Copy link

Your Godot version: v4.2.2.stable.official [15073afe3]

Issue description: Details on how texture mapping works with Line2D.

URL to the documentation page (if already existing): https://docs.godotengine.org/en/stable/classes/class_line2d.html


I thought clarification/details on how the texture maps to the Line2D given settings is in order.

When using the "width curve", the width_curve value appears to be a multiplier to the width specified in pixels for actual width of line as interpolated along.

When making a change to the texture width (or other settings), you may need to toggle visible off/on to see update.

As for texture mapping to line. These results were obtained using a 2D Linear Gradient Texture.

  • When using line width of 10 pixels, and a texture of 256 width x 256 height. UV appears to be wrapping at 10 pixels wide.
  • When using line width of 10 pixels, and a texture of 256 width x 64 height. UV appears to be wrapping at 40 pixels wide.

Therefore, it seems for texture mapping the line. The height of texture relates to the width of line. Wrapping of the "U" coordinate will happen at TEXTURE_WIDTH / TEXTURE_HEIGHT * LINE_WIDTH (in pixels).


It strikes me this sort of documentation would be helpful to know how to use the Line2D. Is the above reasonable to expect from the documentation? Is the reason it is not there already is due to massive nature of documenting everything?

It would be helpful for me to make an update and do a pull request? Are examples desired as above, or just the technical detail?

Is it a bug that I must toggle visibility to update, or reasonable to expect and perform when making these sorts of changes.

@tetrapod00 tetrapod00 added area:class reference Issues and PRs about the class reference, which should be addressed on the Godot engine repository topic:2d labels Nov 21, 2024
@tetrapod00
Copy link
Contributor

tetrapod00 commented Nov 21, 2024

Hi! Thanks for your interest in contributing. I can't answer all the questions about Line2D specifically, but as for your general questions:

Is the above reasonable to expect from the documentation?

Maybe, though the class reference tries to be concise and so sometimes leaves details like this out. I think if you can find a good general and concise way to communicate the behavior it could be a good addition.

Is the reason it is not there already is due to massive nature of documenting everything?

Very likely! There is always more work to be done in documentation, as you can see by the 800+ open issues we have here.

It would be helpful for me to make an update and do a pull request?

Yes, though I should say up front that it may need some revisions, and not every PR ends up merged for various reasons (I personally have several docs PRs stuck in review in the main repo). One thing that can make it easier to review is to provide some examples of what you tested, or possibly even an MRP demonstrating the behavior.

Changes to the class reference are made in the main repo, not here, and we have a tutorial for it here: https://docs.godotengine.org/en/stable/contributing/documentation/updating_the_class_reference.html. Even though you would be editing the main repo, you're not changing C++, just XML files.

Are examples desired as above, or just the technical detail?

Examples are okay but ideally you can also find a way to describe the general behavior, and use the examples to clarify the general behavior.

Is it a bug that I must toggle visibility to update, or reasonable to expect and perform when making these sorts of changes.

I'm not familiar with the details of this class, but that sounds like a bug to me. There are a few longstanding bugs that might as well be intended at this point, though. See if you can find a related issue in the main repo, and if not, feel free to report it.

I do see that you listed 4.2.2 as your version, so you might check 4.3 and see if its improved at all. Also, if you're documenting things, iyou should probably test the behavior on at least 4.3, and maybe a 4.4 dev version.

I hope that answers your questions, feel free to stop by in the docs channel of the contributors chat if you have any more.

@runningopenloop
Copy link
Author

runningopenloop commented Nov 22, 2024

Thanks for the kind reply and the inline responses.

So this feedback relates to:

  • Behavior of the class when LineTextureMode LINE_TEXTURE_TILE = 1
  • width property
  • (Not width_curve property --> doesn't impact repeat)
  • texture height (and width).

For proper operation:

  • Line2D Class: Fill Section, Texture Mode Property = Tile. This sets the behavior user is looking for... a tiled texture along line.
  • Dependencies include:
    • Line2D Class's inherited CanvasItem's Class: Texture Section, Repeat Property = Enabled is required.
    • Line2D Class: Fill Section, Texture Property's
      • Repeat Section, Repeat Property, seems to not be critical to proper behavior but has a subtle impact.
      • This repeat is specific to the GradientTexture2D and therefore beyond the scope of documentation in Line2D.
      • Although there seems to be a subtle interaction when Tiled texture is used with GraidentTexture2D... but understanding the details and what is going on is probably high hanging fruit.

My inclination would be to add information to the LineTextureMode enum.

  • This sectional already advises the repeat requirement of the CanvasItem.texture_repeat.
    • It would be nice to understand what the subtle interaction with Line2D tile map but doesn't seem important.
  • I plan to elaborate that the Line2D.width is a factor along with texture width and height to repeat. if I manage to get the workflow squared away --> seems yes.

I did confirm that change texture size didn't cause update to display in both 4.3 and 4.4-dev5, so I'll report it.

@tetrapod00
Copy link
Contributor

I set up a basic test scene and, and yeah it seems like the behavior is relatively complex. If you can figure out exactly what it is, this will be a good thing to document.

Godot_v4.4-dev5_win64_FN84kuJpvS.mp4
Godot_v4.4-dev5_win64_XrhG8aVLY5.mp4

I think that your initial suggestion was correct, from a little experimenting: Wrapping of the "U" coordinate will happen at TEXTURE_WIDTH / TEXTURE_HEIGHT * LINE_WIDTH (in pixels).

The behavior here is definitely complex enough that when you submit a PR with the changes, you should include either:

  • Several images and/or videos showing your testing setup and the results
  • A full Minimal Reproduction Project that features an interactive way to see the behavior
  • Or both.

Most reviewers will not be casually aware of the behavior here.

Thank you very much if you decide to make a PR for this, it's a very nuanced behavior that is not immediately obvious.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:class reference Issues and PRs about the class reference, which should be addressed on the Godot engine repository enhancement topic:2d
Projects
None yet
Development

No branches or pull requests

2 participants