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

Be able to disable particle culling in the Forge configuration file #189

Closed
Lolothepro opened this issue Oct 24, 2023 · 10 comments
Closed
Labels
1.20 Targeted at Minecraft 1.20 enhancement New (or improvement to existing) feature or request rendering Related to rendering

Comments

@Lolothepro
Copy link
Contributor

No description provided.

@Technici4n
Copy link
Member

What is the point of disabling an optimization, assuming the bugs with it are fixed?

@Lolothepro
Copy link
Contributor Author

Lolothepro commented Oct 24, 2023

What is the point of disabling an optimization, assuming the bugs with it are fixed?

If "alwaysSetupTerrainOffThread", the Forge expert pipeline have a setting to disable it, it might be a good idea to also make a setting to disable particle culling.

@Lolothepro
Copy link
Contributor Author

then, if it's too complicated to do, you shouldn't do it at all.

@Technici4n
Copy link
Member

I get it but it would be nice to have a more fleshed out reasoning to make sure that we understand if someone depends on disabling this or if this is just a low priority "nice to have"...

(BTW, the 45° block fix is not a NeoForge setting, and should not have been a Forge setting.)

@Lolothepro
Copy link
Contributor Author

a low priority "nice to have"

@XFactHD
Copy link
Member

XFactHD commented Oct 24, 2023

If "alwaysSetupTerrainOffThread", the Forge expert pipeline or the 45° block fix have a setting to disable it, it might be a good idea to also make a setting to disable particle culling.

Those three are all bad examples, alwaysSetupTerrainOffThread is a leftover from before the existence of vanilla's "Chunk Builder: Threaded" option and is as far as I can tell entirely superseded by that, the experimental pipeline is not an optimization and, as Tech mentioned, the 45° stuff doesn't have and shouldn't have a config setting.

@sciwhiz12
Copy link
Member

I see no good reason to provide a configuration option for something which (outside of bugs) should never be visible/felt by a user, as particle culling happens on out-of-view particles (to my knowledge).

@sciwhiz12 sciwhiz12 added enhancement New (or improvement to existing) feature or request 1.20 Targeted at Minecraft 1.20 rendering Related to rendering labels Oct 25, 2023
@embeddedt
Copy link
Member

embeddedt commented Apr 26, 2024

I'd like us to revisit this issue and make a final decision. Below I sketch my personal reasoning around the problem. This should not be taken as an official stance of NeoForge.

In general, I suspect the average modded world will fall into one of the following two categories:

  • All particles spawned in the world are roughly uniformly distributed around the player.
  • Most particles spawned in the world originate from one particular location. (For instance, a machine, or some other modded block.)

In the former case, culling should bring a benefit by definition, since even with very rough estimates, it's obvious the player cannot see at least half the world (anything behind them), and culling shouldn't double particle render time. Therefore, there is a benefit to culling the particles over always rendering them.

In the latter case, the particles are likely to only be visible when looking in a certain direction. When not looking in that direction, culling has been tested and confirmed to be a performance benefit as of 1.20.4 (XFactHD tested and saw roughly 4x increase).

The only outstanding concern is therefore the case where the player is actually looking in the direction of those clustered particles. During that time, culling is by definition a regression, since the check will be performed and always return that the particle needs to be rendered. However, it's important to consider that the player is not actually looking in one direction most of the time during regular gameplay. Thus, as long as they are looking away from that location at least 50% of the time, the average performance will still be better, for the same reasoning as in the first paragraph.

We haven't seen any profiling data that suggests particle culling has been a major bottleneck in the past, and as written above, it seems more likely that it would help rather than hinder performance in most real-world scenarios.

Therefore, I don't see the merit in adding an option. I'm happy to be corrected on any of my points above if there's something I didn't take into account. 🙂

@Lolothepro Lolothepro reopened this Apr 26, 2024
@Lolothepro Lolothepro closed this as not planned Won't fix, can't repro, duplicate, stale Apr 26, 2024
@sciwhiz12
Copy link
Member

The above opinion by @embeddedt is not the final decision of the team. This should be left up for some time to gain the opinions of others on the team, and of the community.

If there are no compelling arguments to add an option after such time, we can close this issue then.

@sciwhiz12 sciwhiz12 reopened this Apr 26, 2024
@TelepathicGrunt
Copy link
Contributor

Closing as enough time as has passed and no one argued for this config option

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.20 Targeted at Minecraft 1.20 enhancement New (or improvement to existing) feature or request rendering Related to rendering
Projects
None yet
Development

No branches or pull requests

6 participants