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

Fix cloud fog being broken for high clouds (maybe) #11290

Merged
merged 1 commit into from May 29, 2021

Conversation

Wuzzy2
Copy link
Contributor

@Wuzzy2 Wuzzy2 commented May 25, 2021

This PR might fix #8340.

But this is just a quick and dirty bugfix attempt, I made only superficial testing, which worked. But I have no idea if this bugfix is really fixing cloud fog for good or if this is only a fake fix.

So please check for yourself, if it makes sense.

@sfan5
Copy link
Member

sfan5 commented May 29, 2021

m_box has the camera offset subtracted:

m_box = aabb3f(-BS * 1000000.0f, height_bs - BS * m_camera_offset.Y, -BS * 1000000.0f,
BS * 1000000.0f, height_bs + thickness_bs - BS * m_camera_offset.Y, BS * 1000000.0f);

Camera::update() is called with camera offset added:

minetest/src/client/game.cpp

Lines 3752 to 3758 in f0725ce

v3f camera_node_position = camera->getCameraNode()->getPosition();
v3s16 camera_offset = camera->getOffset();
camera_node_position.X = camera_node_position.X + camera_offset.X * BS;
camera_node_position.Y = camera_node_position.Y + camera_offset.Y * BS;
camera_node_position.Z = camera_node_position.Z + camera_offset.Z * BS;
clouds->update(camera_node_position,
sky->getCloudColor());

Consequently, height calcuations need to take the camera offset into account.

@sfan5 sfan5 added One approval ✅ ◻️ Trivial The change is a trivial bug fix, documentation or maintenance change, as per the Git Guidelines and removed Testing needed labels May 29, 2021
@sfan5 sfan5 merged commit ff48619 into minetest:master May 29, 2021
@Wuzzy2
Copy link
Contributor Author

Wuzzy2 commented May 29, 2021

Heh, that was exactly my thought process as well. But I just wanted someone to look at it, just in case I overlooked something. :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bugfix 🐛 PRs that fix a bug @ Client / Audiovisuals One approval ✅ ◻️ Trivial The change is a trivial bug fix, documentation or maintenance change, as per the Git Guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inside cloud fog is broken for very high clouds
3 participants