Skip to content

Implement dynamic block tinting support with BlockTintsFactory integration#5357

Open
marchermans wants to merge 4 commits intoFabricMC:26.1.2from
marchermans:feature/dynamic-block-tint-sources
Open

Implement dynamic block tinting support with BlockTintsFactory integration#5357
marchermans wants to merge 4 commits intoFabricMC:26.1.2from
marchermans:feature/dynamic-block-tint-sources

Conversation

@marchermans
Copy link
Copy Markdown

TLDR:

Implement a way for mods to provide dynamic tints if they can not know the amount and implementation of block tint sources at startup.


Underlying changes:

  • Provide an API which allows for the direct retrieval of the computed tint values and cache them for the duration of the render.
  • Added a registration endpoint which allows modders to register the factories.

Implementation:

  • Mixin based implementation for the vanilla block model renderer
  • Direct implementation for indigo

@PepperCode1 PepperCode1 added enhancement New feature or request module: renderer indigo Pull requests and issues related to Indigo's implementation of the rendering api priority: medium Medium priority PRs that should get reviews area: rendering labels Apr 27, 2026
Copy link
Copy Markdown
Member

@PepperCode1 PepperCode1 left a comment

Choose a reason for hiding this comment

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

This is a solid and simple API with only a few things that should be changed, but I am concerned about two use cases that this API does not consider. Please comment on how likely you think these are to come up in the future.

  1. Using dynamic tints and static tints at the same time; for example, a block may register static tint sources for indices 0 and 1, and then also want to collect dynamic tints for indices past 1. It is possible to always use the static sources through the dynamic system, but that is less convenient and potentially less efficient (see next point). What would the downside be of always collecting the static sources and then appending dynamic values? Also, ignoring the factory without warning in some cases seems like a footgun; even if the use case remains unsupported, this footgun can be solved by throwing exceptions during registration so that both static sources and a dynamic factory cannot be registered on the same block simultaneously.
  2. Computing dynamic tints lazily, likely for performance; static tints are always computed lazily, so there may be a case where it is desirable to do so for dynamic tints to. Or, perhaps not, as the whole list of tints is computed at once instead of individually per index.

@marchermans
Copy link
Copy Markdown
Author

This is a solid and simple API with only a few things that should be changed, but I am concerned about two use cases that this API does not consider. Please comment on how likely you think these are to come up in the future.

1. Using dynamic tints and static tints at the same time; for example, a block may register static tint sources for indices 0 and 1, and then also want to collect dynamic tints for indices past 1. It is possible to always use the static sources through the dynamic system, but that is less convenient and potentially less efficient (see next point). What would the downside be of always collecting the static sources and then appending dynamic values? Also, ignoring the factory without warning in some cases seems like a footgun; even if the use case remains unsupported, this footgun can be solved by throwing exceptions during registration so that both static sources and a dynamic factory cannot be registered on the same block simultaneously.

2. Computing dynamic tints lazily, likely for performance; static tints are always computed lazily, so there may be a case where it is desirable to do so for dynamic tints to. Or, perhaps not, as the whole list of tints is computed at once instead of individually per index.

I pushed a new commit with your requested changes, and with regards to your questions and how likely you think they happen:

  1. I don't think it will happen often, but if it does it is quite simple to invoke a static tint source directly from a dynamic one for a given index, and put it in the list. The downside to us supporting both is just a more complex setup and mixin. Personally I see both systems as anti to each other, and I doubt that it happens often enough that both are needed in a way that can not be directly invoked from the dynamic one. So the overhead in implementing this properly is quite complicated, because what if the static tints need to be at index 3 or four, which vanilla currently does not even support. So in my eyes the overhead is not worth the percentage of cases where this happens, at least for now. The other downside you already pointed out in 2) which is that the tints are now all precomputed, I will go into that there. I added an exception to the registry which prevents using both. Check if that is something you are interested in and if you think this makes sense in this way.

  2. The pre-computed cache is a performance nick-nack. The users of this API are specialists with models that are dynamic and so they kind of ahead of time know exactly which tints are needed, I don't personally envision a lot of use cases where this would not be the case, but they exist. In those cases people would indeed take a tiny performance hit because of it compared to the static code paths. There is not really something I can do about this, because of the way vanilla has this implemented. And the overhead of making this system more complicated does not weight up against the performance hit in those extreme corner cases.

Greets,

Marc

Copy link
Copy Markdown
Member

@PepperCode1 PepperCode1 left a comment

Choose a reason for hiding this comment

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

Your changes are good and your explanations make sense to me. Thanks.

@modmuss50
Copy link
Copy Markdown
Member

Please add a small test to the relevant test mod, it doesnt even need its own block.

@marchermans
Copy link
Copy Markdown
Author

Please add a small test to the relevant test mod, it doesnt even need its own block.

Done, I added a new block anyway because it was easier to test and figure out.

The block placed will have two sides with different random colors each.

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

Labels

area: rendering enhancement New feature or request module: renderer indigo Pull requests and issues related to Indigo's implementation of the rendering api priority: medium Medium priority PRs that should get reviews

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants