View Issue Details

IDProjectCategoryView StatusLast Update
0003140FSSCPgameplaypublic2015-05-06 14:55
ReporterItalianmoose Assigned ToMageKing17  
PrioritylowSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
PlatformLinuxOSUbuntuOS Version14.10
Product Version3.7.2 RC5 
Target Version3.7.4Fixed in Version3.7.3 
Summary0003140: Triplewide resolutions mess with aspect lock
DescriptionWhen I'm running my preferred resolution of 5760x1080, the tracking triangle for missiles is oversensitive to motion in yaw, and struggles to track targets moving across the screen - even with a missile like the harpoon.
Steps To ReproduceRun at a very wide resolution
Load up with aspect lock missiles
Try and get a lock whilst manoeuvring
Additional InformationI can only check this in Linux - can't get Windows to run FS2 triplewide.
TagsNo tags attached.

Activities

Italianmoose

2015-02-06 18:52

reporter   ~0016473

Using MVPs, all mods that use aspect lock suffer from this.
I know it's occurred on 3.7.x, before then I can't say as I wasn't running these resolutions before then.

Flags: /media/peter/Main/Games/Freespace 2/fs2_open_3.7.2_RC5 -mod MediaVPs_2014 -nomotiondebris -missile_lighting
-3dshockwave -post_process -soft_particles -fxaa -fb_explosions -no_vsync -no_fps_capping -dualscanlines -targetinfo
-orbradar -rearm_timer -ballistic_gauge -ship_choice_3d -weapon_choice_3d -3dwarp -warp_flash -brief_lighting -fps
-ambient_factor 105 -no_emissive_light -spec_exp 9 -spec_point 0.3 -spec_static 0.8 -spec_tube 0.7 -ogl_spec 120

Italianmoose

2015-02-06 20:01

reporter   ~0016474

Last edited: 2015-02-07 12:25

Problem is down to FS2's missile tracking being based on a fixed rate that doesn't take very different x and y resolutions into account, so with very large x-resolutions the aspect lock can't keep up

See hudlock.cpp

Also may affect missile tracking for everyone not using the original resolution!

niffiwan

2015-02-09 02:39

developer   ~0016481

Just thinking out loud, it might be possible to set a scaling factor in the gauge parsing code (hudparse.cpp) based on the current res vs the original res (640x480?) then apply that to the pixel counts when the locking tri position is being calculated?

MageKing17

2015-02-09 05:12

developer   ~0016484

If the problem were a simple matter of there being too many pixels for the absolute, pixel-based approach to work, then there shouldn't be any difference in behavior between 5760x1080 and 1920x1080 (because the lock indicator's starting position is based on absolute distance, so it'll be appearing in roughly the same spot on a triplewide setup as on a single monitor). The fact that it's causing over-correction upon changes in yaw means there's some piece of code (which I am unable to find) that changes the lock indicator's position as your ship rotates, and changes it in a way that's affected by the aspect ratio (since a quick test at 1920x480 shows the same issue).

Italianmoose

2015-02-09 11:22

reporter   ~0016488

I'll run tests this evening on normal aspect ratios to see if tracking speed is affected. If it is affected by resolution then it might just be two separate issues, one of which will get worse as resolutions get higher and higher!

Are there any other bits of the HUD that aren't locked to a position? It would be a way of isolating where the problem comes from.

Italianmoose

2015-02-09 19:38

reporter   ~0016489

I think it is indeed two issues. I tested different resolutions and got these results:

Times in seconds
640x480 1920x1080 3840x2160
2.3 3.1 5.9
2.3 3.0 5.7
2.2 3.0 5.8
2.2 3.1 5.3
2.3 3.0 5.8
2.2 5.8

Next I'll test weird aspect ratios.

niffiwan

2015-02-09 20:24

developer   ~0016490

Just wondering, which missile are you testing with, or rather, I'm interested in knowing the tabled lock time to see which resolution "is native".

Italianmoose

2015-02-09 20:25

reporter   ~0016491

Sorry, forgot to mention. The Harpoon.

MageKing17

2015-02-09 20:39

developer  

hudlock.cpp.patch (1,037 bytes)   
Index: code/hud/hudlock.cpp
===================================================================
--- code/hud/hudlock.cpp	(revision 11242)
+++ code/hud/hudlock.cpp	(working copy)
@@ -1155,8 +1155,10 @@
 	if ( lock_local_pos.xyz.z > 0.0f ) {
 		// Get the location of our target in the "virtual frame" where the locking computation will be done
 		float w = 1.0f / lock_local_pos.xyz.z;
-		float sx = ((gr_screen.clip_center_x*2.0f) + (lock_local_pos.xyz.x*(gr_screen.clip_center_x*2.0f)*w))*0.5f;
-		float sy = ((gr_screen.clip_center_y*2.0f) - (lock_local_pos.xyz.y*(gr_screen.clip_center_y*2.0f)*w))*0.5f;
+		// Let's force our "virtual frame" to be square, to avoid extreme aspect ratios resulting in overcorrection. -MageKing17
+		float scale = MIN(gr_screen.clip_center_x, gr_screen.clip_center_y);
+		float sx = gr_screen.clip_center_x + (lock_local_pos.xyz.x * scale * w);
+		float sy = gr_screen.clip_center_y - (lock_local_pos.xyz.y * scale * w);
 
 		Player->current_target_sx = (int)sx;
 		Player->current_target_sy = (int)sy;
hudlock.cpp.patch (1,037 bytes)   

MageKing17

2015-02-09 20:39

developer   ~0016492

I think I've tracked down the cause of the overcorrection due to extreme aspect ratios; I've attached a patch that might fix the issue, but I don't have time to test it at this exact moment; I made sure it compiles, but I have to run out the door right now. Could somebody give it a try and see if it fixes at least that part of the problem?

Italianmoose

2015-02-09 21:24

reporter   ~0016493

Works beautifully! Thank you :)

MageKing17

2015-02-11 16:50

developer   ~0016495

Upon further reflection, both the aspect ratio problem and the different lock time problem should be solvable simultaneously by making the "virtual frame" simply be 640x480. New patch attached.

Italianmoose

2015-02-11 20:36

reporter   ~0016496

New patch, same testing method as before:

1024x768: 2.2, 2.2, 2.5, 2.3, 2.5, 2.2
1920x1080: 2.4, 2.2, 2.5, 2.3, 2.3, 2.4
5760x1080: 2.4, 2.5, 2.4, 2.5, 2.3, 2.3

3.7.2 RC5 (i.e. no fixes whatsoever):

1024x768: 2.5, 2.5, 2.4, 2.5, 2.1, 2.5
1920x1080: 3.2, 3.3, 3.3, 3.2, 3.3, 3.2
5760x1080: 8.7, 8.6, 8.7, 8.6, 8.7, 8.5

MageKing17

2015-04-18 19:33

developer   ~0016645

New version of the patch uploaded that attempts HUD scaling. Could use people testing this to see if it feels right. Might also want to compare with 640x480 without the patch, to see if it behaves the same way.

Italianmoose

2015-04-19 17:48

reporter   ~0016646

New new patch seems to be fine. No significant difference between resolutions :)

Not sure what I'm looking for with the HUD scaling - being a bit dense this evening!

MageKing17

2015-04-19 19:16

developer   ~0016647

"Not sure what I'm looking for with the HUD scaling - being a bit dense this evening!"

When using a high-resolution HUD (or the default HUD with scaling turned off, like the -hdg.tbm included with the 2014 MediaVPs), the original patch would result in the lock indicator only traversing a small portion of the screen, because it was based on absolute number of pixels and part of the point of the patch was to cap the number of those pixels (so as to avoid changes in lock time due to resolution). With the new version of the patch, when drawing the HUD gauge, it adds a scaling multiplier based on the values it used to use for the size of the virtual frame in order to allow the lock indicator to traverse as much of the screen as it used to (and just doing it faster, basically).

Yarn

2015-04-20 00:02

developer   ~0016648

Last edited: 2015-04-20 00:11

In both 3140.patch and 3140v2.patch, if the target isn't close to the center, the lock triangle starts too far from the target, causing the lock to take too long.

(Because 3140.patch lacks scaling, the effect isn't very noticeable visually there unless the resolution is below 1024x768, but it still happens.)


EDIT: I figured out why this happens: In your change to hud_lock_determine_lock_point(), you need to divide VIRTUAL_FRAME_WIDTH and VIRTUAL_FRAME_HEIGHT by 2.

MageKing17

2015-04-20 03:34

developer   ~0016649

"In your change to hud_lock_determine_lock_point(), you need to divide VIRTUAL_FRAME_WIDTH and VIRTUAL_FRAME_HEIGHT by 2."

D'oh! I optimized the calculations to avoid the double multiplication by 2 followed by a multiplication by 0.5, only to forget that I was no longer using half the width.

I think it would be simpler to change it to use a half-width #define...

MageKing17

2015-04-20 03:39

developer  

3140.patch (1,184 bytes)   
Index: code/hud/hudlock.cpp
===================================================================
--- code/hud/hudlock.cpp	(revision 11315)
+++ code/hud/hudlock.cpp	(working copy)
@@ -29,6 +29,11 @@
 #include "debugconsole/console.h"
 
 
+// Used for aspect locks. -MageKing17
+#define VIRTUAL_FRAME_HALF_WIDTH	320.0f
+#define VIRTUAL_FRAME_HALF_HEIGHT	240.0f
+
+
 vec3d lock_world_pos;
 
 static float Lock_start_dist;
@@ -1155,8 +1160,9 @@
 	if ( lock_local_pos.xyz.z > 0.0f ) {
 		// Get the location of our target in the "virtual frame" where the locking computation will be done
 		float w = 1.0f / lock_local_pos.xyz.z;
-		float sx = ((gr_screen.clip_center_x*2.0f) + (lock_local_pos.xyz.x*(gr_screen.clip_center_x*2.0f)*w))*0.5f;
-		float sy = ((gr_screen.clip_center_y*2.0f) - (lock_local_pos.xyz.y*(gr_screen.clip_center_y*2.0f)*w))*0.5f;
+		// Let's force our "virtual frame" to be 640x480. -MageKing17
+		float sx = gr_screen.clip_center_x + (lock_local_pos.xyz.x * VIRTUAL_FRAME_HALF_WIDTH * w);
+		float sy = gr_screen.clip_center_y - (lock_local_pos.xyz.y * VIRTUAL_FRAME_HALF_HEIGHT * w);
 
 		Player->current_target_sx = (int)sx;
 		Player->current_target_sy = (int)sy;
3140.patch (1,184 bytes)   

MageKing17

2015-04-20 03:39

developer  

3140v2.patch (2,036 bytes)   
Index: code/hud/hudlock.cpp
===================================================================
--- code/hud/hudlock.cpp	(revision 11315)
+++ code/hud/hudlock.cpp	(working copy)
@@ -29,6 +29,11 @@
 #include "debugconsole/console.h"
 
 
+// Used for aspect locks. -MageKing17
+#define VIRTUAL_FRAME_HALF_WIDTH	320.0f
+#define VIRTUAL_FRAME_HALF_HEIGHT	240.0f
+
+
 vec3d lock_world_pos;
 
 static float Lock_start_dist;
@@ -262,8 +267,9 @@
 		// show the rotating triangles if target is locked
 		renderLockTriangles(sx, sy, frametime);
 	} else {
-		sx = fl2i(lock_point.screen.xyw.x) - (Player->current_target_sx - Players[Player_num].lock_indicator_x); 
-		sy = fl2i(lock_point.screen.xyw.y) - (Player->current_target_sy - Players[Player_num].lock_indicator_y);
+		const float scaling_factor = (gr_screen.clip_center_x < gr_screen.clip_center_y) ? (gr_screen.clip_center_x / VIRTUAL_FRAME_HALF_WIDTH) : (gr_screen.clip_center_y / VIRTUAL_FRAME_HALF_HEIGHT);
+		sx = fl2i(lock_point.screen.xyw.x) - fl2i(i2fl(Player->current_target_sx - Players[Player_num].lock_indicator_x) * scaling_factor);
+		sy = fl2i(lock_point.screen.xyw.y) - fl2i(i2fl(Player->current_target_sy - Players[Player_num].lock_indicator_y) * scaling_factor);
 		gr_unsize_screen_pos(&sx, &sy);
 	}
 
@@ -1155,8 +1161,9 @@
 	if ( lock_local_pos.xyz.z > 0.0f ) {
 		// Get the location of our target in the "virtual frame" where the locking computation will be done
 		float w = 1.0f / lock_local_pos.xyz.z;
-		float sx = ((gr_screen.clip_center_x*2.0f) + (lock_local_pos.xyz.x*(gr_screen.clip_center_x*2.0f)*w))*0.5f;
-		float sy = ((gr_screen.clip_center_y*2.0f) - (lock_local_pos.xyz.y*(gr_screen.clip_center_y*2.0f)*w))*0.5f;
+		// Let's force our "virtual frame" to be 640x480. -MageKing17
+		float sx = gr_screen.clip_center_x + (lock_local_pos.xyz.x * VIRTUAL_FRAME_HALF_WIDTH * w);
+		float sy = gr_screen.clip_center_y - (lock_local_pos.xyz.y * VIRTUAL_FRAME_HALF_HEIGHT * w);
 
 		Player->current_target_sx = (int)sx;
 		Player->current_target_sy = (int)sy;
3140v2.patch (2,036 bytes)   

Goober5000

2015-04-22 05:34

administrator   ~0016650

It would be good to get this resolved for 3.7.2 if possible.

chief1983

2015-04-24 21:53

administrator   ~0016666

Is this affected by the patch in http://www.hard-light.net/forums/index.php?topic=89262.0 ? That seemed to have good reviews and might be a candidate for inclusion soon.

MageKing17

2015-04-24 22:36

developer   ~0016667

Code-wise, they don't interact at all (that patch doesn't change hudlock.cpp). Behavior-wise, I don't know; somebody with a triplewide setup would need to test and see if this patch still works as expected in combination with that patch (but theoretically it should).

Italianmoose

2015-04-24 22:46

reporter   ~0016668

I ran with both patches enabled before 3_7_2 final, and it worked a treat. I may not have much time over the next month to pursue further testing, however.

MageKing17

2015-04-24 23:09

developer   ~0016669

Italianmoose, was that before or after the April 19th update?

Italianmoose

2015-04-25 07:58

reporter   ~0016670

Before

Italianmoose

2015-04-25 21:19

reporter   ~0016672

Just rebuilt using updated code, and the two patches behave beautifully :)

chief1983

2015-05-04 19:38

administrator   ~0016691

Since these patches both well together, should we get a pull request together for them?

MageKing17

2015-05-05 00:11

developer   ~0016694

Last edited: 2015-05-05 20:06

I'm not fond of the second hunk, but I can't really think of a better way to write it. I guess I'll just make the pull request and if somebody else comes up with a better way to do it later, we'll fix it then.

And pull request: https://github.com/scp-fs2open/fs2open.github.com/pull/73

chief1983

2015-05-06 14:55

administrator   ~0016697

Resolved in PR#73 https://github.com/scp-fs2open/fs2open.github.com/pull/73

Issue History

Date Modified Username Field Change
2015-02-06 15:23 Italianmoose New Issue
2015-02-06 18:52 Italianmoose Note Added: 0016473
2015-02-06 20:01 Italianmoose Note Added: 0016474
2015-02-07 12:25 Italianmoose Note Edited: 0016474
2015-02-09 02:39 niffiwan Note Added: 0016481
2015-02-09 05:12 MageKing17 Note Added: 0016484
2015-02-09 11:22 Italianmoose Note Added: 0016488
2015-02-09 19:38 Italianmoose Note Added: 0016489
2015-02-09 20:24 niffiwan Note Added: 0016490
2015-02-09 20:25 Italianmoose Note Added: 0016491
2015-02-09 20:39 MageKing17 File Added: hudlock.cpp.patch
2015-02-09 20:39 MageKing17 Note Added: 0016492
2015-02-09 20:39 MageKing17 Assigned To => MageKing17
2015-02-09 20:39 MageKing17 Status new => code review
2015-02-09 21:24 Italianmoose Note Added: 0016493
2015-02-11 16:49 MageKing17 File Added: 3140.patch
2015-02-11 16:50 MageKing17 Note Added: 0016495
2015-02-11 20:36 Italianmoose Note Added: 0016496
2015-04-18 19:32 MageKing17 File Added: 3140v2.patch
2015-04-18 19:33 MageKing17 Note Added: 0016645
2015-04-19 17:48 Italianmoose Note Added: 0016646
2015-04-19 19:16 MageKing17 Note Added: 0016647
2015-04-20 00:02 Yarn Note Added: 0016648
2015-04-20 00:11 Yarn Note Edited: 0016648
2015-04-20 03:34 MageKing17 Note Added: 0016649
2015-04-20 03:39 MageKing17 File Deleted: 3140.patch
2015-04-20 03:39 MageKing17 File Deleted: 3140v2.patch
2015-04-20 03:39 MageKing17 File Added: 3140.patch
2015-04-20 03:39 MageKing17 File Added: 3140v2.patch
2015-04-22 05:34 Goober5000 Note Added: 0016650
2015-04-24 21:53 chief1983 Note Added: 0016666
2015-04-24 22:36 MageKing17 Note Added: 0016667
2015-04-24 22:46 Italianmoose Note Added: 0016668
2015-04-24 23:09 MageKing17 Note Added: 0016669
2015-04-25 07:58 Italianmoose Note Added: 0016670
2015-04-25 21:19 Italianmoose Note Added: 0016672
2015-05-04 19:38 chief1983 Note Added: 0016691
2015-05-05 00:11 MageKing17 Note Added: 0016694
2015-05-05 20:06 MageKing17 Note Edited: 0016694
2015-05-06 14:55 chief1983 Note Added: 0016697
2015-05-06 14:55 chief1983 Status code review => resolved
2015-05-06 14:55 chief1983 Resolution open => fixed
2015-05-06 14:55 chief1983 Fixed in Version => 3.7.3
2015-05-06 14:55 chief1983 Target Version => 3.7.4