Beware the hook hiding under the clutter

A few years ago, while working on the release of 3dsMax (2014?), I was working on fixing warnings that came from a static analysis tool (PVS Studio). The analyzer had identified code in the status panel that could lead to possible memory corruption. As I started to look around in the code related to the status panel, I found some code for unrelated features and some dead code. In other words code for features that was not where it was supposed to be.

These features used to be on the status panel, but since then got moved around or removed. For instance the snaps used to be on the status panel (Remember in max 3?), but since then got moved to the toolbar on the top area of the app. What I found was that all the old features that ever used to be on the status panel were still there! When the UI got updated over the years, the programmer(s) just turned off the UI features, and simply left the old code. They didn’t even comment out the code, or indicate that a particular UI widget was not used… It was just abandoned. It was a hard to know what was still relevant, and what was not.

The snaps were at least still relevant, I put in a breakpoint, executed some snaps and hit that code! great! But wait! The snaps were now in a different functional area of the UI than the status panel. To be exact, for people not familiar with 3dmsax, the snaps are at the top of the screen on the main toolbar, and the status panel is the row of UI controls on the bottom of the screen. So why should toolbar code be hiding in the status panel code? What a Zig zag! Who cares? The computer doesn’t care where the code is, as long as the appropriate function gets compiled to an object file, and is available to the linker, that code can be anywhere. So why should humans care? Well because humans are much more expensive to operate than a computer. Humans read the code and have to understand the code first. So as long as that feature was hiding in the status panel, it was as good as taking a random book in the library, ripping off the catalogue ID, and placing it in a random location in the library.

Disorganized Library. In code typified by dead code and features implemented in the wrong files.

The tax associated with disorganization in code is immense. I’ve seen it first hand. I remember the first time I tried to find the code for the material editor. It took me an hour. I think I had to find some text that was somewhere in the material editor, and do a brute force text search of every file in the source code. Then look up some more things, do more brute force text searches until I found the code I was looking for. It was the equivalent of taking a book in the library, ripping off the cover with its catalogue ID, putting a different cover with an obsfucated name on it (generally with no vowels), and putting the book somewhere else in the library. Good luck finding it.

For an example of that, try finding the code for the editable mesh modifier in the public SDK of 3dsmax. You will have a hard time if you look for editablemesh.vcxproj, since it’s actually called pack1.vcxproj. Thankfully it is in a folder called /samples/mesh/editablemesh. (btw, the SDK sample code is only organized because of work I did way back in 2005).

Which leads us to another problem. A feature will be called one thing in the documentation, have a completely different name for the source code files, and have completely different class names in the code files. Imagine if you could have a code file name foo.cpp, the class names in the file are variants of the name baz, and the feature as exposed to the customer is named wizz. It’s all a confusing job to learn the lay of the land. Maps obviously don’t exist in this land, and only through experience do you learn where things are. Browsing the code is simply useless.

So I started to move the misplaced code. I used a code coverage tool called bullseye to aid me in my testing. At one point I had removed most of the code and hit around 85% functional coverage, and something slightly less for MCDC coverage. (MCDC coverage is the real indicator of code coverage btw). But there was this stubborn windows hook code that I could just not hit. Doing a brute text search for the hook, I could not find where the hook was used. It appeared to be dead code. So I removed it.

Fast forward a few months, and a bug report came that the cross hair feature was broken. That is where the cursor in the viewport can be replaced by cross hair lines. Ok, what caused it? My removal of the apparently dead hook. Ok, sorry about that.

Now if you know anything about hooks, you must know that they are only used as a last resort. And even then I and a few colleagues agree that using a hook is still generally the wrong approach. Hooks are an evil API that literally traps every single message that gets passed through your message pump (millions in a typical session). And for each message the system will execute your callback, and allow you to do who knows what with the message. There is no way that hooks can have a beneficial affect on performance. In fact with dozens (or more?) of messages getting sent every second, it can only have a negative affect on performance.

So the cross hair bug was reported, and the dev who was fixing it (not me) asked to revert the code changes by puting back the hook to fix the cross hair cursor. Not so fast I said. Why was the hook code in the status panel in the first place? Viewport code had it’s own place in the code. This hook code was far far removed from that! Finding this hook here was like finding the a gas line in the glove compartment of an automobile– in other word a complete and utter hack. This cross hair business has no place in status panel code. Well, something smelled fishy here. So I suggested to the other developer that they research the problem in greater depth and find another solution to the problem of the hook and crosshair. They did and in a week or so, they came back with a simple fix consisting of setting a variable (or something trivial like that) to turn on the cross hair. And best of all that fix was in it’s proper location: the viewport code. Excellent a great fix! Congratulations to the developer who found the real place to fix it.

Meanwhile once I found out what the hook was for, I researched it a little more and found out how really bad the hook was. That hook got installed and activated when max first started up, and stuck around until max shut down. It didn’t matter if you used the 3D viewport window cross hair or not, that hook was installed and running 100% of the time. Sucking CPU time for every message it trapped.






Leave a Reply

Fill in your details below or click an icon to log in: Logo

You are commenting using your account. Log Out /  Change )

Twitter picture

You are commenting using your Twitter account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )

Connecting to %s