Large Maps Do Not Display Properly [closed]

[greebo]

I extended the AABB of the light, otherwise the selection test would never return something positive. Only visible lights are tested against selection. It may well be that these changes are responsible for this, although I have no idea why.

 

I tend to do the following (after I resolved the other crash ): I will go back the SVN history in steps of, say 20 revisions and make some snapshot builds. Angua and me will go through this and narrow down the revision that is responsible for that crap.

[OrbWeaver]

I extended the AABB of the light, otherwise the selection test would never return something positive. Only visible lights are tested against selection. It may well be that these changes are responsible for this, although I have no idea why.

 

I just looked through askave.map with a text editor, and there are no light_center values that are larger than the light_radius, which suggests that this is not in fact the problem.

 

I tend to do the following (after I resolved the other crash ): I will go back the SVN history in steps of, say 20 revisions and make some snapshot builds. Angua and me will go through this and narrow down the revision that is responsible for that crap.

 

That is certainly a good idea (however time-consuming), I was thinking of doing that myself but decided to look at the renderer first.

[greebo]

We're currently setting up the development tools on angua's notebook, so we can sweep over this twice as fast. Latest SVN is at revision 740, DarkRadiant 0.7.1 was around revision 546. So we're going to check if rev 546 works, and if yes, we will split the intervals in half and track down the bogus version.

[greebo]

Ok, we've located the error: the bug appears first in revision 554, which is my commit concerning the draggable light_center. Revision 553 is working well. Which sucks, because commit 554 is large and there are quite some changes that cannot be disentangled easily. I'm afraid that I will have to re-do the whole thing until I know what change exactly is responsible for the mess.

 

I will look into it tomorrow, as I have to leave now.

[OrbWeaver]

Glad you've found it at last. You have also discovered the reason I always make small commits (well, that and the fact that if I get confused and have to revert the change, there is less work to redo).

 

I have started to split out the render sorting classes and have also added a stream insertion (operator<<) on AABB, which may help if you are trying to find out what is causing the objects not to appear.

[greebo]

I normally mean and try to keep the commits small and separate too, but sometimes this is hard to do, because a small change entails a bunch of other changes and I don't want to commit un-compilable code or at least a non-working DarkRadiant version.

 

Admittedly, this was not entirely the case in the light_center commit, but with some of the brush disentangling and such. I could have done better and I will keep this in mind in the future, the lesson is learned .

[OrbWeaver]

I normally mean and try to keep the commits small and separate too, but sometimes this is hard to do, because a small change entails a bunch of other changes and I don't want to commit un-compilable code or at least a non-working DarkRadiant version.

 

This can be challenge sometimes, I agree. I take the view that a commit shouldn't break the build, and shouldn't regress important functionality (like disable map-loading), so sometimes it is unavoidable that a large change gets committed in one go.

 

To really get around this problem requires the use of branches, which I have never quite got the hang of. One way to do this would be to create a new branch for each major piece of work - refactoring a large module, rewriting an entire feature etc. - and then merge the branch changes back into the trunk when completed, so that the trunk is always functional but the branch can be as WIP and broken as you like, since nobody will be using it.

[greebo]

Some news on this: as we suspected, it's the Light::localAABB() method that I had to adapt to make the light_center touchable when placed outside of the light volume.

 

I will further look into it what exactly can be done about this. At the moment there are three methods (I think) that return various types of light-AABBs, so I will have to figure out, which one of these is used by the renderer.

 

It's a single line that got changed, so hopefully I can work with the current SVN build as well. This would make merging the fix into the codebase easier.

[OrbWeaver]

I think it's Instance::worldAABB() which is used by the renderer. I have been trying to understand the visibility code, which seems to employ a certain amount of redundancy and duplicated checks, making it confusing to understand.

 

It starts with ForEachVisible, a scenegraph walker class which gets called for every Instance in the scene graph. First there is a visibility check:

 

VolumeIntersectionValue visible = (path.top().get().visible()) 
									   ? m_state.back() 
									   : c_volumeOutside;

 

where m_state.back() has already been set to c_volumePartial. I don't think this has anything at all to do with culling, it is a nasty hack which uses the c_volumeOutside as a flag meaning "not visible for any reason", based on a visible() flag set on the Node.

 

If the visibility flag is still c_volumePartial, the AABB is then tested:

 

if(visible == c_volumePartial) {
visible = m_volume.TestAABB(instance.worldAABB());
}

 

and depending on the result, the contained walker (RenderHighlighted) is then called on the instance. What is confusing is that RenderHighlighted has its OWN visibility and culling checks.

 

if (Cullable_testVisible(instance, m_volume, parentVisible) != c_volumeOutside)
  Renderable* renderable = Instance_getRenderable(instance);
  // blah

[greebo]

Yeah, I know that code, I have been there before. I don't think I will change anything here, as this would affect all other instances as well, and I certainly don't want to do change anything on that scale.

 

I'm still compiling the new revision, hopefully I can make the map visible again by reverting the localAABB() code. This will break the light_center dragging, but I will find some way around this.

[greebo]

Ok, I could make the map visible again by reverting the code, so the big problem is solved. As expected the light center is only draggable when within the small light "diamond", so I will try to fix this in a different way.

[OrbWeaver]

Ok, I could make the map visible again by reverting the code, so the big problem is solved. As expected the light center is only draggable when within the small light "diamond", so I will try to fix this in a different way.

 

That's bizarre. Do you have any idea why the light's AABB was causing the map to disappear? At the very most, I would suspect a dodgy AABB to cause the light to disappear, not the whole map.

 

It could be that there is a more serious bug in the culling code, which your light center just happened to expose.

[greebo]

As I'm returning a larger AABB than the standard one, I would assume that the lights don't disappear at all (even when they're not visible, but this is obviously not the case).

[greebo]

Fixed. There was a problem with an uninitialised member variable in Doom3LightRadius.h. I don't know if I should laugh or slap myself in the face.

 

It was damn hard to find, because usually the lightCenterChanged() method gets called immediately after instantiating a light, but in some cases it does not. I've no idea, why this is the case, but it seems to be random and it therefore appeared more often in large maps. Linux seems to be more stable with these things as well, maybe there is some compiler-specific initialisiation of member-variables?

 

Anyway, we can kick that one off the list...

[OrbWeaver]

Excellent, glad you've found it. My first thought was that there would be lack of initialisation in there somewhere, given the unpredictability of the error.

[PinkDot]

My contrgratulations! Reading this thread was like thrilling detective story with two detectives and one well hiden criminalist...

[greebo]

Well, then I will possibly be sued for aiding and abetting the crime

[Crispy]

The stunning twist: Holmes did it!

[PinkDot]

Well, then I will possibly be sued for aiding and abetting the crime

You cleared yourself, so you won't be sued for anything... But you could have been in worse situation if it was somebody else who had discovered the truth...

[OrbWeaver]

I have now added initialisation statements into the default constructors for Vector2,3 and 4 so that "uninitialised" vectors are set to 0, rather than left in an undefined state.

[greebo]

That's a good move.

 

What else is missing for a 0.8.0 release? I'm currently working on the CommandList Dialog, but that is not a very vital feature, because it's always editable through the input.xml file.

[OrbWeaver]

I think we are ready for 0.8.0 now that this bug is fixed. I will do the Windows build and perform some regression testing with existing maps.

[OrbWeaver]

I just did a full rebuild on Windows and now I cannot create a single brush, entity or light without DarkRadiant immediately crashing. I will see if I can replicate this on Linux so that the debugger can be used (although based on past experience this will be a stupid Windows-only bug that evades any attempt to use a debugger).

[greebo]

Reproducable on my system. Damn. I will also look into this.

[OrbWeaver]

Reproducable on Linux as well. Phew