Jump to content
The Dark Mod Forums

Recommended Posts

The crash on shutdown is gone now as well, seems that it was related to that. We're much closer to a stable version now, I will do some tests with Bonehoard now. :)

Link to post
Share on other sites
  • Replies 132
  • Created
  • Last Reply

Top Posters In This Topic

edit: Actually, std::size_t is an unsigned type as well, but it seems that it has a different sizeof() value on 64-bit systems. Makes much more sense now.

 

Ah yes, that makes sense. You can only assume that std::size_t == unsigned int on a 32-bit system.

Link to post
Share on other sites

AFAIK int is always defined as the natural machine word. So on a 64 bit system it should also be 64 bits, while on a 32bit system it has 32 bits and on a 16 bit system it would be 16 bits. You also can not rely on longs because AFAIK long is only defined to be bigger or equal then int, while word is defined to be smaller then int. I think some compilers have int32, int64 etc for this reason but I think it's not standardized.

Gerhard

Link to post
Share on other sites

I got reproducible crash in DDSlib during the load of city_area.map. Could you check on occasion if it crashes on your end as well? If it does not, it might as well be another 64bit vs. 32bit problem.

 

I'll do a full recompile and see if it still crashes.

Link to post
Share on other sites

Exactly. I was just about to post, because I found several compiler warnings in ddslib.c where a pointer is cast to an (unsigned int) and back. What can I do here? I guess I need an IFDEF?

Link to post
Share on other sites

Hm, I changed the offending cast from (unsigned int) to (uintptr_t) but it still crashes. The ddslib.c is full of casts, so this might be a larger task. Sucks.

Link to post
Share on other sites

Is that where it checks the FOURCC identifying the DDS type? The cast to unsigned int is just a piece of C hackery that is obviously non-portable, it should probably be done the obvious way e.g.

 

if (val[0] = 'R' && val[1] = 'X' && val[2] = 'G' && val[3] = 'B') { // blah }

Link to post
Share on other sites

I tried that, but it still crashes (somewhere else, I think it's in the middle of a loop). It's likely that there is a pointer to something incremented with the ++ operator, causing an overflow.

Link to post
Share on other sites

I think this has been discussed before, but how about including the DevIL image library. I think it is 64 bit compliant.

 

This would of course only make sense if the rest of the codebase is fine so far, which it doesn't appear to be. I just got another crash while deselecting a face, and I haven't even looked into that. I wonder if it's all worth the hassle...

Link to post
Share on other sites

Ah, I think I found something. There is a void* pointer in the ddsBuffer_t structure, which is supposedly shifting the structure members.

Link to post
Share on other sites

Thanks for that, New Horizon. :) I feel a bit guilty of causing so much trouble with the scenegraph, hopefully it turns out to be worth it.

 

And I found something interesting as well regarding the crash when deselecting a brush face: This is the rough callstack (I've got a testmap with a light and some brushes):

Select_Deselect >> SelectAllComponentsWalker >> LightInstance::setSelectedComponents >> VertexInstance::setSelected >> FaceInstance::select_vertex

What we have here is a problem with namespacing. There exists a VertexInstance class in both the entity module and the radiant core, which was not a problem till now. Of course it's a logical consequence after adding the compiler switches, but I didn't think of that. The fix is probably easy and hopefully there won't be too much other occurrences of objects sharing the same name across modules. Once again, gdb is a blessing here, I wouldn't have found that using std::cout debug outputs, or at least not that fast. :)

Link to post
Share on other sites
Thanks for that, New Horizon. :) I feel a bit guilty of causing so much trouble with the scenegraph, hopefully it turns out to be worth it.

 

I wouldn't worry about that. :) I know that such refactoring can become a hard task, but it's usually worth the effort if done properly. I know that I wouldn't like to do such an ardous task and I think it's great that you really do all this. :)

Gerhard

Link to post
Share on other sites
I think it's great that you really do all this. :)

 

So do I. The amount of work going into DR is incredible, and it's going to be for the best in the long run. The bad things in DR are being stripped and burned. In the end, we're going to have a far better editor to work with. :)

Link to post
Share on other sites

DR is already much better in many regards. :) And if the remaining major issues will be resolved then it wil be real fun to work with an editor that actually is usefull. :)

 

IMO the major issue for me is, that it can't show proper rendering, but I don't expect this anytime soon. I think this is the one feature that requires me to load up DoomEdit, in order to see the lights and how they look.

Gerhard

Link to post
Share on other sites

Ok, it seems the most obvious problems are resolved now, fingers crossed. I could test it on both a 32 bit and 64 bit linux, loading bonehoard, loading city_area, do some fiddling with the map objects, open the various inspectors, load models, load shaders, etc. Everything seems to be fine for the moment.

 

I assume the Windows build is ok as well, but I will have to check that of course. I'll do the next few development steps in Windows again, as the Eclipse on this system is lacking the code search (the C++ indexer is constantly crashing) and I find grep a bit inconvenient.

Link to post
Share on other sites

I've opened a new branch scenegraph_refactor_2, for further experiments.

 

So far I've created an abstract base class for the scene::Node, namely scene::INode. The NodeSmartReference has been replaced by boost::shared_ptr, all the dynamic_cast stuff has been migrated to boost::dynamic_pointer_cast, which is equivalent. scene::Path is now a Stack<:inodeptr> instead of the NodeSmartReference stuff, which seems to work just fine.

 

I can load a map, but it's not very stable yet, it crashes as soon as I load a second map, but I'm a bit surprised that it works at all.

Link to post
Share on other sites

Yeah, that was my intention too. :) This shouldn't be too hard, given that it doesn't differ too much from the std::stack interface. Otherwise I'd have to traverse the entire codebase (again).

 

I could fix the map loading meanwhile, but there remain quite a few unstable things. It's also likely that I will have to rewrite the graph_tree_model (used for the EntityList), because it's not compatible with shared_ptrs.

Link to post
Share on other sites

I checked for memory leaks and it looks like the boost::shared_ptr approach is working. All the nodes constructed during a bonehoard map load get destructed on "New Map", which is good.

 

I think I finally got the grip of how the scenegraph is working currently and I already have a few improvement ideas, which might speed up things a bit and make everything clearer to work with.

 

Additionally, I added an auto-increment ID to the scene::Node classes, to facilitate detection memory leaks and such - the interface is not yet complete, but the ID works fine. :)

Link to post
Share on other sites

I had a look at Radiant's Stack implementation and it provides indeed some non-standard methods like a Constructor taking a starting element and a parent() method (which accesses the element below top()).

 

Both things are really convenient in my opinion, that's why I came up with this: I redefined the Stack class and let it derive from std::vector, extending its interface by the custom Constructor and the parent() method in addition to the pop(), top() and push() methods required by a proper stack:

 

template<typename Type>
class Stack :
public std::vector<Type>
{
public:
// Default constructor
Stack() 
{}

// Constructor taking an initial element. 
// The stack is starting with one top element. 
Stack(const Type& initialElement) {
	push(initialElement);
}

// Accessor method to retrieve the last element inserted.
Type& top() {
	assert(std::vector<Type>::size() > 0);
	return *(std::vector<Type>::end()-1);
}

// Accessor method to retrieve the last element inserted.
const Type& top() const {
	assert(std::vector<Type>::size() > 0);
	return *(std::vector<Type>::end()-1);
}

// Accessor method to retrieve the element below the last inserted.
Type& parent() {
	assert(std::vector<Type>::size() > 1);
	return *(std::vector<Type>::end()-2);
}

// Accessor method to retrieve the element below the last inserted.
const Type& parent() const {
	assert(std::vector<Type>::size() > 1);
	return *(std::vector<Type>::end()-2);
}

// Add an element to the stack, this becomes the top() element
void push(const Type& newElement) {
	push_back(newElement);
}

// Remove the topmost element, the size of the stack is reduced by 1.
void pop() {
	assert(std::vector<Type>::size() > 0);
	erase(std::vector<Type>::end()-1);
}
};

As soon as I will have checked whether this is used in any other places than scene::Path, I can of course rename it completely and define it as scene::Path itself, which is cleaner.

Link to post
Share on other sites

Ok, the Stack class is used somewhere else, so I renamed my implementation directly to scene::Path and left the old Stack class alone.

Link to post
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.


×
×
  • Create New...