View Issue Details

IDProjectCategoryView StatusLast Update
0003067FSSCPgraphicspublic2014-06-27 13:54
ReporterMageKing17 Assigned ToMageKing17  
PrioritylowSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Product Version3.7.2 
Summary0003067: Nebula flashing not following player view point
DescriptionWith 2945 getting fixed, it's now very noticeable that the nebula lightning code doesn't actually account for the camera's current location, so the farther away from your starting point you get, the more the lightning flashes are all clustered towards the "center" of where the screen would be if it were at the starting point of the level, so you get a constant stream of bright white flashes. I'm currently working on fixing this behavior.
TagsNo tags attached.

Relationships

related to 0002945 resolvedMageKing17 No EMP/Lightning sound in debug mode 

Activities

Goober5000

2014-06-21 04:20

administrator   ~0015899

What exactly is it about the change that "makes the flashes more annoying"?

MageKing17

2014-06-21 04:51

developer   ~0015900

See http://www.hard-light.net/forums/index.php?topic=87863.msg1753609#msg1753609 for the complaint that inspired this (although he says he's fine with the EMP flashes, so I guess globally scaling all flashes isn't the desired behavior).

MageKing17

2014-06-21 07:15

developer   ~0015901

I've made a third patch that, instead of adding a commandline parameter to act as a kludge to reduce the "annoyingness" of the flashes (probably by getting rid of them altogether), attempts to apply some sanity to the flashes by scaling them based on how far away the bolt is from the screen. Despite this being a change from the retail code, it manages to feel closer to playing retail.

MageKing17

2014-06-21 16:07

developer   ~0015902

After further investigation, it looks like this is an actual bug whereby the code isn't accounting for camera movement at all.

MageKing17

2014-06-21 16:36

developer  

neblightning.cpp.patch (3,833 bytes)   
Index: code/nebula/neblightning.cpp
===================================================================
--- code/nebula/neblightning.cpp	(revision 10831)
+++ code/nebula/neblightning.cpp	(working copy)
@@ -931,6 +931,7 @@
 	size_t idx;	
 	vec3d temp, pt;
 	vec3d glow_a, glow_b;
+	ubyte flags;
 
 	// direction matrix
 	vm_vec_sub(&dir, &a->pos, &b->pos);
@@ -955,15 +956,17 @@
 		vm_vec_add2(&pt, &a->pos);
 			
 		// transform
-		if (Cmdline_nohtl) {
-			g3_rotate_vertex(&c->vex[idx], &pt);
-		} else {
+		g3_rotate_vertex(&c->vex[idx], &pt);
+		g3_project_vertex(&c->vex[idx]);		
+		if (!Cmdline_nohtl) {	// This is a dirty, dirty hack -MageKing17
+			flags = c->vex[idx].flags;
 			g3_transfer_vertex(&c->vex[idx], &pt);
+			c->vex[idx].flags = flags;
 		}
-		g3_project_vertex(&c->vex[idx]);		
 
 		// if first frame, keep track of the average screen pos
-		if((c->vex[idx].screen.xyw.x >= 0)
+		if( !(c->vex[idx].flags & PF_OVERFLOW)
+			&& (c->vex[idx].screen.xyw.x >= 0)
 			&& (c->vex[idx].screen.xyw.x < gr_screen.max_w)
 			&& (c->vex[idx].screen.xyw.y >= 0)
 			&& (c->vex[idx].screen.xyw.y < gr_screen.max_h))
@@ -975,18 +978,18 @@
 	}
 	// calculate the glow points		
 	nebl_calc_facing_pts_smart(&glow_a, &glow_b, &dir_normal, &a->pos, pinch_a ? 0.5f : width * 6.0f, Nebl_type->b_add);
-	if (Cmdline_nohtl) {
-		g3_rotate_vertex(&c->glow_vex[0], &glow_a);
-	} else {
+	g3_rotate_vertex(&c->glow_vex[0], &glow_a);
+	g3_project_vertex(&c->glow_vex[0]);
+	g3_rotate_vertex(&c->glow_vex[1], &glow_b);
+	g3_project_vertex(&c->glow_vex[1]);	
+	if (!Cmdline_nohtl) {	// This is a dirty, dirty hack -MageKing17
+		flags = c->glow_vex[0].flags;
 		g3_transfer_vertex(&c->glow_vex[0], &glow_a);
-	}
-	g3_project_vertex(&c->glow_vex[0]);
-	if (Cmdline_nohtl) {
-		g3_rotate_vertex(&c->glow_vex[1], &glow_b);
-	} else {
+		c->glow_vex[0].flags = flags;
+		flags = c->glow_vex[1].flags;
 		g3_transfer_vertex(&c->glow_vex[1], &glow_b);
+		c->glow_vex[1].flags = flags;
 	}
-	g3_project_vertex(&c->glow_vex[1]);	
 
 	// maybe do a cap
 	if(cap != NULL){		
@@ -1002,15 +1005,17 @@
 			vm_vec_add2(&pt, &b->pos);
 			
 			// transform
-			if (Cmdline_nohtl) {
-				g3_rotate_vertex(&cap->vex[idx], &pt);
-			} else {
+			g3_rotate_vertex(&cap->vex[idx], &pt);
+			g3_project_vertex(&cap->vex[idx]);			
+			if (!Cmdline_nohtl) {	// This is a dirty, dirty hack -MageKing17
+				flags = cap->vex[idx].flags;
 				g3_transfer_vertex(&cap->vex[idx], &pt);
+				cap->vex[idx].flags = flags;
 			}
-			g3_project_vertex(&cap->vex[idx]);			
 
 			// if first frame, keep track of the average screen pos			
-			if( (cap->vex[idx].screen.xyw.x >= 0)
+			if( !(cap->vex[idx].flags & PF_OVERFLOW)
+				&& (cap->vex[idx].screen.xyw.x >= 0)
 				&& (cap->vex[idx].screen.xyw.x < gr_screen.max_w)
 				&& (cap->vex[idx].screen.xyw.y >= 0)
 				&& (cap->vex[idx].screen.xyw.y < gr_screen.max_h))
@@ -1023,18 +1028,18 @@
 		
 		// calculate the glow points		
 		nebl_calc_facing_pts_smart(&glow_a, &glow_b, &dir_normal, &b->pos, pinch_b ? 0.5f : width * 6.0f, bi->b_add);
-		if (Cmdline_nohtl) {
-			g3_rotate_vertex(&cap->glow_vex[0], &glow_a);
-		} else {
+		g3_rotate_vertex(&cap->glow_vex[0], &glow_a);
+		g3_project_vertex(&cap->glow_vex[0]);
+		g3_rotate_vertex(&cap->glow_vex[1], &glow_b);
+		g3_project_vertex(&cap->glow_vex[1]);
+		if (!Cmdline_nohtl) {	// This is a dirty, dirty hack -MageKing17
+			flags = cap->glow_vex[0].flags;
 			g3_transfer_vertex(&cap->glow_vex[0], &glow_a);
-		}
-		g3_project_vertex(&cap->glow_vex[0]);
-		if (Cmdline_nohtl) {
-			g3_rotate_vertex(&cap->glow_vex[1], &glow_b);
-		} else {
+			cap->glow_vex[0].flags = flags;
+			flags = cap->glow_vex[1].flags;
 			g3_transfer_vertex(&cap->glow_vex[1], &glow_b);
+			cap->glow_vex[1].flags = flags;
 		}
-		g3_project_vertex(&cap->glow_vex[1]);
 	}
 }
 
neblightning.cpp.patch (3,833 bytes)   

MageKing17

2014-06-21 16:37

developer   ~0015903

Okay, I've got a patch that fixes the actual issue at hand, but it is clearly a kludge. If anybody with actual C/C++ experience wants to make a better fix, by all means, do so.

MageKing17

2014-06-24 02:00

developer  

3067.patch (4,387 bytes)   
Index: code/nebula/neblightning.cpp
===================================================================
--- code/nebula/neblightning.cpp	(revision 10833)
+++ code/nebula/neblightning.cpp	(working copy)
@@ -955,15 +955,15 @@
 		vm_vec_add2(&pt, &a->pos);
 			
 		// transform
-		if (Cmdline_nohtl) {
-			g3_rotate_vertex(&c->vex[idx], &pt);
-		} else {
-			g3_transfer_vertex(&c->vex[idx], &pt);
+		g3_rotate_vertex(&c->vex[idx], &pt);
+		g3_project_vertex(&c->vex[idx]);
+		if (!Cmdline_nohtl) {
+			g3_transfer_vertex(&c->vex[idx], &pt, true);
 		}
-		g3_project_vertex(&c->vex[idx]);		
 
 		// if first frame, keep track of the average screen pos
-		if((c->vex[idx].screen.xyw.x >= 0)
+		if( !(c->vex[idx].flags & PF_OVERFLOW)
+			&& (c->vex[idx].screen.xyw.x >= 0)
 			&& (c->vex[idx].screen.xyw.x < gr_screen.max_w)
 			&& (c->vex[idx].screen.xyw.y >= 0)
 			&& (c->vex[idx].screen.xyw.y < gr_screen.max_h))
@@ -975,18 +975,14 @@
 	}
 	// calculate the glow points		
 	nebl_calc_facing_pts_smart(&glow_a, &glow_b, &dir_normal, &a->pos, pinch_a ? 0.5f : width * 6.0f, Nebl_type->b_add);
-	if (Cmdline_nohtl) {
-		g3_rotate_vertex(&c->glow_vex[0], &glow_a);
-	} else {
-		g3_transfer_vertex(&c->glow_vex[0], &glow_a);
-	}
+	g3_rotate_vertex(&c->glow_vex[0], &glow_a);
 	g3_project_vertex(&c->glow_vex[0]);
-	if (Cmdline_nohtl) {
-		g3_rotate_vertex(&c->glow_vex[1], &glow_b);
-	} else {
-		g3_transfer_vertex(&c->glow_vex[1], &glow_b);
+	g3_rotate_vertex(&c->glow_vex[1], &glow_b);
+	g3_project_vertex(&c->glow_vex[1]);
+	if (!Cmdline_nohtl) {
+		g3_transfer_vertex(&c->glow_vex[0], &glow_a, true);
+		g3_transfer_vertex(&c->glow_vex[1], &glow_b, true);
 	}
-	g3_project_vertex(&c->glow_vex[1]);	
 
 	// maybe do a cap
 	if(cap != NULL){		
@@ -1002,15 +998,15 @@
 			vm_vec_add2(&pt, &b->pos);
 			
 			// transform
-			if (Cmdline_nohtl) {
-				g3_rotate_vertex(&cap->vex[idx], &pt);
-			} else {
-				g3_transfer_vertex(&cap->vex[idx], &pt);
+			g3_rotate_vertex(&cap->vex[idx], &pt);
+			g3_project_vertex(&cap->vex[idx]);
+			if (!Cmdline_nohtl) {
+				g3_transfer_vertex(&cap->vex[idx], &pt, true);
 			}
-			g3_project_vertex(&cap->vex[idx]);			
 
 			// if first frame, keep track of the average screen pos			
-			if( (cap->vex[idx].screen.xyw.x >= 0)
+			if( !(cap->vex[idx].flags & PF_OVERFLOW)
+				&& (cap->vex[idx].screen.xyw.x >= 0)
 				&& (cap->vex[idx].screen.xyw.x < gr_screen.max_w)
 				&& (cap->vex[idx].screen.xyw.y >= 0)
 				&& (cap->vex[idx].screen.xyw.y < gr_screen.max_h))
@@ -1023,18 +1019,14 @@
 		
 		// calculate the glow points		
 		nebl_calc_facing_pts_smart(&glow_a, &glow_b, &dir_normal, &b->pos, pinch_b ? 0.5f : width * 6.0f, bi->b_add);
-		if (Cmdline_nohtl) {
-			g3_rotate_vertex(&cap->glow_vex[0], &glow_a);
-		} else {
-			g3_transfer_vertex(&cap->glow_vex[0], &glow_a);
-		}
+		g3_rotate_vertex(&cap->glow_vex[0], &glow_a);
 		g3_project_vertex(&cap->glow_vex[0]);
-		if (Cmdline_nohtl) {
-			g3_rotate_vertex(&cap->glow_vex[1], &glow_b);
-		} else {
-			g3_transfer_vertex(&cap->glow_vex[1], &glow_b);
+		g3_rotate_vertex(&cap->glow_vex[1], &glow_b);
+		g3_project_vertex(&cap->glow_vex[1]);
+		if (!Cmdline_nohtl) {
+			g3_transfer_vertex(&cap->glow_vex[0], &glow_a, true);
+			g3_transfer_vertex(&cap->glow_vex[1], &glow_b, true);
 		}
-		g3_project_vertex(&cap->glow_vex[1]);
 	}
 }
 
Index: code/render/3d.h
===================================================================
--- code/render/3d.h	(revision 10833)
+++ code/render/3d.h	(working copy)
@@ -354,7 +354,7 @@
  */
 void g3_stop_user_clip_plane();
 
-ubyte g3_transfer_vertex(vertex *dest, vec3d *src);
+ubyte g3_transfer_vertex(vertex *dest, vec3d *src, bool keep_flags = false);
 
 int g3_draw_2d_poly_bitmap_list(bitmap_2d_list* b_list, int n_bm, uint additional_tmap_flags);
 int g3_draw_2d_poly_bitmap_rect_list(bitmap_rect_list* b_list, int n_bm, uint additional_tmap_flags);
Index: code/render/3dmath.cpp
===================================================================
--- code/render/3dmath.cpp	(revision 10833)
+++ code/render/3dmath.cpp	(working copy)
@@ -81,12 +81,14 @@
 
 }
 
-ubyte g3_transfer_vertex(vertex *dest,vec3d *src)
+ubyte g3_transfer_vertex(vertex *dest, vec3d *src, bool keep_flags)
 {
 	dest->world = *src;
 
-	dest->codes = 0;
-	dest->flags = 0;
+	if (!keep_flags) {
+		dest->codes = 0;
+		dest->flags = 0;
+	}
 
 	return 0;
 }
3067.patch (4,387 bytes)   

MageKing17

2014-06-24 02:01

developer   ~0015909

Less-inelegant patch uploaded that avoids the ugly "save the flags and then restore them afterwards" kludge by adding an optional argument to g3_transfer_vertex() that avoids clearing the flags. Still a hack, but I'm less displeased with it (not actually pleased, mind you, but less displeased).

MageKing17

2014-06-24 16:13

developer  

3067_simple.patch (3,487 bytes)   
Index: code/nebula/neblightning.cpp
===================================================================
--- code/nebula/neblightning.cpp	(revision 10833)
+++ code/nebula/neblightning.cpp	(working copy)
@@ -931,6 +931,7 @@
 	size_t idx;	
 	vec3d temp, pt;
 	vec3d glow_a, glow_b;
+	vertex tempv;
 
 	// direction matrix
 	vm_vec_sub(&dir, &a->pos, &b->pos);
@@ -954,39 +955,30 @@
 		}
 		vm_vec_add2(&pt, &a->pos);
 			
-		// transform
-		if (Cmdline_nohtl) {
-			g3_rotate_vertex(&c->vex[idx], &pt);
-		} else {
-			g3_transfer_vertex(&c->vex[idx], &pt);
-		}
-		g3_project_vertex(&c->vex[idx]);		
+		g3_transfer_vertex(&c->vex[idx], &pt);
+		g3_rotate_vertex(&tempv, &pt);
+		g3_project_vertex(&tempv);
 
 		// if first frame, keep track of the average screen pos
-		if((c->vex[idx].screen.xyw.x >= 0)
-			&& (c->vex[idx].screen.xyw.x < gr_screen.max_w)
-			&& (c->vex[idx].screen.xyw.y >= 0)
-			&& (c->vex[idx].screen.xyw.y < gr_screen.max_h))
-		{
-			Nebl_flash_x += c->vex[idx].screen.xyw.x;
-			Nebl_flash_y += c->vex[idx].screen.xyw.y;
+		if (tempv.codes == 0) {
+			Nebl_flash_x += tempv.screen.xyw.x;
+			Nebl_flash_y += tempv.screen.xyw.y;
 			Nebl_flash_count++;
 		}
+
+		if (Cmdline_nohtl) {
+			memcpy(&c->vex[idx], &tempv, sizeof(vertex));
+		}
 	}
 	// calculate the glow points		
 	nebl_calc_facing_pts_smart(&glow_a, &glow_b, &dir_normal, &a->pos, pinch_a ? 0.5f : width * 6.0f, Nebl_type->b_add);
 	if (Cmdline_nohtl) {
 		g3_rotate_vertex(&c->glow_vex[0], &glow_a);
+		g3_rotate_vertex(&c->glow_vex[1], &glow_b);
 	} else {
 		g3_transfer_vertex(&c->glow_vex[0], &glow_a);
-	}
-	g3_project_vertex(&c->glow_vex[0]);
-	if (Cmdline_nohtl) {
-		g3_rotate_vertex(&c->glow_vex[1], &glow_b);
-	} else {
 		g3_transfer_vertex(&c->glow_vex[1], &glow_b);
 	}
-	g3_project_vertex(&c->glow_vex[1]);	
 
 	// maybe do a cap
 	if(cap != NULL){		
@@ -1001,24 +993,20 @@
 			}
 			vm_vec_add2(&pt, &b->pos);
 			
-			// transform
-			if (Cmdline_nohtl) {
-				g3_rotate_vertex(&cap->vex[idx], &pt);
-			} else {
-				g3_transfer_vertex(&cap->vex[idx], &pt);
-			}
-			g3_project_vertex(&cap->vex[idx]);			
+			g3_transfer_vertex(&cap->vex[idx], &pt);
+			g3_rotate_vertex(&tempv, &pt);
+			g3_project_vertex(&tempv);
 
-			// if first frame, keep track of the average screen pos			
-			if( (cap->vex[idx].screen.xyw.x >= 0)
-				&& (cap->vex[idx].screen.xyw.x < gr_screen.max_w)
-				&& (cap->vex[idx].screen.xyw.y >= 0)
-				&& (cap->vex[idx].screen.xyw.y < gr_screen.max_h))
-			{
-				Nebl_flash_x += cap->vex[idx].screen.xyw.x;
-				Nebl_flash_y += cap->vex[idx].screen.xyw.y;
+			// if first frame, keep track of the average screen pos
+			if (tempv.codes == 0) {
+				Nebl_flash_x += tempv.screen.xyw.x;
+				Nebl_flash_y += tempv.screen.xyw.y;
 				Nebl_flash_count++;
 			}
+
+			if (Cmdline_nohtl) {
+				memcpy(&cap->vex[idx], &tempv, sizeof(vertex));
+			}
 		}
 		
 		// calculate the glow points		
@@ -1025,16 +1013,11 @@
 		nebl_calc_facing_pts_smart(&glow_a, &glow_b, &dir_normal, &b->pos, pinch_b ? 0.5f : width * 6.0f, bi->b_add);
 		if (Cmdline_nohtl) {
 			g3_rotate_vertex(&cap->glow_vex[0], &glow_a);
+			g3_rotate_vertex(&cap->glow_vex[1], &glow_b);
 		} else {
 			g3_transfer_vertex(&cap->glow_vex[0], &glow_a);
-		}
-		g3_project_vertex(&cap->glow_vex[0]);
-		if (Cmdline_nohtl) {
-			g3_rotate_vertex(&cap->glow_vex[1], &glow_b);
-		} else {
 			g3_transfer_vertex(&cap->glow_vex[1], &glow_b);
 		}
-		g3_project_vertex(&cap->glow_vex[1]);
 	}
 }
 
3067_simple.patch (3,487 bytes)   

MageKing17

2014-06-24 16:15

developer   ~0015911

Simpler patch that avoids hackish modifications to g3_transfer_vertex() and just uses a temporary vertex object to be projected, because the screen coordinates never need to be referenced for that vertex ever again. Also simplifies the "offscreen" check to use the "codes" value of the vertex, which is used in other code elsewhere and seems rather simpler than re-doing extra work.

Goober5000

2014-06-26 01:49

administrator   ~0015914

I am a huge fan of your patch refinement. :yes: Good justification with solid design rationale. Now all we need is a graphics guy who knows how to properly review this. Swifty, can you take a look?

The_E

2014-06-27 13:54

administrator   ~0015929

Fix committed to trunk in revision 10845

Issue History

Date Modified Username Field Change
2014-06-20 23:08 MageKing17 New Issue
2014-06-20 23:08 MageKing17 File Added: cmdline_flash_intensity.patch
2014-06-20 23:09 MageKing17 Relationship added related to 0002945
2014-06-20 23:09 MageKing17 Status new => code review
2014-06-20 23:09 MageKing17 Assigned To => MageKing17
2014-06-20 23:09 MageKing17 Status code review => assigned
2014-06-20 23:10 MageKing17 Status assigned => code review
2014-06-21 04:20 Goober5000 Note Added: 0015899
2014-06-21 04:51 MageKing17 Note Added: 0015900
2014-06-21 05:34 MageKing17 File Added: nebula_flashes_only.patch
2014-06-21 07:10 MageKing17 File Added: neblightning.cpp.patch
2014-06-21 07:15 MageKing17 Note Added: 0015901
2014-06-21 16:04 MageKing17 File Deleted: cmdline_flash_intensity.patch
2014-06-21 16:04 MageKing17 File Deleted: nebula_flashes_only.patch
2014-06-21 16:04 MageKing17 File Deleted: neblightning.cpp.patch
2014-06-21 16:06 MageKing17 Severity feature => minor
2014-06-21 16:06 MageKing17 Reproducibility N/A => always
2014-06-21 16:06 MageKing17 Status code review => assigned
2014-06-21 16:06 MageKing17 Summary Flashing in nebula now more annoying; add -flash_intensity commandline parameter? => Nebula flashing not following player view point
2014-06-21 16:06 MageKing17 Description Updated
2014-06-21 16:07 MageKing17 Note Added: 0015902
2014-06-21 16:36 MageKing17 File Added: neblightning.cpp.patch
2014-06-21 16:37 MageKing17 Note Added: 0015903
2014-06-21 16:37 MageKing17 Status assigned => code review
2014-06-24 02:00 MageKing17 File Added: 3067.patch
2014-06-24 02:01 MageKing17 Note Added: 0015909
2014-06-24 16:13 MageKing17 File Added: 3067_simple.patch
2014-06-24 16:15 MageKing17 Note Added: 0015911
2014-06-26 01:49 Goober5000 Note Added: 0015914
2014-06-26 01:49 Goober5000 Assigned To MageKing17 => Swifty
2014-06-27 13:46 The_E Assigned To Swifty => The_E
2014-06-27 13:46 The_E Status code review => assigned
2014-06-27 13:54 The_E Note Added: 0015929
2014-06-27 13:54 The_E Status assigned => resolved
2014-06-27 13:54 The_E Resolution open => fixed
2014-06-27 13:54 The_E Assigned To The_E => MageKing17