Add visible section context to RenderLevelStageEvent

[embeddedt]

This PR introduces the ability to iterate through visible chunk sections in the RenderLevelStageEvent. This allows mods that want to overlay their rendering on top of terrain to do so more efficiently, since they can skip rendering their content in sections that have already been deemed invisible by the vanilla terrain renderer. Currently, mods that do this can only do a basic render distance check, and/or frustum cull each section in render distance a second time.

Performance-wise, there is no regression if a mod is not using the feature, as we reuse the existing visible section list already populated by vanilla's renderer.

The IRenderableSection interface is introduced both for a simpler/better documented API surface, as well as to allow other mods that take over the terrain renderer to implement the API without being locked into the various side effects of using vanilla's section renderer objects. The surface is intentionally kept small for now; it can be expanded later if there is a use case.

The new gametest added provides an example of how to use the API.

[neoforged-pr-publishing]

• Publish PR to GitHub Packages

Last commit published: aa9a997c1a4353b0f018897e53376d63e4e219e3.

PR Publishing

The artifacts published by this PR:

• 📦 net.neoforged:neoforge:21.1.23-pr-1472-add-visible-section-context
• 📦 net.neoforged:testframework:21.1.23-pr-1472-add-visible-section-context

Repository Declaration

In order to use the artifacts published by the PR, add the following repository to your buildscript:

repositories {
    maven {
        name 'Maven for PR #1472' // https://github.com/neoforged/NeoForge/pull/1472
        url 'https://prmaven.neoforged.net/NeoForge/pr1472'
        content {
            includeModule('net.neoforged', 'neoforge')
            includeModule('net.neoforged', 'testframework')
        }
    }
}

MDK installation

In order to setup a MDK using the latest PR version, run the following commands in a terminal.
The script works on both *nix and Windows as long as you have the JDK bin folder on the path.
The script will clone the MDK in a folder named NeoForge-pr1472.
On Powershell you will need to remove the -L flag from the curl invocation.

mkdir NeoForge-pr1472
cd NeoForge-pr1472
curl -L https://prmaven.neoforged.net/NeoForge/pr1472/net/neoforged/neoforge/21.1.23-pr-1472-add-visible-section-context/mdk-pr1472.zip -o mdk.zip
jar xf mdk.zip
rm mdk.zip || del mdk.zip

To test a production environment, you can download the installer from here.

[Technici4n]

Is this really necessary?

[embeddedt]

It makes it a bit easier to determine what was patched in when not looking at the patch file, because there are inner classes after the patched-in logic, but I can remove it if you really don't like it.

[jellysquid3]

It's worth mentioning that for mods like Sodium, the "render sections" it has do not store this information. Adding this information would increase the heap size by 10+ MB (for the container objects + block positions + bounding boxes) at render distance 32, or would otherwise require a lot of allocations at runtime when mods iterate over sections (ex. iterating 10,000 chunks at 60 times a second would allocate say 50 MB/s of garbage on the heap.)

Furthermore, trying to iterate over tens of thousands of potentially visible chunks, when those objects are all over the heap, will be very slow. Each render section object will be scattered across the heap and probably cause a significant number of Cache + TLB misses on the CPU. This is already a major source of slowness in the game and having multiple mods doing it each frame (for whatever effect) is not great.

In my opinion, it would be possible to get around this overhead by instead creating a custom iterator interface that has methods to inspect the position/bounds/etc of each section. Then, such an implementation of the iterator (i.e. Sodium) could avoid allocating objects on the fly, and use whatever data structure is most optimal behind the scenes, without exposing a unique IRenderableSection per iteration.

[jellysquid3]

Also, if you don't mind me asking, what is the use case for IRenderableSection#getCompiledRenderLayers() here?

At least for the case of Sodium, we don't actually use the RenderType, rather it gets mapped to a small set of TerrainRenderPass enums (for which there is "Opaque", "Alpha-Tested", and "Alpha-Blended".) So it's not clear how we would implement the interface and it still be useful to mods. Some clarification would be helpful.

[marchermans]

It's worth mentioning that for mods like Sodium, the "render sections" it has do not store this information. Adding this information would increase the heap size by 10+ MB (for the container objects + block positions + bounding boxes) at render distance 32, or would otherwise require a lot of allocations at runtime when mods iterate over sections (ex. iterating 10,000 chunks at 60 times a second would allocate say 50 MB/s of garbage on the heap.)

Furthermore, trying to iterate over tens of thousands of potentially visible chunks, when those objects are all over the heap, will be very slow. Each render section object will be scattered across the heap and probably cause a significant number of Cache + TLB misses on the CPU. This is already a major source of slowness in the game and having multiple mods doing it each frame (for whatever effect) is not great.

In my opinion, it would be possible to get around this overhead by instead creating a custom iterator interface that has methods to inspect the position/bounds/etc of each section. Then, such an implementation of the iterator (i.e. Sodium) could avoid allocating objects on the fly, and use whatever data structure is most optimal behind the scenes, without exposing a unique IRenderableSection per iteration.

Allthough I personally understand your ideas here, and yes I can see that this could potentially cause issues with sodium or other renderers, when this was discussed internally, a balance was struck between a maintainable API, and a useable API surface.

From discussions between the OP and the maintainers, I think we could offer a compromise:
Instead of exposing/requiring a List we could create a custom interface that accepts a Consumer<IRenderableSection> that is used as a virtual iterator with a forEach method on it. This ensures that we can trivially implement this in vanilla, and you can implement this for your own backing storage. And do the iteration in what ever way you see fit, yes it still means you need to implement this interface on some object that you have internally that you use yourself to keep track of the visible sections in the game, just as we do with vanilla, but it should prevent allocations from needing to occur.

We really do not want this iterator to have some methods to check properties of the next available section (at least that is what I infer from your inspect the position/bounds/etc of each section), because this would be a major pain to keep compatible while this API evolves over time. That is the main reason for IRenderableSection to even be used in the first place, instead of requiring a hard RenderSection object, which would be even more pain for you i imagine.

[marchermans]

Also, if you don't mind me asking, what is the use case for IRenderableSection#getCompiledRenderLayers() here?

At least for the case of Sodium, we don't actually use the RenderType, rather it gets mapped to a small set of TerrainRenderPass enums (for which there is "Opaque", "Alpha-Tested", and "Alpha-Blended".) So it's not clear how we would implement the interface and it still be useful to mods. Some clarification would be helpful.

The API idea here is, to allow for easy detection of the rendertypes in the subchunk, so that both empty and partially filled, or subsection filled with a particular rendertype can be detected by modders that use this event to react to certain areas of the world, and change their dynamic geometry.

I assume you internally have some sort of map that maps from vanilla RenderTypes to your custom enum, and I assume that this is reversable, as such, you could trivially detect which kinds of geometry sodium has rendered in a section, and return the relevant rendertypes that back those entries.

As such I really see no problem in implementing this API, or am I overlooking something here?

[jellysquid3]

Instead of exposing/requiring a List we could create a custom interface that accepts a Consumer<IRenderableSection> that is used as a virtual iterator with a #forEach method on it.

I'm not sure what this changes exactly. The current revision of the pull request would also allow us to implement Iterable and have next() use whatever storage is appropriate.

My point of contention was that having mods iterate over many (potentially irrelevant) sections while looking for a specific render type is a lot of overhead regardless of renderer.

If there was an API such as #getSectionsWithRenderType(RenderType) that returned an iterator over just the relevant sections (with the renderer doing whatever is most optimal) that would be a lot more preferable. For NeoForge, it would be sufficient to just implement a for-loop over the visible section list, check which RenderTypes are present, and return those.

...because this would be a major pain to keep compatible while this API evolves over time...

What I'm suggesting is that one would have an Iterator interface with #next(), #getOrigin(), #getBoundingBox(), etc. For your case with Vanilla, it would make sense to me that you can just store the RenderSection privately within the iterator, and have #getOrigin() and such just access the value on the current render section. I'm not sure that I understand why this would necessarily be more difficult to maintain than an iterator over objects with the same data.

But that gets into the other problem, which is that we don't even have RenderTypes in the first place...

I assume you internally have some sort of map that maps from vanilla RenderTypes to your custom enum, and I assume that this is reversible...

The mapping of RenderType to our render passes is one-way, unfortunately. For example, CUTOUT and CUTOUT_MIPPED are merged into "Alpha-Tested", with TRANSLUCENT and TRIPWIRE being mapped to "Alpha-Blended". The other information about mip-mapping and alpha cutoffs is stored in the vertex data, so we don't need as many draw calls/render passes.

We could keep track internally of what RenderTypes were encountered when building the chunk, but that information doesn't mean anything to our mod. And it isn't clear how useful that would be, since mods can't access the vertex data or anything else for that RenderType on the section.

It would ultimately lead to the situation where each mod is ignoring the RenderType information, and instead mapping each chunk section to their own render object, which contains a list of their own render types that needs to be dealt with. At which point, the interface as suggested does not make a lot of sense to me.

[jellysquid3]

Also, thanks for giving the time to talk about this. I'm not trying to shoot the proposed solution down. Just trying to think of some way that we can work with it on our side.

[marchermans]

What I'm suggesting is that one would have an Iterator interface with #next(), #getOrigin(), #getBoundingBox(), etc. For your case with Vanilla, it would make sense to me that you can just store the RenderSection privately within the iterator, and have #getOrigin() and such just access the value on the current render section. I'm not sure that I understand why this would necessarily be more difficult to maintain than an iterator over objects with the same data.

I understand that, but that is basically not implementable without a performance hit on vanilla at all, but I willing to look into it a bit more.

Obviously given that i have a significant task load at the moment, if you could provide a pseudo implementation of your idea, that works with vanilla, then I would be open to address it immediately.

If there was an API such as #getSectionsWithRenderType(RenderType) that returned an iterator over just the relevant sections (with the renderer doing whatever is most optimal) that would be a lot more preferable. For NeoForge, it would be sufficient to just implement a for-loop over the visible section list, check which RenderTypes are present, and return those.

That is an options sure @embeddedt as the OP what do you think would such an approach be equally acceptable?

[embeddedt]

If the Set<RenderType> API is problematic we can just remove it and only provide the info on whether the section is empty/non-empty. The rest of the concerns here should be addressable with a custom mutable IRenderableSection object that returns the appropriate data for the backing render section. We can add a note to the API that the identity/longevity of the returned IRenderableSection should not be relied on.

[neoforged-automation]

@embeddedt, this pull request has conflicts, please resolve them for this PR to move forward.