Skip to content

Comments

Refactor names related to textures (low priority)#770

Merged
andrewlook merged 3 commits intomainfrom
justin/textures
Sep 9, 2025
Merged

Refactor names related to textures (low priority)#770
andrewlook merged 3 commits intomainfrom
justin/textures

Conversation

@jkbelcher
Copy link
Collaborator

Did a refactor pass (name changes only) to clarify a few things:

  • iChannel textures
  • ModelCoords and ModelIndex textures

There were places where "coords" was used in reference to both model-derived textures, and places where "coords" was used only in reference to the first texture. Attempting to create consistency I chose:

  • "ModelTextures" refers to both lxmodel-derived textures
  • "Coords" refers to the first texture and "Index" refers to the second texture.

@jkbelcher jkbelcher requested a review from zranger1 August 24, 2025 23:12
Copy link
Collaborator

@zranger1 zranger1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Tested - the iChannel[n] textures and internal coordinate and indexing samplers are all working. I especially appreciate the cleanup of the messy naming around the shadertoy-compatible iChannels. That's been around since before year 1, and it has always been confusing.

Approved, but maybe hold merge 'till after the burn.

@jkbelcher jkbelcher changed the title Refactor names related to textures Refactor names related to textures (low priority) Aug 27, 2025
@andrewlook
Copy link
Collaborator

note: i merged the stacked PR #773 into this branch, then will rebase it into master (to preserve the 3 resulting commits)

@andrewlook andrewlook merged commit 586f387 into main Sep 9, 2025
1 check passed
@andrewlook andrewlook deleted the justin/textures branch September 9, 2025 01:29
@andrewlook
Copy link
Collaborator

thanks for thinking ahead here @jkbelcher to make the naming something that is clearer before we keep adding stuff onto this 🙏

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants