Skip to content
This repository has been archived by the owner on Jun 19, 2021. It is now read-only.

One vertex buffer to rule them all #190

Conversation

LukasKalbertodt
Copy link
Member

@LukasKalbertodt LukasKalbertodt commented Aug 7, 2016

Instead of using instancing, this PR saves a whole chunk in one vertex and index buffer. This increases the performance of rendering the ground a lot. I lack exact benchmarks, but I hope a few of you could provide such benchmarks.

Without rendering shadow or plants, I get between 50 and 60 FPS on my integrated Intel HD Graphics 530. With plants enabled I still get around 50 FPS in biomes with few plants, but FPS drops a lot in forests.

There is still a lot of room for improvements, but I figured this is enough for now.

Oh and by the way: I fixed the indexing bug. Or at least one big indexing bug.

@@ -31,34 +31,37 @@ impl AxialPoint {
/// system using `world::{HEX_INNER_RADIUS, HEX_OUTER_RADIUS}`.
pub fn to_real(&self) -> Point2f {
Point2f {
x: ((2 * self.q + self.r) as DefaultFloat) * HEX_INNER_RADIUS,
x: ((2 * self.q - self.r) as DefaultFloat) * HEX_INNER_RADIUS,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wow, not even this was correct?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well... there are many "correct" versions. In the version of the axial coordinate system that I chose this was wrong, yes.

@jonas-schievink
Copy link
Contributor

The indexing fixes sadly do not fix #88

let s_diff = (rs - s).abs();

if q_diff > r_diff && q_diff > s_diff {
// rq = -rr - rs;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this stay or should this go?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I actually want this to stay, since it then represents the algorithm from redblobgames.com more closely. But I can add a comment. Will do it tomorrow when fixing the other stuff.

@jonas-schievink
Copy link
Contributor

The buffer building code is a tad too complex for my liking, but very well documented so looks good to me

@LukasKalbertodt
Copy link
Member Author

The buffer building code is a tad too complex for my liking, but very well documented so looks good to me.

I think so, too, but I wasn't able to come up with a less complex solution yesterday that does exactly the same thing. But feel free to simplify it 😛 I'd like to tweak it a lot still, but I have other things to do.

@LukasKalbertodt
Copy link
Member Author

Btw, I just checked: this PR introduces the flickering plants. That's pretty nice, since the PR doesn't change any of the plant rendering code (except for the two deleted empty lines). OpenGL, fuck yeah 😳

We should check on what GPUs the plant flickering occurs.

Note: the light calculation is still wrong somehow,
but I roughly verified my normals.
@jonas-schievink jonas-schievink merged commit 0f33e6e into OsnaCS:master Aug 8, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants