Flower's Dev Diary (Week Of March 18th)

[FlowerChild]

Oh fucking wow...

Ok, so for the longest time, one of the parts that has bugged me the most about the MC rendering code is this stupid test to see if each individual side of each individual block rendered in the entire fucking game was the index of the grass texture.

That drove my inner game programmer nuts as it was a shit-ton of useless tests being performed an insane number of times, probably at great performance cost. It's seen in lines like this:

Code: Select all

            if (cfgGrassFix && var27 == 3 && this.overrideBlockTexture < 0)

Where '3' is the index of the grass texture, and triggers special case rendering code to color the grass based on the biome. Now, this is easily, easily avoided by having custom rendering code for the grass blocks themselves so all these useless tests don't have to be performed on every other block.

So, I've been hoping that those would finally disappear when all the texturing code got a rewrite, which it just did.

Well, I just got to the block rendering code during my update, and found out instead, that all those tests have been replaced by this:

Code: Select all

            if (cfgGrassFix && var22.getIconName().equals("grass_side") && !this.hasOverrideBlockTexture())

Now, this will probably only mean anything to programmers, but fuck me...they just replaced that already costly test with a string compare on every single face, of every single fucking block in the game, in what is basically the inner fucking rendering loop of Minecraft.

Face palms just don't cover it. Remember how I was beginning to have doubts not just about the design, but programming ability at Mojang right now? Yeah...I don't have any doubts any longer.

[DaveYanakov]

That kind of sounds like you just found the root of the massive FPS loss

[FlowerChild]

That kind of sounds like you just found the root of the massive FPS loss

I may have. I won't fix that for the next release, as I want people to have a basis for comparison, plus I already have too much on my plate, but I will be sure to in a following one.

No guarantees without benchmarking of course, but yeah, it wouldn't have surprised me if that was previously a big performance drain for no good reason, and an even bigger one now. It always struck me as a piece of code that Notch probably implemented in a hurry and never got back to fixing, or which he forgot about entirely, which is why I was hopeful that it with these texturing changes, as I knew someone would have to revisit that code and would probably spot the problem there.

Well...they did definitely revisit it...stared it right in its maw...and made it way way worse.

That's just a clear indication of someone who isn't a game programmer and has no sense of the cost of their actions. It looks like a simple call, so unless you know better, you don't realize the kind of overhead you're invoking by doing something like that.

Whether something is a grass block or not is known. Constantly testing for it for every face rendered on every single block when that has already been established at a much higher level is just nuts. It's like, I have custom textures for the dirt slabs which do precisely the same thing, and incur none of that overhead, so I know with certainty that it's in no way necessary.

[FlowerChild]

...and there we go. I think I also just found the performance drop that people experienced a few releases ago with BTW due to a similar error I think I made with snow on the dirt slabs. I added a much simpler conditional to the same area of the rendering code, and didn't even think of it when it was mentioned, until I just reviewed that code while porting it over.

I took a short-cut there to minimize the hassle I'd experience during version updates by testing something a 2nd time that had already been established so that I could keep my own code isolated from vanilla's.

If that had that kind of impact, then what Mojang has been doing in that code is way way worse.

Yup, I'm confident I can get a significant performance boost out of MC within the next few releases. Looking over it more closely now, there's actually a shit ton of optimization work I can do here with relatively little effort. I never really wanted to touch that bit much as it was already modified by the old Forge code that I'm getting rid of as part of this update, but there's really no reason for me not to go to town on it now.

[TheYaMeZ]

they just replaced that already costly test with a string compare on every single face, of every single fucking block in the game, in what is basically the inner fucking rendering loop of Minecraft..

Wow that is truly an amazing example of extreme head-desk. I've never looked into the source code of minecraft but if this is the kinda thing you can find in there, they really really need to set aside some time for optimization. I'm sure there are reasonable fixes for common bad performance scenarios like going into a cave while its raining, which still brings my frame rate crashing down for a few seconds even after a year.

[savagelung]

Although I have no doubt this particular line is an issue, Jeb identified the FPS drop in 1.5 as being caused by a "bugfix" that added transparency to clouds. The commit for that particular fix was reverted, which is why many people saw FPS improvements in 1.5.1 (myself included). I wonder how they didn't catch this one in their debugging...

Great catch on that bug though FC! Color impressed, it took me ages just to figure out how they implemented sideways logs -- I never would have caught this check. I'm tempted now to go through the Optifine code to see if its implementation of Better Grass addresses this bug.

[FlowerChild]

Actually, here, you can see my own error:

Code: Select all

        if (this.renderMinY < 0.0D || this.renderMaxY > 1.0D)
        {
            var16 = (double)(((float)var11 + 0.0F) / 256.0F);
            var18 = (double)(((float)var11 + 15.99F) / 256.0F);
        }

        // FCMOD: Code added
        if ( this.renderMinY < 0.0D )
        {
            var16 = ((double)(var11 + 16) - ( renderMaxY + 1D ) * 16.0D) / 256.0D;
            var18 = ((double)(var11 + 16) - ( renderMinY + 1D ) * 16.0D - 0.01D) / 256.0D;
        }
        // END FCMOD

The stuff marked by FCMOD are my own changes. As you can see the test for < 0 is already performed above that, but it's much simpler during version updates for me to just copy paste code additions into the new version rather than have to fiddle with changing existing code, so I opted to do the above < 0 test a second time, which I'm fairly convinced generated a 25% frame rate drop for some people.

The code within the conditional definitely isn't the problem as it's only triggered when snow is resting on dirt slabs, which is certainly the exception case. It's the conditional itself that's causing the slowdown.

As you can also see, the above conditional, which is performed the same number of times as the vanilla code I quoted before is MUCH simpler and less performance intensive than that vanilla test. If the above therefore has a 25% impact on some systems, you can be fairly certain that me getting rid of those vanilla tests will have a much larger impact.

[FlowerChild]

Great catch on that bug though FC! Color impressed, it took me ages just to figure out how they implemented sideways logs -- I never would have caught this check. I'm tempted now to go through the Optifine code to see if its implementation of Better Grass addresses this bug.

I'm going to be anal here and say: it's not a bug.

Really it's not. The code is working fine and as intended. Optimizing something is not the same thing as bug fixing.

[FlowerChild]

Actually, I think what I'm going to do is put together a little patch for the last release for 1.4.7 to verify the above. I'm fairly convinced that this is the source of the performance drop some people were experiencing but want to be absolutely certain as it means there's a great deal of potential for optimization here, and with all the other changes it's unlikely it will be noticeable to folks if I do this just as part of the 1.5 update.

Please stand by :)

[FlowerChild]

Ok, done. Will post the patch shortly in the relevant thread.

For anyone that has been following this though, here's the modified version of the code I posted above:

Code: Select all

        // FCMOD: Code change
        /*
        if (this.renderMinY < 0.0D || this.renderMaxY > 1.0D)
        */
        if ( this.renderMinY < 0.0D )
        {
            var16 = ((double)(var11 + 16) - ( renderMaxY + 1D ) * 16.0D) / 256.0D;
            var18 = ((double)(var11 + 16) - ( renderMinY + 1D ) * 16.0D - 0.01D) / 256.0D;
        }
        else if ( this.renderMaxY > 1.0D )
        // END FCMOD
        {
            var16 = (double)(((float)var11 + 0.0F) / 256.0F);
            var18 = (double)(((float)var11 + 15.99F) / 256.0F);
        }

Deceptively simple difference, but let's see if that does the job I suspect it will :)

[BobSlingblade679]

Just curious, does that checking of every side occur only on chunk load or every frame?

[FlowerChild]

Patch posted here in the relevant thread:

viewtopic.php?f=9&t=6954&p=118166#p118166

Let me know what happens :)

Note that this may not apply to everyone, so if you didn't experience the performance drop before, you may not experience the increase now.

[FlowerChild]

Just curious, does that checking of every side occur only on chunk load or every frame?

I've never delved into the details, but my understanding is that it happens every time a block is added to the display list. So yes, that would be on chunk load or every time a block is updated and thus requires a display update as well.

The important bit is that it happens a shit-load ;)

[BobSlingblade679]

Just curious, does that checking of every side occur only on chunk load or every frame?

I've never delved into the details, but my understanding is that it happens every time a block is added to the display list. So yes, that would be on chunk load or every time a block is updated and thus requires a display update as well.

The important bit is that it happens a shit-load ;)

Huh. I think its odd they don't test if the block is dirt/grass first. That alone would save a lot of tests.

[FlowerChild]

Huh. I think its odd they don't test if the block is dirt/grass first. That alone would save a lot of tests.

It's a matter of thinking a certain way man. Game programmers typically do, other varieties of programmers typically do not as they are both not taught to, and don't need to in their day to day work.

Game programming is its own skill set. If you're not habituated to it, you won't see stuff like this. I've programmed in straight assembler in commercial games in the past (like I always say...I'm an old fart), so when I see a string compare in an inner loop, I immediately have a good sense of the kind of performance overhead involved. If all you've ever seen is the exceedingly high level level Java code, you'll have no idea of what's actually going on at the level of the CPU when you make a call like that.

I don't doubt Notch is a game programmer, which is why I always thought the test versus the texture index was just a quick implementation or oversight on his part, but again, if someone goes back and does a second pass at that code and only makes the problem worse, then my immediate assumption is that they are definitely not a game programmer.

[MoRmEnGiL]

My general feeling is that the mc code is kind of a mess, because one man started it small, and then it blew out of proportion and is fiddled with by completely different men.

I wonder if it would benefit from a complete rewrite from the ground up, with all the knowledge they theoretically *should* have being applied.

Nope, they'd fuck it up. But theoretically..

[FlowerChild]

I wonder if it would benefit from a complete rewrite from the ground up, with all the knowledge they theoretically *should* have being applied.

Gods no. Optimization in no way requires a rewrite. It's only a small fraction of the code that consumes 90% of performance. The trick is identifying that portion and knowing what to do about it.

"Rewrite" is just the call to arms of the poorly informed. What they're doing now is effectively a rewrite in many ways and they're just making things worse in the process, because as far as I can tell, their concept of "better" is the text-book variety that has absolutely nothing to do with performance, and everything to do with theoretically ideal code structuring techniques (which often times have the exact opposite effect).

[MoRmEnGiL]

You yourself though have stated that if you were to do things from the start you would change a few things, like metadata. I seriously doubt notch had any idea how big mc would become, and brilliant his coding might have been, but since then a lot has happened.

Patchwork after patchwork, I do get the feeling the whole thing is a bit of a mess. That bit you posted above seems like proof of this.

The question is, at which point is it better to start from scratch rather than attempt to patch things up. I'm sure if it was just up to you you'd do a great job of tidying it up, but dunno, I always got a bad vibe when too many fingers are one pie.

So you think the mc code in general is pretty good, just needs some work here and there? Because this is good news to me.

[devak]

So you think the mc code in general is pretty good, just needs some work here and there? Because this is good news to me.

The thing is, code is usually pretty low-impact. You only need one "bad" line of code to greatly reduce performance. the vast majority of performance drains probably come from a handful of lines.

[Itamarcu]

You are awesome. Now I love my laptop again.

[Graphite]

The question is, at which point is it better to start from scratch rather than attempt to patch things up.

Think you might find Joel on software - Things You Should Never Do, Part I an interesting read.

[FlowerChild]

You yourself though have stated that if you were to do things from the start you would change a few things, like metadata.

That's *if* I were to start over, I wouldn't start over because of it.

I'm moving onto RTH for a number of reasons. Being able to do stuff like the above is a pleasant side-effect of that, but no way I'd choose to rewrite a whole code base if I didn't have to.

If staying with the MC code-base were a viable option for me, I'd most certainly do so, and you saw how long I deliberated over the decision to move on because of the hassle it will inevitably entail. If I stayed, I'd continue to adjust my designs to the technical limitations present. You can be certain I'll be adjusting my designs to whatever limitations are within RTH as well, but since I'm writing my own code base for a variety of other reasons, I may as well push those limitations a bit further out in areas that are important to me while I'm at it. I'm also putting a heck of a lot of thought into the trade-offs involved and how much I can get away with.

For example, while I may be using a format that allows for increased "metadata", I will likely be limiting myself to 256 of what are referred to as blockIDs in MC in trade, while MC is pushing that limit outwards. Because to me, fewer blocks with more complex interactions between them is more important than having a larger number of blocks and expandability for potential modding. I'm not just going nuts like most other MC-like games I'm seeing out there and trying to pretend like I can push in every direction at once and expect the game to still run at a decent frame rate. Doing so is pure game-dev noobery.

Limitations always exist. That doesn't represent a problem with the code, but is rather symptomatic of a technical design that acknowledges the realities of limited CPU power.

[Sarudak]

What??? Limitations on return to home? No way! :)

Honestly though 256 is a whole heck of a lot if you think about it. Especially if you consider all of vanilla minecraft only takes up about 150 and they're making very poor use of metadata.

[warmist]

<...> I'm not just going nuts like most other MC-like games I'm seeing out there and trying to pretend like I can push in every direction at once and expect the game to still run at a decent frame rate. Doing so is pure game-dev noobery.

Limitations always exist. That doesn't represent a problem with the code, but is rather symptomatic of a technical design that acknowledges the realities of limited CPU power.

This is so hard to overcome. The need to noob it up by e.g. including something so cool you just learned be it a new strange structure or a dna-inspired mechanic. I always struggle between what is a cool mechanic to add, and what sounds cool to a programmer. Also how do you begin with the game (or a mod)? Have you planed something out with BTW (in the beginning afaik RTH is planned out quite detaily) or just it accumulated to what we see (i have been playing for quite some time now, but i would never have guessed that there was a master plan behind it).

[FlowerChild]

What??? Limitations on return to home? No way! :)

Honestly though 256 is a whole heck of a lot if you think about it. Especially if you consider all of vanilla minecraft only takes up about 150 and they're making very poor use of metadata.

Yup, indeed. There's a ton of blocks consuming individual blockIDs for stuff like different varieties of stone (smooth, cobble, etc) that could easily be crammed into one with very little trouble. Also, the code base isn't really setup for stuff like that, with tools for example always treating blocks with the same blockID the same way.

Given increased metadata as well, the possibilities for such combinations increase greatly along with it. I don't want to push that too far, as if you do the code becomes very unwieldy to work with, but for strictly aesthetic blocks that are functionally equivalent, it's definitely the way to go.

This is so hard to overcome. The need to noob it up by e.g. including something so cool you just learned be it a new strange structure or a dna-inspired mechanic.

Yup, it is hard to overcome, and largely comes down to experience. I've been saying on IRC for awhile now that I expect the initial reaction to RTH to be rather "blah" because people are going to be expecting flashy graphics or whatever out of a new game, and that's not what I'm about. What I *am* about is providing increased gameplay possibilities, and that's what I intend for RTH to provide in spades. It's going to take awhile for people to pick up on it though, as it's certainly not something that will come across in initial screenshots or videos.

I want RTH to be all about substance, not flash, as I see the next generation of MC-like games all going towards the latter, when one of the things that made MC great in the first place was that it focused on the former. So many times I see videos for these games and pickup on the fact that they're actually *more limited* in terms of gameplay than MC is, because the developers decided they wanted flash and then were smacked in the face by the harsh realities of the trade-offs that requires.