All Projects FSSCP  wxFRED

View Issue Details

Related ChangesetsJump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0003016FSSCPgraphicspublic2014-03-09 06:032014-08-17 17:34
ReporterYarn Assigned ToMageKing17  
PriorityhighSeveritymajorReproducibilityalways
Status resolvedResolutionfixed 
Platformx64OSWindows 7 
Product Version3.7.1 
Target Version3.7.2 
Summary0003016: Briefing window FOV is too wide
DescriptionThe FOV of the briefing window is much wider than it was in retail, resulting in icons being closer together than they should be. I can't tell when this bug was introduced; it happens as far back as 3.6.10, the oldest FSO release that I can find, so this is indeed an old bug.

In both FSO and retail, icon position is the same in both 640x480 and 1024x768. Furthermore, if the -nohtl flag is used, then the retail FOV is used.

I understand that a ton of missions and campaigns may be designed for this wider FOV, so fixing this issue may result in their briefings looking wrong (but at least they would still be playable).
Steps To ReproduceThis is best observed in the attached mission. In its briefing, there are four node icons. They should be close to the corners of the window, as they are in retail. They should not be near the center.
Additional InformationI have attached three screenshots that show how the attached mission's briefing looks in retail, normal FSO, and FSO with the -nohtl flag. Notice that FSO displays the icons close together, unlike retail, unless -nohtl is used.

I should mention that the ship models in the tech room exhibit a similar behavior regarding FOV.

In case it's important, I'm using an AMD Radeon 6870, Catalyst verion 13.12.
TagsNo tags attached.

Relationships

related to 0003096 resolvedYarn FOV discrepancy in briefing editor 

Activities

Yarn

2014-03-09 06:03

developer  

BriefingIconTest.fs2 (4,266 bytes)

Yarn

2014-03-09 06:03

developer  

briefing_fso.jpg (174,618 bytes)   
briefing_fso.jpg (174,618 bytes)   

Yarn

2014-03-09 06:03

developer  

briefing_fso_nohtl.jpg (144,342 bytes)   
briefing_fso_nohtl.jpg (144,342 bytes)   

Yarn

2014-03-09 06:03

developer  

briefing_retail.jpg (132,012 bytes)   
briefing_retail.jpg (132,012 bytes)   

Yarn

2014-03-22 07:50

developer  

mantis3016.patch (544 bytes)    
Index: code/mission/missionbriefcommon.cpp
===================================================================
--- code/mission/missionbriefcommon.cpp	(revision 10517)
+++ code/mission/missionbriefcommon.cpp	(working copy)
@@ -1155,7 +1155,7 @@
 	Assert(Briefing);
 
 	g3_start_frame(0);
-	g3_set_view_matrix(&Current_cam_pos, &Current_cam_orient, 0.5f);
+	g3_set_view_matrix(&Current_cam_pos, &Current_cam_orient, 0.29375f);
 
 	brief_maybe_create_new_grid(The_grid, &Current_cam_pos, &Current_cam_orient);
 	brief_render_grid(The_grid);
mantis3016.patch (544 bytes)   

Yarn

2014-03-22 07:51

developer   ~0015673

I think I know why this is happening: FSO without -nohtl uses a vertical FOV for 3D rendering (which is normally desirable), while FSO with -nohtl and retail FS2 use a horizontal FOV. This affects the briefing window as well as actual 3D rendering. Since the window itself is rather wide, this distinction results in the problem illustrated in this ticket.

Attached is a possible fix, which reduces the briefing FOV to match retail. Yes, it does affect the many custom missions and campaigns released over the past several years (as would any attempt to fix this), but I think it's necessary for proper retail compatibility (which, if I'm not mistaken, is one of the primary directives of the SCP). Perhaps a better, more sophisticated approach is possible.

niffiwan

2014-05-12 02:32

developer   ~0015741

Changing to code review to discuss the patch/ramifications of making this change.

Goober5000

2014-05-28 01:23

administrator   ~0015767

I am strongly in favor of using this patch. It's been long-running SCP policy that any change from retail is a regression bug.

However, it might not be a bad idea to add a Briefing FOV setting in game_settings.tbl. (Probably be good to copy the regular FOV configuration option into that table too.)

MjnMixael

2014-05-28 03:12

manager   ~0015768

Please use a game_settings.tbl flag...

The_E

2014-06-16 16:12

administrator   ~0015885

Fix committed to trunk@10824.

MageKing17

2014-06-18 14:21

developer   ~0015891

FRED2_Open doesn't respect the new FOV value. For some reason, fredrender.cpp looks like it's taking a const of 0.325, and multiplying it by 4/9ths pi... which gives ~0.45, which is pretty close to 0.5. Someone who can compile FRED should take another look.

(Points of interest in /code/fred2/fredrender.cpp: lines 70-71 (FOV definitions) and lines 143-144 (where FOV is used).)

The_E

2014-06-18 15:49

administrator   ~0015892

Fix committed to trunk@10825.

Yarn

2014-08-17 02:15

developer   ~0016222

Last edited: 2014-08-17 02:16

 I noticed that the icons and labels (but not the actual ships) in the briefing editor are still incorrectly positioned as before. To see what I mean, look at the newly attached screenshot, which was taken using the test mission that's attached here. Notice how the icons are not located at the corners where they should be, and how the Alpha 1 label is drawn close to the center rather than on top of the ship. This can make proper icon placement a nightmare. (This problem actually existed before revision 10824, although it wasn't this severe.)

I'd try to fix this myself, but I can't compile FRED.

Yarn

2014-08-17 02:18

developer  

BriefingEditorBug.png (12,782 bytes)   
BriefingEditorBug.png (12,782 bytes)   

MageKing17

2014-08-17 02:58

developer  

fredrender.cpp.patch (882 bytes)    
Index: code/fred2/fredrender.cpp
===================================================================
--- code/fred2/fredrender.cpp	(revision 10995)
+++ code/fred2/fredrender.cpp	(working copy)
@@ -1489,7 +1489,11 @@
 	gr_set_font(FONT1);
 	light_reset();
 
-	g3_set_view_matrix(&eye_pos, &eye_orient, 0.5f);
+	if (Briefing_dialog) {
+		g3_set_view_matrix(&eye_pos, &eye_orient, Briefing_window_FOV);
+	} else {
+		g3_set_view_matrix(&eye_pos, &eye_orient, 0.5f);
+	}
 	Viewer_pos = eye_pos;  // for starfield code
 	
 	fred_enable_htl();
@@ -1594,7 +1598,11 @@
 		gr_set_clip(0, 0, True_rw, True_rh);
 
 	g3_start_frame(0);	 // ** Accounted for
-	g3_set_view_matrix(&eye_pos, &eye_orient, 0.5f);
+	if (Briefing_dialog) {
+		g3_set_view_matrix(&eye_pos, &eye_orient, Briefing_window_FOV);
+	} else {
+		g3_set_view_matrix(&eye_pos, &eye_orient, 0.5f);
+	}
 }
 
 void game_do_frame()
fredrender.cpp.patch (882 bytes)   

MageKing17

2014-08-17 03:03

developer   ~0016223

Changing g3_set_view_matrix() to use Briefing_window_FOV instead of the hardcoded 0.5f makes the icons actually respect the FOV now. I'm unsure if the second g3_set_view_matrix() call is actually relevant to anything that would care about Briefing_window_FOV, but then, I'm unsure as to why render_frame() uses an "end the frame at the start of the function and start it at the end of the function" paradigm at all. Seems safer to just change both instances, just in case.

niffiwan

2014-08-17 04:10

developer   ~0016227

Fix committed to trunk@11001.

Related Changesets

fs2open: trunk r10824

2014-06-16 12:31

The_E


Ported: N/A

Details Diff		 Fix for Mantis 3016 (Briefing window FOV different from retail).
Also adds a new setting to game_settings.tbl, "$Briefing Window FOV:" under Graphics, which takes a single float argument to set the FOV of the briefing window.
 Affected Issues
0003016
mod - /trunk/fs2_open/code/mod_table/mod_table.cpp Diff File
mod - /trunk/fs2_open/code/mod_table/mod_table.h Diff File
mod - /trunk/fs2_open/code/mission/missionbriefcommon.cpp Diff File

fs2open: trunk r10825

2014-06-18 12:08

The_E


Ported: N/A

Details Diff		 Fix for Mantis 3016 part 2 (Make FRED respect the Briefing Window FOV setting in game_settings.tbl
 Affected Issues
0003016
mod - /trunk/fs2_open/code/fred2/management.cpp Diff File
mod - /trunk/fs2_open/code/fred2/fredrender.cpp Diff File

fs2open: trunk r11001

2014-08-17 00:39

niffiwan


Ported: N/A

Details Diff		 Followup fix for mantis 3016 (FRED Briefing window FOV)

Make FRED icons & labels respect the Briefing_window_FOV		 Affected Issues
0003016
mod - /trunk/fs2_open/code/fred2/fredrender.cpp Diff File

Issue History

Date Modified Username Field Change
2014-03-09 06:03 Yarn New Issue
2014-03-09 06:03 Yarn File Added: BriefingIconTest.fs2
2014-03-09 06:03 Yarn File Added: briefing_fso.jpg
2014-03-09 06:03 Yarn File Added: briefing_fso_nohtl.jpg
2014-03-09 06:03 Yarn File Added: briefing_retail.jpg
2014-03-22 07:50 Yarn File Added: mantis3016.patch
2014-03-22 07:51 Yarn Note Added: 0015673
2014-03-22 07:51 Yarn Description Updated
2014-05-12 02:32 niffiwan Note Added: 0015741
2014-05-12 02:32 niffiwan Assigned To => Yarn
2014-05-12 02:32 niffiwan Status new => code review
2014-05-28 01:23 Goober5000 Note Added: 0015767
2014-05-28 01:23 Goober5000 Priority normal => high
2014-05-28 01:23 Goober5000 Target Version => 3.7.2
2014-05-28 03:12 MjnMixael Note Added: 0015768
2014-06-16 16:12 The_E Changeset attached => fs2open trunk r10824
2014-06-16 16:12 The_E Note Added: 0015885
2014-06-16 16:12 The_E Status code review => resolved
2014-06-16 16:12 The_E Resolution open => fixed
2014-06-18 14:21 MageKing17 Assigned To Yarn => The_E
2014-06-18 14:21 MageKing17 Note Added: 0015891
2014-06-18 14:21 MageKing17 Status resolved => feedback
2014-06-18 14:21 MageKing17 Resolution fixed => reopened
2014-06-18 15:49 The_E Changeset attached => fs2open trunk r10825
2014-06-18 15:49 The_E Note Added: 0015892
2014-06-18 15:49 The_E Status feedback => resolved
2014-06-29 05:34 Goober5000 Assigned To The_E => Yarn
2014-06-29 16:42 MageKing17 Status resolved => feedback
2014-06-29 16:43 MageKing17 Status feedback => resolved
2014-06-29 16:43 MageKing17 Resolution reopened => fixed
2014-08-17 02:15 Yarn Assigned To Yarn =>
2014-08-17 02:15 Yarn Note Added: 0016222
2014-08-17 02:15 Yarn Status resolved => feedback
2014-08-17 02:15 Yarn Resolution fixed => reopened
2014-08-17 02:15 Yarn Status feedback => confirmed
2014-08-17 02:16 Yarn Note Edited: 0016222
2014-08-17 02:18 Yarn File Added: BriefingEditorBug.png
2014-08-17 02:19 Yarn Status confirmed => acknowledged
2014-08-17 02:21 Yarn Severity minor => major
2014-08-17 02:58 MageKing17 File Added: fredrender.cpp.patch
2014-08-17 02:59 MageKing17 Status acknowledged => code review
2014-08-17 02:59 MageKing17 Resolution reopened => open
2014-08-17 03:03 MageKing17 Note Added: 0016223
2014-08-17 04:07 niffiwan Assigned To => MageKing17
2014-08-17 04:07 niffiwan Status code review => assigned
2014-08-17 04:10 niffiwan Changeset attached => fs2open trunk r11001
2014-08-17 04:10 niffiwan Note Added: 0016227
2014-08-17 04:10 niffiwan Status assigned => resolved
2014-08-17 04:10 niffiwan Resolution open => fixed
2014-08-17 17:34 Yarn Relationship added related to 0003096