This is not a good way to go about doing this. Render code should not be present or relevant to the server (which capabilities are), Capabilities also add other complexity when present that is additional overhead that is strictly not necessary in a render context (the cost of getCap is significant to be running every frame).

diesieben07 wrote in #8048

As an alternative to the event, we could add a capability that mods can add to either their own or other ItemStacks which would then be called to do this rendering. That way no event dispatching needs to happen but the system is still pluggable even for items from other mods or vanilla.

Which is why this thing was designed as a capability.
The only real thing exposed to the server is the registration of the capability, but as adding IItemDecorations is mostly done using a helper in ForgeHooksClient and rendering obviously happens clientside, mod authors should be able to know, that this code should only be used on the logical Client.

Getting the capability, once initialized, is just an array lookup through the ICapabilityProvider in CapabilityDispatcher until one returns the cap you want.
As shown by the performance test I did, I don't think it's as bad as you describe it to be (especially comparing this to RenderLivingEvent.Pre and Post). A List in the ItemStack class will be faster, but that is imo much more exposed on the serverside and has a higher maintenance cost.

Also during normal gameplay only 9 ItemStacks will be usually visible, so the impact in normal gameplay is more likely around/below 0.25ms on each frame

What kind of an event?
one fired for each ItemStack being rendered at the same patch location this PR does this atm similar to #7693 or #8048?
one fired for each gui layer (see #6927)?
or one for each frame rendered?
The first one might has performance concerns as previously commented to the two PRs, but I can create that and provide the data, if wanted.
The latter two have the added difficulty to gather all render positions discord conversation. This would require to get the position of dragged stacks, creativetab icons, merchantoffers etc. to be collected (and a way for mods to add their "slots"/rendered Items to that).
Another way to handle this, would be to not add those decorations to an ItemStack using a Capability, but to add them using the IClientItemExtension added by #8786. That however would be a breaking change as the implementations now need a field to store the Decorations (which would be fine, since no RB is out yet).

Just for my own knowledge: Why is rendering with capabilities on itemstacks such a big problem?

Because capabilities are not designed for it and they are not supposed to be Client only. But i can ask internally if we might want to add support for it.

PR's for events that fire on stack rendering have historically been automatically denied, so it may only be accepted as an extension to the client item interface

The ItemDecorations have been moved to the IClientItemExtension. Notable changes:
Each Item has by default it's own IClientItemExtension created by IClientItemExtension#DEFAULT_FACTORY so that each one has it's own List Instance.
This is a breaking change, since IClientItemExtension got a new non default method

Just make the method default and return Collections.emptyList() to make it non-breaking. Then you also don't need all these default impl classes.

Nevermind, I see the intention is to be able to add to other items, which would require an existing mutable list to be accessible.

You could, however, get around the binary break by adding two methods - one getItemDecorators that returns an immutable view, and one addItemDecorator that allows for adding a decorator.

Then getItemDecorators could default to Collections.emptyList and addItemDecorator could default to doing nothing until implementers fix their implementations.

That is true, but would mean that mod authors that already implement this interface might not be aware of this change. I've also considered a throw in dev scenario, but that would only throw if someone tries to add a a decorator in dev which might not happen. Logging the default impl with warning might be an option, but also has the problem of only being fixed if someone reports it to the mod author, which is hard to do as there is no context to which item a decorator was tried to be added.

The List should probably not live in IClientItemExtensions, but rather another client-only data structure for the items, so that mods using IClientItemExtensions do not have to care about it.

diesieben sniped me a little here:
I don't think putting the "burden" for such a feature on everyone implementing IClientItemExtension is a good idea. Every implementation would either be exactly the same or completely broken because the modder thinks "I don't need this feature -> return unmodifiable empty list".

They could extend ClientItemExtensionImpl and override the methods they want similar to how I did it with the CustomArmorModelTest. A seperate client only object stored in Item would increase the patch size and maintenance cost of such a feature as well as duplicate a bit of the code already used by IClientItemExtension and potentially hinders discoverability of this feature.
If another client-only data structure is wanted, I would be open for name suggestions.

That would still make it a breaking change because you would need to enforce the use of ClientItemExtensionImpl by changing the generic type in the consumer in Item#initializeClient() and also technically still put the burden on the modder.

It's a breaking change, but there was not Recommended Build yet and with this interface being kinda new I doubt many people will be effected by this break. The generic change in initializeClient is not necessary, and anyone can create their own implementation of IClientItemExtension and provide the list themselves.

The IClientItemExtension interface is not new, it was just renamed from IItemRenderProperties

Performance tests:

How I acquired the data

• All measures where taken over 10000 frames
• The IItemDecoration used for this is the StackSizeDurabilityBar from the TestMod or an implementation that does nothing for the "without rendering happening" column
• Map implementation used is the Object2ObjectFastOpenHashMap from fastutil
• Jei measurements where taken with default maxsize of 9x16 items
• Jei measurements where taken over GuiEventHandler#onDrawBackgroundPost from this Update to support the breaking changes in forge mezz/JustEnoughItems#2886 PR by pupnewster
• Creative screen measurements where taken with all 5 CreativeRows filled and the hotbar filled (so 54 itemstacks being catched by the measurement)
• Creative screen measurements where taken over the AbstractContainerScreen#render call without JEI
• Time measurements are obtained using System#nanoTime and averaged before rounding to ms with three decimal places
• All the data was obtained with a i5-7500 and Geforce GTX 1060 with a few (but the same programs running)

Time Measurements

JEI

Creative Menu

My Conclusion

Performance is not a big problem, if not too many decorators are used. Most of the code execution time is coming from rendering and not the overhead of this feature, hence modders should be careful what they do with this feature, but providing this feature is not too big of a problem.
Using a Map is surprisingly fast, even though the direct field is usually faster. Patchwise I would say that the added field in Item is not too big of a problem, as a similar patch in the exact same place is already present and the performance benefits might when mods add a lot of items.

Further actions

I'm going to fix the things mentioned by diesieben and add javadocs where necessary.

With the data provided, I await a decision regarding Map or field in Item