Allow server side occlusion culling. #5320

lhofhansl · 2017-02-28T07:13:16Z

Moved occlusion culling code from ClientMap to Map.
Changed transparency condition from

		const ContentFeatures &f = nodemgr->get(n);
		if(f.solidness == 0)
			is_transparent = (f.visual_solidness != 2);
		else
			is_transparent = (f.solidness != 2);

to
f.drawtype == NDT_NORMAL
This is needed so the same code can be used on client and server without virtual dispatching in hot code path.
(see ContentFeature::updateTextures for why this is correct)

Optionally enable occlusion code on the server, using the player eye-position as reference point.

In my tests this saved between 50 and 80% of the blocks sent to the server when block_send_optimize_distance is set to the maximum with no visual impact.

lhofhansl · 2017-02-28T15:25:18Z

@Fixer-007 what about it? :)
This will generally send less data, but it can't do anything about a laggy network.

Fixer-007 · 2017-02-28T15:52:25Z

If network lags while I'm walking on server, will culling then also lag?

SmallJoker · 2017-02-28T18:02:04Z

src/clientiface.cpp

@@ -298,6 +301,12 @@ void RemoteClient::GetNextBlocks (
 if(block->getDayNightDiff() == false)
 continue;
 }
+
+ if (occ_cull && !block_is_invalid &&
+ env->getMap().isBlockOccluded(block, cam_pos_nodes))


if (occ_cull && !block_is_invalid && env->getMap().isBlockOccluded(block, cam_pos_nodes)) {

EDIT: http:https://dev.minetest.net/Code_style_guidelines#Spaces

Oops. That one slipped through.

SmallJoker · 2017-02-28T18:04:38Z

src/clientiface.cpp

@@ -197,6 +197,9 @@ void RemoteClient::GetNextBlocks (
 s32 nearest_sent_d = -1;
 //bool queue_is_full = false;

+ v3s16 cam_pos_nodes = floatToInt(camera_pos, BS);


I knew that would come up. :)
We're kinda schizophrenic about this. I'll change it.

SmallJoker · 2017-02-28T18:13:25Z

@lhofhansl Is this considered as an alternative method to prevent blocks from being sent to the clients like pull #5309 would do?

lhofhansl · 2017-02-28T21:31:43Z

@Fixer-007 The server side culling is independent on that.

lhofhansl · 2017-02-28T21:32:43Z

@SmallJoker This is definitely related to #5309 and #4811 .
Right now I'd suggest to keep all of them. Upon more testing we can retire one or more of the other.

lhofhansl · 2017-02-28T22:23:01Z

But in fact this can replace all the other server side optimizations.

Fixer-007 · 2017-02-28T23:08:02Z

@lhofhansl Come to VanessaE Dreambuilder Survival or Creative (or other VanessaE's servers), they have this patch.

lhofhansl · 2017-02-28T23:10:20Z

@Fixer-007 Are you saying something broke or renders incorrectly or is slowing down? I tested with my own server and mostly focused in the number of blocks the client reports as "seen".

lhofhansl · 2017-03-01T02:11:30Z

@Fixer-007 Cool. I might do that. Never actually player on a public server :)

What this does it just has the server filter the blocks to send by what the play would actually see. There are other mechanisms that take the player's position into account, so I would not expect this to cause new visual lag.

I'll do some tests with some scenes and compare the day-night spread optimization (which leads to rendering issues) to the occlusion culling.

lhofhansl · 2017-03-01T02:18:15Z

Update style and const declaration. No functional changes.

lhofhansl · 2017-03-01T02:22:45Z

To make review easier...

just moved the occlusion code from ClientMap to Map
isOccludes is now a member, removed the Map and iNodeDef parameter accordingly
Move the occlusion logic in ClientMap.updateDrawlist into its own method
Change the transparent logic as indicated in the initial comment (it really does not change when a node is considered transparent)
Called the isBlockOccluded also ClientIface.GetNextBlocks.

The change looks bigger than it is, because I had to move some code around.

lhofhansl · 2017-03-01T02:48:03Z

Looks like max_block_send_distance is set only to 3 on that server (I can see the edge of the world with a v_range > 50). Is that true?

lhofhansl · 2017-03-01T05:23:06Z

Some numbers:

Water scene:

-	Default	#5309 (no underground opt)	#4811 (opt distance set to max)	#5320 (this PR with opt dist = max)
Blocks Rendered	331	508	506	506
Blocks in view	426	1771	2954	1098

Land scene:

-	Default	#5309 (no underground opt)	#4811 (opt distance set to max)	#5320 (this PR with opt dist = max)
Blocks Rendered	250	345	341	341
Blocks in view	448	1506	2043	1045

v_range was 200

lhofhansl · 2017-03-01T05:26:25Z

These two scenes should be somewhat the worst case.

So this is clearly superior to #5309 , and this will always render correctly (if the culling is incorrect it is incorrect in exactly the same way the client culling is incorrect).

I think we can remove the WIP status from this.

lhofhansl · 2017-03-01T05:35:41Z

Only concern is potential CPU spent on the server to do occlusion culling for each player. The upside is that at any point only blocks are sent to the client that the player can actually see.

paramat · 2017-03-01T06:19:05Z

+# If enabled the server will perform map block occlusion culling based on
+# on the eye position of the player.

Is occlusion culling also disabled on the client? I guess it is because it is less effective so would not be of any use.

+# The client will not longer receive most invisible
+# so that the utility of noclip mode is reduced.

What does this mean? Can we still fly underground with noclip? (essential for dev work).

What is it about server occlusion culling that makes it more effective than on the client?

Fixer-007 · 2017-03-01T19:19:30Z

So amount of rendered stuff will increase by 35-50% ? Thats a lot of fps drain :(

lhofhansl · 2017-03-02T01:39:24Z

@Fixer-007 We'll keep the day-night optimization - which has the rendering errors. Server side occlusion help even on top of that. But when you want correct rendering, than occlusion culling can give you that and save blocks (in tight caves that can be saving 90% or more).

lhofhansl · 2017-03-02T01:42:31Z

+# If enabled the server will perform map block occlusion culling based on
+# on the eye position of the player.

Is occlusion culling also disabled on the client? I guess it is because it is less effective so would not be >of any use.

No it's not. Server culling just makes sure that the server does not send blocks the player cannot see. Since the player moves around, there will be cached blocks on the client that still should not be rendered, so client side culling is still useful.

+# The client will not longer receive most invisible
+# so that the utility of noclip mode is reduced.

What does this mean? Can we still fly underground with noclip? (essential for dev work).

Flying around with noclip still works, but the client will naturally have fewer blocks to see, that's all.

What is it about server occlusion culling that makes it more effective than on the client?

It doesn't. It's the same logic. It just happens before the blocks are sent to the client, rather than after.

paramat · 2017-03-02T20:17:18Z

I see, thanks, seems good.

lhofhansl · 2017-03-03T02:32:30Z

Just to make 100% sure, you set server_side_occlusion_culling to true in your minetest.conf file, right? :)

lhofhansl · 2017-03-03T15:33:07Z

Heh. Glad I asked. On the server only.
I should mage like make it the default anyway. In no testing have I seen the slightest problem.

lhofhansl · 2017-03-03T18:22:13Z

Let me turn this default on.

lhofhansl · 2017-03-03T20:41:46Z

Turned on by default now.

lhofhansl · 2017-03-04T05:39:00Z

As discussed in #5309 perhaps now it's time to increase block_send_optimize_distance to 6

lhofhansl · 2017-03-04T06:12:36Z

Some typical scenes	default	distance = 6	distance = 6 + this PR
land1	396	578	412
land2	549	700	635
water	426	644	560
cave	444	704	45

I think it's safe to turn this on and set block_send_optimize_distance to 6

lhofhansl · 2017-03-04T22:37:00Z

Anything else I need to do with this?

This saves a bunch of block sending from server to client. Since this uses the same algorithm on the server that we have been using on the client this will never miss blocks that client occlusion culling would not filter out as well.

paramat · 2017-03-05T00:16:47Z

Sorry for quiet dev, many devs seem to be absent or busy at the moment, and the few left overwhelmed, i think this has a lot of potential, we are very interested and i'll try to look as soon as i can. I support the concept.
Are you back from travelling yet? Hopefully in a while we can add you as a core dev, if you are ok with that, we certainly need the help.
Does this help FPS due to fewer blocks on the client?

lhofhansl · 2017-03-05T00:18:51Z

(Yeah back from travel - work still keeping me busy, though. Happy to help wherever I can, with code, reviewing, keeping the code sane, etc)

lhofhansl · 2017-03-05T01:56:04Z

Oh and FPS should be unaffected. The complexity of the rendered scene is exactly the same.
Perhaps slightly better jitter because fewer unnecessary blocks are loaded into the client.

Zeno- · 2017-03-05T03:50:20Z

👍

rubenwardy · 2017-03-05T12:28:24Z

It's worth benchmarking the server, and seeing [if/how much] it reduces load

lhofhansl · 2017-03-05T21:41:48Z

Ran it through the Linux perf counters. The difference I'm seeing are within the noise. Without this PR I see perhaps a little more time spent in malloc (1% or so). With the PR I see (also 1% or so) in Map::getNodeNoEx and Map::getSectorNoGenerateNoExNoLock. All within the noise of a few runs.

Like the day-night light test optimization to see whether a block is near ground level, the blocks needs to be actually loaded in order to do occlusion culling (again just like for the day-night optimization), so I would not expect lighter load on the server; just lighter load on the network.

(Next step is making the occlusion more effective - i.e. re-visiting #4933 )

lhofhansl · 2017-03-05T22:26:36Z

Put some counters into the server to verify that the client's "blocks in range" does reflect the number of blocks sent from the server. It does :)

HybridDog · 2017-03-06T14:25:06Z

This also helps against cheating, doesn't it?
If some player e.g. changes the source code to get noclip and sets the drawtype of nyancat rainbows to glasslike in nodedef.cpp, he/she can not longer fly around underground and see all the nyancats stuck in the stone.
And local_map_saving does not longer store redundant blocks, which results in a smaller map.sqlite.

lhofhansl · 2017-03-07T02:00:23Z

@HybridDog yes it would incidentally help with that, but you couldn't rely on that. You can just fly around and look at the scene from different angles and over the time the client would cache the scene from all sides and you'd be able to do the very same hacks.

HybridDog · 2017-03-07T08:38:31Z

Can you automatically temporary disable server side occlusion culling for players which aren't in caves (test if they stay in sunlight) when the server has a high CPU usage?

paramat · 2017-03-07T09:42:41Z

^ That's not a good idea.

lhofhansl · 2017-03-08T21:04:15Z

It would only be a good idea with Occlusion Culling would be computationally expensive.
It's not and the blocks it'd load need to loaded anyway. Extra CPU is very light (within the measuring noise)

sorcerykid · 2017-03-09T16:38:44Z

How computationally expensive is it on a high-traffic server? At peak hours I average about 45-50 connected players. The potential gains on the network side are appealing, but I'm concerned this patch might prove to be a performance drain for the CPU. Thanks.

lhofhansl · 2017-03-09T17:14:06Z

By far the main drain is handling ABMs (also done per player). Occlusion culling is not measurable compared to the ABM cost, it's within the noise of the measurement.
Loading and serving all these blocks is also more expensive than the extra check for occlusion culling (which is done only on blocks that passed range and the day-night-light checks), and block loading also needs to be done per player.

In the end I have no way to experimentally verify that with dozens of players, though.

If it turns out to be a CPU drain for a specific server it can always be turned off via the config option - but I really doubt it will ever be a problem.

sofar · 2017-03-12T02:09:43Z

👍, this will be good for mp servers

sfan5 added the WIP The PR is still being worked on by its author and not ready yet. label Feb 28, 2017

SmallJoker reviewed Feb 28, 2017

View reviewed changes

paramat added @ Server / Client / Env. Feature ✨ PRs that add or enhance a feature Performance labels Feb 28, 2017

Allow server side occlusion culling.

30b34a1

lhofhansl mentioned this pull request Mar 1, 2017

Declarative block sending optimization setting. #5309

Closed

paramat removed the WIP The PR is still being worked on by its author and not ready yet. label Mar 2, 2017

Enable server side occlusion culling by default.

0f4207c

paramat added the One approval ✅ ◻️ label Mar 6, 2017

sofar added >= Two approvals ✅ ✅ and removed One approval ✅ ◻️ labels Mar 12, 2017

sofar merged commit ff80696 into minetest:master Mar 12, 2017

lhofhansl mentioned this pull request Apr 25, 2017

Fix block drawing glitch at block boundaries #4925

Closed

lhofhansl mentioned this pull request Oct 29, 2020

Client may unload blocks it shouldn't #10567

Closed

lhofhansl mentioned this pull request Jan 29, 2022

DISCUSS: Lua time budget for MapGen #12003

Closed

lhofhansl mentioned this pull request Mar 30, 2024

Feature request: Implement LOD #14502

Closed

Allow server side occlusion culling. #5320

Allow server side occlusion culling. #5320

Conversation

lhofhansl commented Feb 28, 2017 • edited Loading

lhofhansl commented Feb 28, 2017

Fixer-007 commented Feb 28, 2017 • edited Loading

SmallJoker Feb 28, 2017 • edited Loading

Choose a reason for hiding this comment

lhofhansl Feb 28, 2017

Choose a reason for hiding this comment

SmallJoker Feb 28, 2017

Choose a reason for hiding this comment

lhofhansl Feb 28, 2017

Choose a reason for hiding this comment

SmallJoker commented Feb 28, 2017 • edited Loading

lhofhansl commented Feb 28, 2017

lhofhansl commented Feb 28, 2017

lhofhansl commented Feb 28, 2017

Fixer-007 commented Feb 28, 2017

lhofhansl commented Feb 28, 2017

lhofhansl commented Mar 1, 2017

lhofhansl commented Mar 1, 2017

lhofhansl commented Mar 1, 2017

lhofhansl commented Mar 1, 2017

lhofhansl commented Mar 1, 2017 • edited Loading

lhofhansl commented Mar 1, 2017 • edited Loading

lhofhansl commented Mar 1, 2017

paramat commented Mar 1, 2017

Fixer-007 commented Mar 1, 2017

lhofhansl commented Mar 2, 2017

lhofhansl commented Mar 2, 2017 • edited Loading

paramat commented Mar 2, 2017

lhofhansl commented Mar 3, 2017

lhofhansl commented Mar 3, 2017

lhofhansl commented Mar 3, 2017

lhofhansl commented Mar 3, 2017

lhofhansl commented Mar 4, 2017

lhofhansl commented Mar 4, 2017 • edited Loading

lhofhansl commented Mar 4, 2017

paramat commented Mar 5, 2017 • edited Loading

lhofhansl commented Mar 5, 2017

lhofhansl commented Mar 5, 2017

Zeno- commented Mar 5, 2017

rubenwardy commented Mar 5, 2017 • edited Loading

lhofhansl commented Mar 5, 2017

lhofhansl commented Mar 5, 2017

HybridDog commented Mar 6, 2017

lhofhansl commented Mar 7, 2017

HybridDog commented Mar 7, 2017

paramat commented Mar 7, 2017

lhofhansl commented Mar 8, 2017

sorcerykid commented Mar 9, 2017

lhofhansl commented Mar 9, 2017

sofar commented Mar 12, 2017

lhofhansl commented Feb 28, 2017 •

edited

Loading

Fixer-007 commented Feb 28, 2017 •

edited

Loading

SmallJoker Feb 28, 2017 •

edited

Loading

SmallJoker commented Feb 28, 2017 •

edited

Loading

lhofhansl commented Mar 1, 2017 •

edited

Loading

lhofhansl commented Mar 1, 2017 •

edited

Loading

lhofhansl commented Mar 2, 2017 •

edited

Loading

lhofhansl commented Mar 4, 2017 •

edited

Loading

paramat commented Mar 5, 2017 •

edited

Loading

rubenwardy commented Mar 5, 2017 •

edited

Loading