Jump to content
Eternal Lands Official Forums
Entropy

Special effects

Recommended Posts

Today's patch:

 

* Summons are hooked in, but need testing

* Point particles are looking pretty nice now. There were two bugs: one was pure idiocy on my part (an old debugging statement that set alpha to 1.0 was still in there). The other was trickier: due to some vid card/GL implementations, point sizes are sometimes capped at relevantly small values. I had to detect this and have it switch over to billboards for large particles. Also, a few other steps were needed to make things look right, such as factoring in camera zoom.

* You no longer need SFX to build eye candy. It only respects -DEYE_CANDY. SFX is thus now a completely distinct element.

 

Also, while it's not in this patch, I'm well on my way to taking care of the leaf slowdown. The problem is twofold: one, why is it going to slow to begin with, and two, why can't the LOD reducer take care of the problem? As for #1, I've figured that out (and just need to decide what to do about it). get_wind_vec is using up a lot of CPU. The reason for the staggering drop in frame rates reported by ttlanhil is simple: it wasn't his video card's problem; it was his CPU. Most of the graphics he was looking at used very little CPU, so they were only limited by his (powerful) video card. However, it was CPU that was killing leaf performance, so as soon as that became a bottleneck, his framerate dropped way down. A team is only as weak as its weakest member, after all :icon13: So, I'll be optimizing the heck out of that function, and probably a few others, in addition to fixing #2 (improving the particle culling/LOD reducing algorithm)

 

The next patch will be all about that, plus the fact that the leaves come out as silver and yellow on some video cards (meaning too much specular/diffuse/ambient) but not on others.

Share this post


Link to post
Share on other sites
The reason for the staggering drop in frame rates reported by ttlanhil is simple: it wasn't his video card's problem; it was his CPU. Most of the graphics he was looking at used very little CPU, so they were only limited by his (powerful) video card. However, it was CPU that was killing leaf performance, so as soon as that became a bottleneck, his framerate dropped way down. A team is only as weak as its weakest member, after all :P So, I'll be optimizing the heck out of that function, and probably a few others, in addition to fixing #2 (improving the particle culling/LOD reducing algorithm)
considering that my CPU is a dual-code E6300 @ 1.86GHz, it's not that weak a link :icon13: ... perhaps the LOD could also be applied to extra cut-downs on get_wind_vec (beyond the optimizations you're already planning), if that's one of the functions that makes a noticable difference (I dunno how hard it'd be to have a light-weight version also available, but even well optimized that function might increase the minimum specs for EL... which brings another point, why not have eye-candy as one of the options in ctrl+o (next to SFX, probably)?)

Share this post


Link to post
Share on other sites

* Point particles are looking pretty nice now.

Yep, that's fixed my blocky fire problem. Now The fires look nice with or without use_point_particles set. Close up (zoomed) fire looks the same with use_point_particles either way. Zoomed out use_point_particles=1 looks more detailed. The camp fires and private fires look the same to me, if anything, I'd say the camp fire is a little small compared to the stack of wood. If it helps I'll post some screen shots later today. Thanks.

Share this post


Link to post
Share on other sites

Bluap: Yes, making F9 fires smaller than normal campfires is still on the todo list :blink:

 

Entropy: How about some time between 10:30 and 12:30 CST this evening?

 

Ttlanhil:

 

* "considering that my CPU is a dual-code E6300 @ 1.86GHz, it's not that weak a link". Dual core won't help you; eye candy isn't multithreaded. Note that, by their very nature, special effects (including particle systems) put loads on the CPU as well as the video card. A special effect that stands still and doesn't change isn't much of a special effect :icon13: The goal with special effects is to make them efficient enough that everyone can enjoy them.

* ". perhaps the LOD could also be applied to extra cut-downs on get_wind_vec." Certainly a possibility to have the algorithm simplified on low LOD, although I'd rather have an algorithm that's good enough that it doesn't need to have it's functionality cut down :D I haven't done much of any optimization on leaves before, so, for all I know, I might be able to make an order of magnitude improvement just through optimizations. We'll see tonight.

* "which brings another point, why not have eye-candy as one of the options in ctrl+o (next to SFX, probably)?" Sounds like a good option, but that'll have to go onto the TODO list and wait its turn, just like everything else :blush: If someone wants that option sooner, feel free to hook it in. You simply need to disable the calls to ec_idle and ec_draw (each is called but once in the EL code), and eye candy will stop putting loads on the CPU and video card. That won't clear it's state or free what memory it uses, but that shouldn't be important -- while it uses a relevant amount of memory, it's not a huge hog. And it's going to become even less of one shortly when I convert the math cache from doubles to floats. I think probably around 10 megs total for a system with textures loaded, equations cached, and loaded with max particles from a dozen effects.

 

Some optimizations that I am considering in case general tuning isn't good enough:

 

* Instead of each particle determining its own wind vector, having the effect subdivide up the region that it covers into a grid, with grid size determined by LOD (low LOD = very blocky grid). The effect would determine the wind vector for each grid square that's near enough to the camera to matter, once every frame. The particles would then just coarsely round off their spatial coordinates to get their grid coordinates and grab that precalculated local wind vector. If this makes particles within a particular grid square act too coarsely, I could have them linearly interpolate between their neighboring grid squares.

* Particles not updating their local wind vector every frame; rather, each frame, a particle has, say, a 1 in 8 chance of updating its local wind vector. Otherwise, it would use the last wind vector that it calculated.

* A combination of the above.

 

Also, one thing to keep in mind: we're all, assumedly, building debug builds. Turn -O3 on, and you'll get a huge performance boost. In fact, I should probably try that to see how performance changes *before* I start optimizing.

 

Entropy: I assume that release builds have optimizations turned on? If not, there's a lot I'll want to do to my code, because a lot of it was written with the assumption that the compiler would optimize out certain things that I know it does if you use the -O flags (fixed-length loop unrolling, precalculation of loop exit conditions, simplification of equations using temporaries, etc). The general rule is "don't optimize something that the compiler will take care of if it will hurt the clarity/maintainability of your code to do so". However, if the compiler won't take care of them, and CPU time is at a premium...

Edited by KarenRei

Share this post


Link to post
Share on other sites

New patch:

 

* Converted math cache from doubles to floats

* Fixed major CPU hog: idle was being called many times per frame! This

should fix leaf speed.

* Improved leaf coloration on some video cards. Probably made worse on

others, though. Needs more testing/work.

 

In progress:

 

* Ready to test a change I made that I think will make campfires bigger than F9 fires... perhaps. Haven't been able to test it yet, though. Someone gave me an FE, but I don't have any logs. I suppose I could always log in as my partner's char (who has the maps marked like crazy) to see where I can harvest some.

* Discovered point particle rendering bug: camera_x, y, and z seem to be where you stand! Not sure why... I'll figure that out tomorrow. With them being where you stand, when you walk away, point particles shrink much faster than perspective would suggest.

Edited by KarenRei

Share this post


Link to post
Share on other sites

New patch:

* Converted math cache from doubles to floats

* Fixed major CPU hog: idle was being called many times per frame! This

should fix leaf speed.

* Improved leaf coloration on some video cards. Probably made worse on

others, though. Needs more testing/work.

That's much better, FPS drop is not evident when the leaves blow now, and they're various shades of brown too. Well done. I did have one crash seconds after starting the client the first time. I was in a house at beam and crashed just after I stepped outside. Unfortunately, in my excitement to try the latest version I forgot to enable core file dumping, sorry. I have been unable to reproduce the crash so far but I'll try again later.

 

edit: caught one. Rebuild with -Ox replaced with -g but still no table information. Probably done something wrong in my rush, I'll try again later:

(gdb) run -sp 2001
Starting program: /home/paul/el/elc/el.x86.linux.bin -sp 2001
Failed to read a valid object file image from memory.
[Thread debugging using libthread_db enabled]
[New Thread -1225771296 (LWP 31943)]
[New Thread -1252840528 (LWP 31944)]
[New Thread -1261229136 (LWP 31945)]
[Thread -1261229136 (zombie) exited]
[New Thread -1269617744 (LWP 31946)]
WindEffect (0xe268a48) created.
WindEffect (0xe26a600) created.
Deactivating effect 0xe268a48(7301 > 250)
Deactivating effect 0xe26a600(8381 > 250)
WindEffect (0xe268a48) destroyed.
WindEffect (0xe26a600) destroyed.

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread -1225771296 (LWP 31943)]
0xb72d0113 in _nv000048gl () from /usr/lib/libGLcore.so.1
(gdb) bt full
#0  0xb72d0113 in _nv000048gl () from /usr/lib/libGLcore.so.1
No symbol table info available.
#1  0x00000001 in ?? ()
No symbol table info available.
#2  0xbfefe248 in ?? ()
No symbol table info available.
#3  0x0000003f in ?? ()
No symbol table info available.
Previous frame inner to this frame (corrupt stack?)

Edited by bluap

Share this post


Link to post
Share on other sites

Yeah, the idle issue was due to a misunderstanding on my part of how the display handlers were being called. The elwindows.c one gets run many times per, say, gamewin.c display call. I guess it calls many different handlers, one at a time. I had sort of a "what the heck?" moment when I realized that eye candy thought it was getting a framerate of several hundred per second, judged by idle calls :hehe:

 

WindEffect (0xe268a48) created.

WindEffect (0xe26a600) created.

Deactivating effect 0xe268a48(7301 > 250)

Deactivating effect 0xe26a600(8381 > 250)

WindEffect (0xe268a48) destroyed.

WindEffect (0xe26a600) destroyed.

 

Program received signal SIGSEGV, Segmentation fault.

 

There are no effects running when your program crashes. So unless it's crashing in the destructor of the last effect... of course, then that would beg the question, why didn't it crash in the destructor of the previous effect?

 

I'd wager that this problem is unrelated. I can't guarantee it, but it looks that way.

It's also something that I can't reproduce, so if you want me to debug it for you, I'll need a lot more info. :)

Edited by KarenRei

Share this post


Link to post
Share on other sites

There are no effects running when your program crashes. So unless it's crashing in the destructor of the last effect... of course, then that would beg the question, why didn't it crash in the destructor of the previous effect?

I'd wager that this problem is unrelated. I can't guarantee it, but it looks that way.

It's also something that I can't reproduce, so if you want me to debug it for you, I'll need a lot more info. :confused:

True. So I've had a play with electric fence and valgrind. You don't want to know the number of things picked up in GLcore! Anyway, these tools did spot a probable problem in push_texture() of eye_candy.cpp - there were pages of warnings about accessing freed memory. I'm pretty sure the culprit is the SDL_FreeSurface(tex) at line 83, which is then followed by several accesses of tex. Probably we get lucky but its worth fixing. EF gives up during the client initialisation so it's of no more use but valgrind may be. There are other reported issues that I don't understand so I'll just give you the end of the valgrind output. This was a clean exit by the way:

<snip the push_texture() and GLcore! warnings>...
WindEffect (0x7180510) created.
WindEffect (0xd5f54d8) created.
Deactivating effect 0x7180510(7301 > 250)
Deactivating effect 0xd5f54d8(8381 > 250)
==8339== 
==8339== Use of uninitialised value of size 4
==8339==	at 0x44A8519: (within /lib/tls/libc-2.3.6.so)
==8339==	by 0x44AC6EA: vfprintf (in /lib/tls/libc-2.3.6.so)
==8339==	by 0x44CB210: vsnprintf (in /lib/tls/libc-2.3.6.so)
==8339==	by 0x44B2364: snprintf (in /lib/tls/libc-2.3.6.so)
==8339==	by 0x80AFEA5: add_enhanced_actor_from_server (new_actors.c:641)
==8339==	by 0x4482EA7: (below main) (in /lib/tls/libc-2.3.6.so)
==8339== 
==8339== Conditional jump or move depends on uninitialised value(s)
==8339==	at 0x44A8521: (within /lib/tls/libc-2.3.6.so)
==8339==	by 0x44AC6EA: vfprintf (in /lib/tls/libc-2.3.6.so)
==8339==	by 0x44CB210: vsnprintf (in /lib/tls/libc-2.3.6.so)
==8339==	by 0x44B2364: snprintf (in /lib/tls/libc-2.3.6.so)
==8339==	by 0x80AFEA5: add_enhanced_actor_from_server (new_actors.c:641)
==8339==	by 0x4482EA7: (below main) (in /lib/tls/libc-2.3.6.so)
==8339== 
==8339== Conditional jump or move depends on uninitialised value(s)
==8339==	at 0x44AA116: vfprintf (in /lib/tls/libc-2.3.6.so)
==8339==	by 0x44CB210: vsnprintf (in /lib/tls/libc-2.3.6.so)
==8339==	by 0x44B2364: snprintf (in /lib/tls/libc-2.3.6.so)
==8339==	by 0x80AFEA5: add_enhanced_actor_from_server (new_actors.c:641)
==8339==	by 0x4482EA7: (below main) (in /lib/tls/libc-2.3.6.so)
==8339== 
==8339== Invalid read of size 4
==8339==	at 0x8100F94: ec::Vec3::operator-=(ec::Vec3 const&) (eye_candy.h:146)
==8339==	by 0x8100F6A: ec::Vec3::operator-(ec::Vec3 const&) const (eye_candy.h:162)
==8339==	by 0x81171BC: ec::EyeCandy::idle() (eye_candy.cpp:1369)
==8339==	by 0x80F5D53: ec_idle (eye_candy_wrapper.cpp:89)
==8339==	by 0x808B854: display_game_handler (gamewin.c:679)
==8339==	by 0x4482EA7: (below main) (in /lib/tls/libc-2.3.6.so)
==8339==  Address 0xF63313C is not stack'd, malloc'd or (recently) free'd
==8339== 
==8339== Invalid read of size 4
==8339==	at 0x8100FA4: ec::Vec3::operator-=(ec::Vec3 const&) (eye_candy.h:147)
==8339==	by 0x8100F6A: ec::Vec3::operator-(ec::Vec3 const&) const (eye_candy.h:162)
==8339==	by 0x81171BC: ec::EyeCandy::idle() (eye_candy.cpp:1369)
==8339==	by 0x80F5D53: ec_idle (eye_candy_wrapper.cpp:89)
==8339==	by 0x808B854: display_game_handler (gamewin.c:679)
==8339==	by 0x4482EA7: (below main) (in /lib/tls/libc-2.3.6.so)
==8339==  Address 0xF633140 is not stack'd, malloc'd or (recently) free'd
==8339== 
==8339== Invalid read of size 4
==8339==	at 0x8100FB6: ec::Vec3::operator-=(ec::Vec3 const&) (eye_candy.h:148)
==8339==	by 0x8100F6A: ec::Vec3::operator-(ec::Vec3 const&) const (eye_candy.h:162)
==8339==	by 0x81171BC: ec::EyeCandy::idle() (eye_candy.cpp:1369)
==8339==	by 0x80F5D53: ec_idle (eye_candy_wrapper.cpp:89)
==8339==	by 0x808B854: display_game_handler (gamewin.c:679)
==8339==	by 0x4482EA7: (below main) (in /lib/tls/libc-2.3.6.so)
==8339==  Address 0xF633144 is not stack'd, malloc'd or (recently) free'd
==8339== 
==8339== Invalid write of size 1
==8339==	at 0x81173AC: ec::EyeCandy::idle() (eye_candy.cpp:1399)
==8339==	by 0x80F5D53: ec_idle (eye_candy_wrapper.cpp:89)
==8339==	by 0x808B854: display_game_handler (gamewin.c:679)
==8339==	by 0x4482EA7: (below main) (in /lib/tls/libc-2.3.6.so)
==8339==  Address 0xF633174 is not stack'd, malloc'd or (recently) free'd
WindEffect (0x7180510) destroyed.
WindEffect (0xd5f54d8) destroyed.
CampfireEffect (0x54405a0) created.
Deactivating effect 0x54405a0(4906.69 > 250)
CampfireEffect (0x54405a0) destroyed.
==8339== 
==8339== Syscall param write(buf) points to uninitialised byte(s)
==8339==	at 0x452AE11: write (in /lib/tls/libc-2.3.6.so)
==8339==	by 0x44CD614: (within /lib/tls/libc-2.3.6.so)
==8339==	by 0x44CD8CE: _IO_do_write (in /lib/tls/libc-2.3.6.so)
==8339==	by 0x44CECB9: _IO_file_close_it (in /lib/tls/libc-2.3.6.so)
==8339==	by 0x44C318B: fclose (in /lib/tls/libc-2.3.6.so)
==8339==	by 0x80CED2E: save_quickspells (spells.c:730)
==8339==	by 0x4482EA7: (below main) (in /lib/tls/libc-2.3.6.so)
==8339==  Address 0x8009229 is not stack'd, malloc'd or (recently) free'd
==8339== 
==8339== ERROR SUMMARY: 2472 errors from 520 contexts (suppressed: 73 from 1)
==8339== malloc/free: in use at exit: 34,493,783 bytes in 1,052,531 blocks.
==8339== malloc/free: 1,878,647 allocs, 826,116 frees, 794,610,281 bytes allocated.
==8339== For counts of detected errors, rerun with: -v
==8339== searching for pointers to 1,052,531 not-freed blocks.

 

I have not been able to get a crash while running valgrind yet. Hope the above helps anyway.

Share this post


Link to post
Share on other sites

I've noticed those uninitialized accesses in push texture before, and I seem to recall that it wasn't worth my time to delve further. Let's take a look at the other stuff.

 

==8339== Invalid read of size 4

==8339== at 0x8100F94: ec::Vec3::operator-=(ec::Vec3 const&) (eye_candy.h:146)

==8339== by 0x8100F6A: ec::Vec3::operator-(ec::Vec3 const&) const (eye_candy.h:162)

==8339== by 0x81171BC: ec::EyeCandy::idle() (eye_candy.cpp:1369)

==8339== by 0x80F5D53: ec_idle (eye_candy_wrapper.cpp:89)

==8339== by 0x808B854: display_game_handler (gamewin.c:679)

==8339== by 0x4482EA7: (below main) (in /lib/tls/libc-2.3.6.so)

==8339== Address 0xF63313C is not stack'd, malloc'd or (recently) free'd

 

Actually, this does look like something of mine :confused: The relevant line is in eye_candy.cpp:

 

coord_t distance_squared = (camera - *(e->pos)).magnitude_squared();

 

Unless "this" is already freed (which would be quite odd indeed), the only logical explanation is that the effect "e" is already freed. "e" is declared here:

 

std::vector<Effect*>::iterator iter = effects.begin() + i;

Effect* e = *iter;

 

So, this gives us two possibilities: either 1) "i" is out of range for the vector "effects" (the effects list), or 2) there's an effect in the effects list which has been freed already but for some reason wasn't removed from the vector.

 

In the case of 1), I can only picture a compiler error, because I goes:

 

for (int i = 0; i < (int)effects.size(); )

 

The only changes to i that occur in the vector are a single i++ immediately before the continuation of the loop (the reason that the i++ isn't in that for statement is because sometimes, we don't increment -- we delete). So, I'm inclined to suspect #2: "there's an effect in the effects list which has been freed already but for some reason wasn't removed from the vector."

 

There's a problem with that, though: there is only one place in all of eye candy that effects get deleted, and it's in that for loop:

 

const bool ret = e->idle(time_diff);

if (!ret)

{

e->recall = true;

*(e->dead) = true;

effects.erase(iter);

delete e;

}

 

That is, to say, it runs the effect's idle function. If the effect says "Okay, I'm ready to be erased", it flags the effect as "recalled" (just in case), sets the dead flag that was passed to the effect (a way for whoever called an effect to know if it's still around), removes the effect from the effects list, and frees its memory. And, no, neither the iterator for this effect, nor the pointer to the effect, are changed since they're set at the top of the loop.

 

So... this is curious. My current, leading theory (someone who knows the answer, go ahead and dash it): EL is threaded. Is it? Because eye_candy wasn't designed with threads in mind and uses no mutexes, so if we get two idle functions running at once, it could bomb when the array changes size.

 

Bluap: You could test this theory if you wanted. At the top of EyeCandy::idle(), put a print statement like "Starting idle." At the bottom of that function, put a statement like "Stopping idle." If we ever see two starting idles in a row, that's the problem.

Share this post


Link to post
Share on other sites

I've noticed those uninitialized accesses in push texture before, and I seem to recall that it wasn't worth my time to delve further.

That's a shame as the problem is easily explained because you do actually free the memory then access it straight away afterwards. Moving the free to the end of the function removed all the valgrind warnings. Surely, its such a no brainer you might as well fix it :confused: I left the valgrid message out before because I thought it was explainable but here's one of the many:

==8339== Invalid read of size 4
==8339==	at 0x8106F1E: ec::Texture::push_texture(std::string) (eye_candy.cpp:89)
==8339==	by 0x8115D7E: ec::EyeCandy::load_textures(std::string) (eye_candy.cpp:1188)
==8339==	by 0x80F594C: ec_init (eye_candy_wrapper.cpp:22)
==8339==	by 0x809782D: init_stuff (init.c:604)
==8339==	by 0x4482EA7: (below main) (in /lib/tls/libc-2.3.6.so)
==8339==  Address 0x50CF848 is 8 bytes inside a block of size 60 free'd
==8339==	at 0x401CFA5: free (vg_replace_malloc.c:233)
==8339==	by 0x406EA6D: SDL_FreeSurface (in /usr/lib/libSDL-1.2.so.0.11.0)
==8339==	by 0x8106EE1: ec::Texture::push_texture(std::string) (eye_candy.cpp:83)
==8339==	by 0x8115D7E: ec::EyeCandy::load_textures(std::string) (eye_candy.cpp:1188)
==8339==	by 0x80F594C: ec_init (eye_candy_wrapper.cpp:22)
==8339==	by 0x809782D: init_stuff (init.c:604)
==8339==	by 0x4482EA7: (below main) (in /lib/tls/libc-2.3.6.so)

 

 

Bluap: You could test this theory if you wanted. At the top of EyeCandy::idle(), put a print statement like "Starting idle." At the bottom of that function, put a statement like "Stopping idle." If we ever see two starting idles in a row, that's the problem.

Yeah, I've tried to understand the threads before, may be someone who knows could explain what they all do. Anyway, I'm editing the code now, I'll keep you posted.

 

Edits:

1) to add the valgrind warning.

 

2) To report no sign of multiple thread access. Sorry.

Edited by bluap

Share this post


Link to post
Share on other sites

Blah, you're right. It's been ages since I even looked at that, so I was just going by memory. Running EL through valgrind is kind of a pain because it goes so slowly. It's one of those things that I just kind of got used to.

 

Assuming that's the issue (which it should be), it'll be fixed in the next release. As for the issue with deleted effects, thanks for running this test. :P

 

Edit: I'll see what I can come up with myself this evening. Thanks for all you did.

Edited by KarenRei

Share this post


Link to post
Share on other sites

Latest patch:

 

* Made threadsafe, just in case

* Fixed read of deleted memory (thanks, Bluap)

* Slight improvements to leaf color

* Campfire sizes, etc.

* Hooked in optional blood effects (may need some work)

* Probably fixed the bug where, after an effect, created lights would stick

around.

* Converted math cache to floats to save memory.

 

Tomorrow, I'll probably work on getting effect obstructions hooked in.

Share this post


Link to post
Share on other sites

Hi,

 

I get another try at compiling the EYE_CANDY client. There are still some minor compile problems which can be resolved by using this patch

 

Index: console.h
===================================================================
RCS file: /cvsroot/elc/elc/console.h,v
retrieving revision 1.17
diff -u -r1.17 console.h
--- console.h	12 Mar 2007 18:13:29 -0000	1.17
+++ console.h	17 Mar 2007 10:42:14 -0000
@@ -16,7 +16,7 @@
 #define DEF_INFO ""
#endif

-typedef struct command {
+typedef struct {
 char command[64];
 int (*callback)();
} command_t;
Index: eye_candy/effect_breath.h
===================================================================
RCS file: /cvsroot/elc/elc/eye_candy/effect_breath.h,v
retrieving revision 1.4
diff -u -r1.4 effect_breath.h
--- eye_candy/effect_breath.h	15 Mar 2007 06:10:33 -0000	1.4
+++ eye_candy/effect_breath.h	17 Mar 2007 10:49:47 -0000
@@ -37,7 +37,7 @@
 else
   LOD = desired_LOD;
 count_scalar = 3000 / LOD;
-	size_scalar = scale * fastsqrt(LOD) / sqrt(10);
+	size_scalar = scale * fastsqrt(LOD) / sqrt(10.0f);
  };
  static Uint64 get_max_end_time() { return 5000000; };
  virtual Uint64 get_expire_time() { return 5000000 + born; };
Index: eye_candy/eye_candy.h
===================================================================
RCS file: /cvsroot/elc/elc/eye_candy/eye_candy.h,v
retrieving revision 1.8
diff -u -r1.8 eye_candy.h
--- eye_candy/eye_candy.h	16 Mar 2007 06:37:57 -0000	1.8
+++ eye_candy/eye_candy.h	17 Mar 2007 10:55:12 -0000
@@ -87,6 +87,7 @@
 #define round(a) (a - floor(a) < 0.5f ? floor(a) : ceil(a))
 #define remainderf(a, b) (a - (float)round(a / b) * b)
 #define random rand
+ #define usleep(a) Sleep(a / 1000)
#else	
__attribute__ ((noinline)) float fastsqrt(float f);
__attribute__ ((noinline)) float invsqrt(float f);

 

Once build I tried to run it. First the Release build. It crashed right after the logging, however this was also happening for me without EYE_CANDY (for about a week now)

Next I got the Debug build working. I got the campfire effect (colors are ok), I could see leaves (red) and bag drop/get effect. However when the campfire effect died, I got an internal exception with heap corruption. Unfortunatelly I could not invastigate it further.

 

Just for reference my configuration:

 

PentiumM 1.86, GeForce 6600 GO, 512 MB RAM, Windows, Visual Studio 2005 Express Edition

 

Defines:

ELC

WINDOWS

NEW_FRUSTUM

BUG_FIX_3D_OBJECTS_MIN_MAX

NEW_TEX

ATI_9200_FIX

NEW_ACTOR_ANIMATION

AUTO_UPDATE

OPTIONS_I18N

PNG_SCREENSHOT

NOTEPAD

ANTI_ALIAS

COUNTERS

AFK_FIX

CUSTOM_LOOK

CUSTOM_UPDATE

USE_INLINE

FONTS_FIX

SIMPLE_LOD

ZLIB

FUZZY_PATHS

SFX

MASKING

ACTOR_ALPHA

NEW_ALPHA

EYE_CANDY

Edited by Kindar Naar

Share this post


Link to post
Share on other sites

Latest patch:

...

* Fixed read of deleted memory (thanks, Bluap)

...

Great, that's fixed the initialisation valgrind errors. I've been trying to chase down the remaining valgrind errors but have not been able to make much progress. Like you say, running inside valgrind is frustratingly slow.

 

The only thing I can say at the moment is that the error only occurs during destruction of WindEffect, at least it happens for WindEffect but not for CampfireEffect, BagEffect, ImpactEffect, or HarvestingEffect. It's obviously difficult to test any other effects. What we need is a temporary #effect <type> server command.... I've been eye balling the code try to spot a difference but have seen nothing so far. I can't help feeling that this might be related to the intermittent crash I reported earlier in this thread; that happened just after WindEffect destruction. It might me that WindEffect destruction is corrupting memory which leads to the crash.

Share this post


Link to post
Share on other sites

+ #define usleep Sleep

Shouldn't that be #define usleep(x) Sleep(x/1000), since usleep is measured in microseconds, and Sleep in milliseconds?

Share this post


Link to post
Share on other sites

+ #define usleep Sleep

Shouldn't that be #define usleep(x) Sleep(x/1000), since usleep is measured in microseconds, and Sleep in milliseconds?

 

Yes, you are right. I update the patch.

Also, I did test that I crash each time that campfire effect finished (a few seconds after the smoke and fire is gone). The problem is that each time I get a different stack trace, so I assume that some action earlier has corrupted the heap and the action listed on call stack just 'stumbled' upon corrupted heap. VS also reports:

 

HEAP[elc_d.exe]: HEAP: Free Heap block d80dce0 modified at d80dd54 after it was freed
Windows has triggered a breakpoint in elc_d.exe.

This may be due to a corruption of the heap, and indicates a bug in elc_d.exe or any of the DLLs it has loaded.

 

And here is an example stack trace

 

 ntdll.dll!7c901230()	 
 [Frames below may be incorrect and/or missing, no symbols loaded for ntdll.dll]	
 ntdll.dll!7c96c943()	 
 ntdll.dll!7c949eb9()	 
>	elc_d.exe!std::_Iterator_base::operator=(const std::_Iterator_base & _Right={...})  Line 145 + 0xf bytes	C++
 ntdll.dll!7c96d6aa()	 
 ntdll.dll!7c949d18()	 
 elc_d.exe!std::_Iterator_base::~_Iterator_base()  Line 151 + 0x45 bytes	C++
 ntdll.dll!7c91b298()	 
 msvcr80d.dll!1020506c()	 
 msvcr80d.dll!10205089()	 
 msvcr80d.dll!102846a9()	 
 elc_d.exe!ec::randfloat()  Line 1735 + 0xf bytes	C++
 elc_d.exe!ec::randcoord()  Line 1751 + 0x5 bytes	C++
 elc_d.exe!ec::Vec3::randomize(const float scale=2.578e-043#DEN)  Line 249 + 0xe bytes	C++
 elc_d.exe!ec::WindEffect::idle(const unsigned __int64 usec=937500)  Line 645 + 0xa bytes	C++
 elc_d.exe!ec::EyeCandy::idle()  Line 1398 + 0x20 bytes	C++
 elc_d.exe!ec_idle()  Line 105	C++
 elc_d.exe!display_game_handler(window_info * win=0x050c1e78)  Line 680	C
 elc_d.exe!draw_window(window_info * win=0x050c1e78)  Line 1059 + 0x11 bytes	C
 elc_d.exe!display_window(int win_id=0)  Line 1207 + 0x15 bytes	C
 elc_d.exe!display_windows(int level=1)  Line 57 + 0x9 bytes	C
 elc_d.exe!draw_scene()  Line 98 + 0x7 bytes	C
 elc_d.exe!start_rendering()  Line 124	C
 elc_d.exe!Main(int argc=-1, char * * argv=0x00000000)  Line 251	C
 elc_d.exe!WinMain(HINSTANCE__ * hInst=0x00400000, HINSTANCE__ * hPrev=0x00000000, char * lpCmd=0x00151ef7, int nShow=1)  Line 306 + 0xd bytes	C
 elc_d.exe!__tmainCRTStartup()  Line 589 + 0x35 bytes	C
 elc_d.exe!WinMainCRTStartup()  Line 414	C
 kernel32.dll!7c816fd7()	 
 elc_d.exe!ec::CampfireEffect::idle(const unsigned __int64 usec=30962698415439964)  Line 200 + 0x21 bytes	C++
 elc_d.exe!ec::CampfireEffect::idle(const unsigned __int64 usec=14757395258967641292)  Line 200 + 0x21 bytes	C++
 cccccccc()	

 

One thing I notice it that with each crash the stack trace goes up to the level of EyeCandy::idle. Below that level, call are different each time.

Share this post


Link to post
Share on other sites

The only thing I can say at the moment is that the error only occurs during destruction of WindEffect, at least it happens for WindEffect but not for CampfireEffect, BagEffect, ImpactEffect, or HarvestingEffect. It's obviously difficult to test any other effects. What we need is a temporary #effect <type> server command.... I've been eye balling the code try to spot a difference but have seen nothing so far. I can't help feeling that this might be related to the intermittent crash I reported earlier in this thread; that happened just after WindEffect destruction. It might me that WindEffect destruction is corrupting memory which leads to the crash.

 

I would welcome a #effect command, although it wouldn't be that simple. Different effects take different arguments.

 

WindEffect's destructor is incredibly simple:

 

----

delete mover;

delete spawner;

if (EC_DEBUG)

std::cout << "WindEffect (" << this << ") destroyed." << std::endl;

----

 

Not much to it. There's only one constructor, and in it, we have:

 

----

mover = new ParticleMover(this);

spawner = new FilledPolarCoordsSpawner(_bounding_range);

----

 

Neither are deleted anywhere else.

 

I feel bad, because there's not much I can do to debug this. Try as I might, I haven't been able to reproduce your crash on either of my computers. I'll try another valgrind run this evening and see if I can weasel anything useful out of it. :) Have you tried making clean and then rebuilding? Sometimes sources get out of sync with .o files.

 

If you want to debug this, try hollowing out WindEffect bit by bit. Keep the recall checks in both the effect's idle and the particles' idle, don't mess with get_texture, and apart from that, feel free to strip away stuff. Another option would be adding in debugging statements -- although if the issue is memory corruption, I'm not sure what you could do: I use no conventional arrays in the wind effect, and only a few in eye candy. You could also try disabling the wind effect altogether in multiplayer.c.

 

 

 

The problem is that each time I get a different stack trace, so I assume that some action earlier has corrupted the heap and the action listed on call stack just 'stumbled' upon corrupted heap. VS also reports:

 

HEAP[elc_d.exe]: HEAP: Free Heap block d80dce0 modified at d80dd54 after it was freed
Windows has triggered a breakpoint in elc_d.exe.

This may be due to a corruption of the heap, and indicates a bug in elc_d.exe or any of the DLLs it has loaded.

 

And here is an example stack trace

 

 

I can't make heads or tails of that stack trace. Example: it traces back to line 645 of idle, but then, in the next step, complains about randcoord. There is no randcoord in that line. What on Earth is it tracing? :icon13: And I don't get the logic: it somehow manages to jump from randfloat into something about iterators. What? All randfloat is is this:

 

----

return (float)rand() / (float)RAND_MAX;

----

 

No iterators there. :P

 

In short, I can't make heads or tails of that trace, and I'm not sure whether it's actually giving any relevant information at all.

 

Are you still getting crashes when you build without Eye Candy? If so, I'd recommend not building with eye candy and tracking that down first. Reducing a problem to a simpler one is always a good start.

Share this post


Link to post
Share on other sites

Hi,

 

I managed to get one problem solved today. I kept getting crashes in 'Release' version of the client, just after logging in. However the 'Debug' version was fine. After some analysis I came to this offending line

 

WindNeighbor* previous = &(*(neighbors.begin() + (neighbors.size() - 1)));

 

in effect_wind.cpp

 

This line works as long as there is at least one object in the neighbors vector. When the vector is empty, ms C runtime in kind enough to crash. I don't know if on other platform the behaviour is the same.. or if the runtime just lets 'get' an object from (begin - 1).

 

The problem however was, why do I get 1 object in 'Debug' while 0 in 'Release'

 

After some more invastingation I notice that invsqrt function gives DIFFERENT results in both modes. The source of the incompability is the optimization that is done by the compiler. I tried different settings and finally had to disable the used of SSE2 for the eye_candy.cpp. Once this was done, the 'Release' client was able to login and I saw leaves flying everywhere. :fire:

 

To sum up, I see two problems here:

1. the invsqrt function can behave differntly on different compilers/compilere settings

2. becuase of 1., some assumptions (i.e. there always have to be at least one neighbour) can be invalid and lead to crashes/heap corruption/stack corruption

Share this post


Link to post
Share on other sites

Ah, thank you -- something I can work with! :) I can't say why the debug version is different -- perhaps your std::vectors provide padding in debug mode? Not sure. I'm checking in what I assume will be a fix (not running that code if there are no neighbors). That code generates the neighbors list, but the rest of the code should be content without having a neighbors list.

 

Invsqrt: Well, worst case solution, we use an ifdef to return 1.0 / sqrt(f) in the case of windows. However, that's not desirable; invsqrt is two to four times faster than the regular square root function, by my testing. Given that the use of square roots is common (even after optimizing them out wherever possible) due to the nature of 3d math, it would be a relevant slowdown to not have that in there. Not a huge slowdown, but relevant. Of course, not using SSE2 would also be a slowdown. I'm not sure which would be worse.

 

You'll notice that I already had some problems with compilers and invsqrt. You'll see the invsqrt_workaround function. The initial function I implemented didn't use this. It worked fine on lower levels of optimization, but on higher levels of optimization, the compiler would try and be "smart", and its simplifications would mess it up. So, I broke that out into a separate function and classed it as noinline. This let it work fine under g++. I'm not sure why your compiler decides to break it in its present form. It could be ignoring the noinline, thinking that it's smarter than you :D That's not as far fetched as it sounds; I know that some compilers will ignore statements where you tell things *to* be inline when it decides that it would be faster to not inline that code. Another possibility is that there's some other line that should be noinline -- perhaps the "int i = ..." line? I'm not sure beyond that, if neither of those work. It would be interesting to see if you could try things out, or at least detemine what's faster: no SSE2, or using 1.0 / sqrt(f) instead of the normal invsqrt function.

 

Whatever you decide is best (I can't test on windows), I'll make the default in here.

 

One possibility if we can't fix make the compiler do what it's told in Windows: I could create a new object that only has invsqrt in it -- say, invsqrt.cpp. For this one file, in the case of Windows, we would build it without SSE2. For everything else, SSE2 would be used.

 

Let me know what you think.

Edited by KarenRei

Share this post


Link to post
Share on other sites

Whatever you decide is best (I can't test on windows), I'll make the default in here.

 

One possibility if we can't fix make the compiler do what it's told in Windows: I could create a new object that only has invsqrt in it -- say, invsqrt.cpp. For this one file, in the case of Windows, we would build it without SSE2. For everything else, SSE2 would be used.

 

Let me know what you think.

 

I think this is the best approach as I don't know if it is possible to disable the optimisation from code using some attributes. I would like to see the three functions: fastsqrt, invsqrt and invsqrt_workaround moved to a new .cpp file with added comment that SSE2 optimisation should be disabled for this file. Otherwise everyone else trying to get an optimized client from Visual Studio would get the same errors.

The rest of the functions in eye_candy.cpp could continue using SSE2 optiomization.

 

When making these changes could you also apply the patch I listed earlier so that the codes build under VS?

Share this post


Link to post
Share on other sites

That went in with the last run. Although, it's strange: you keep listing that 10 vs. 10.0 line (I think it is in effect_breath) which I fixed ages ago. I keep checking, and it keeps showing up as fixed on mine. Strange.

 

I'll go ahead and move those functions to a new file; I'll edit this post when it's committed.

Share this post


Link to post
Share on other sites
That went in with the last run. Although, it's strange: you keep listing that 10 vs. 10.0 line (I think it is in effect_breath) which I fixed ages ago. I keep checking, and it keeps showing up as fixed on mine. Strange.

 

I'll go ahead and move those functions to a new file; I'll edit this post when it's committed.

 

It's still 10 at my side :| I have revision 1.4 of effect_breath.h

Share this post


Link to post
Share on other sites

Mine says 1.5. You sure you can't update?

 

Patch committed; the invsqrt function is now broken out. I left fastsqrt in eye candy since it's so simple; if we need to move it later, I will.

Share this post


Link to post
Share on other sites

Obstructions:

 

I've been working on hooking in obstructions. The idea was to have two lists (one for campfires and one for objects flying horizontally at roughly eye level. It was suggested that I base obstruction on file names. I started working along this line (with the concept of having 3d_objects.c/add_e3d do the additions, the corresponding removals done likewise, and the same for actors). However, I'm running into a *lot* of files that I'll need to check for; even globbing filenames still comes into an awful lot of checks. Even further, I have the problem that I don't know what sort of bounding box best describes these objects. I see "rock_big3" and I have no clue what to bound it by. In short, I think I'm going down the wrong track here, and don't want to go further without some feedback.

 

Suggestions on how to get bounding objects? Any simple object will do. If I don't already have a suitable bounding object coded, the way I designed them, they're trivial to code.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

×