View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0003070 | FSSCP | HUD | public | 2014-06-30 04:55 | 2014-07-01 16:49 |
| Reporter | Axem | Assigned To | Yarn | ||
| Priority | normal | Severity | major | Reproducibility | always |
| Status | resolved | Resolution | fixed | ||
| Target Version | 3.7.2 | ||||
| Summary | 0003070: HUD bracket problems when you fly too close to a targeted ship | ||||
| Description | If you fly very close to a targeted ship, your targeting brackets will move around the top left of the screen and be pretty useless. Especially bothersome since you can't lock missiles now! Also causes an Int3 if you run into the problem in a debug build. Investigation on IRC led to r10844 being the culprit. MageKing was doing some digging into it but last I saw he didn't find the exact cause yet. | ||||
| Tags | No tags attached. | ||||
|
|
There are a couple of "culprits" here. It looks like the targeting bracket code was kind of a mess beforehand, but r10844 changed the behavior to reveal what a mess it was. The direct cause of the problem is that gr_set_screen_scale() now changes gr_screen.clip_right_unscaled and gr_screen.clip_bottom_unscaled, which it did not used to do. This causes instances of trying to draw the offscreen target indicator with a nonzero HUD_offset_x or HUD_offset_y to encounter the unusual problem that their maximum x and y values are smaller than the width and height of the screen, so despite the gauge clearly trying to, say, draw alongside the bottom of the screen, it fails the "ypos >= gr_screen.clip_bottom_unscaled" conditional. The "actual" problem seems to be caused by the fact that Canvas_width and Canvas_height (used by g3_project_vertex()) are set based on gr_screen.clip_width and gr_screen.clip_height, which HudGauge::resetClip() is basically setting to (gr_screen.max_w - fl2i(HUD_offset_x)) and (gr_screen.max_h - fl2i(HUD_offset_y)), respectively. I'm not sure why projected vertices are limited to the clip_width and clip_height. Low-impact solution may be to have HudGaugeOffscreen::calculatePosition() add fl2i(HUD_offset_x) to xpos and fl2i(HUD_offset_y) to ypos right before the dreaded if-else branches (EDIT: In hindsight, that would just cause it to break on left/top instead of right/bottom). High-impact change would be to change g3_project_vertex() to not use Canvas_width or Canvas_height, or change how Canvas_width and Canvas_height are set by g3_start_frame_func(). Hard to guess what kind of unintentional problems could result from either of those changes, though. |
|
|
Assigning to Yarn to get his opinion because 10844 was his patch. |
|
|
The change mentioned in MageKing17's second paragraph was intentional; as far as I can tell, the unscaled clip boundaries are supposed to match the unscaled resolution (which, in the menus, was previously fixed at 640x480 and 1024x768). I will take a look at the stuff in the third paragraph, which I don't think I changed in revision 10844. Anyway, since this is a serious bug, I'm changing the severity to major and the target version to 3.7.2. |
|
|
hudtarget.cpp.patch (3,469 bytes)
Index: code/hud/hudtarget.cpp
===================================================================
--- code/hud/hudtarget.cpp (revision 10862)
+++ code/hud/hudtarget.cpp (working copy)
@@ -6300,7 +6300,6 @@
bool in_frame = g3_in_frame() > 0;
if(!in_frame)
g3_start_frame(0);
- gr_set_screen_scale(base_w, base_h);
// calculate the dot product between the players forward vector and the vector connecting
// the player to the target. Normalize targ_to_player since we want the dot product
@@ -6369,54 +6368,51 @@
xpos = eye_vertex->screen.xyw.x;
ypos = eye_vertex->screen.xyw.y;
- // we need it unsized here and it will be fixed when things are acutally drawn
- gr_unsize_screen_posf(&xpos, &ypos);
-
xpos = (xpos<1) ? 0 : xpos;
ypos = (ypos<1) ? 0 : ypos;
- if ( xpos >= gr_screen.clip_right_unscaled) {
- xpos = i2fl(gr_screen.clip_right_unscaled);
+ if ( xpos >= gr_screen.clip_right) {
+ xpos = i2fl(gr_screen.clip_right);
*dir = 0;
- if ( ypos < (half_gauge_length - gr_screen.clip_top_unscaled) )
+ if ( ypos < (half_gauge_length - gr_screen.clip_top) )
ypos = half_gauge_length;
- if ( ypos > (gr_screen.clip_bottom_unscaled - half_gauge_length) )
- ypos = gr_screen.clip_bottom_unscaled - half_gauge_length;
+ if ( ypos > (gr_screen.clip_bottom - half_gauge_length) )
+ ypos = gr_screen.clip_bottom - half_gauge_length;
- } else if ( xpos <= gr_screen.clip_left_unscaled ) {
- xpos = i2fl(gr_screen.clip_left_unscaled);
+ } else if ( xpos <= gr_screen.clip_left ) {
+ xpos = i2fl(gr_screen.clip_left);
*dir = 1;
- if ( ypos < (half_gauge_length - gr_screen.clip_top_unscaled) )
+ if ( ypos < (half_gauge_length - gr_screen.clip_top) )
ypos = half_gauge_length;
- if ( ypos > (gr_screen.clip_bottom_unscaled - half_gauge_length) )
- ypos = gr_screen.clip_bottom_unscaled - half_gauge_length;
+ if ( ypos > (gr_screen.clip_bottom - half_gauge_length) )
+ ypos = gr_screen.clip_bottom - half_gauge_length;
- } else if ( ypos <= gr_screen.clip_top_unscaled ) {
- ypos = i2fl(gr_screen.clip_top_unscaled);
+ } else if ( ypos <= gr_screen.clip_top ) {
+ ypos = i2fl(gr_screen.clip_top);
*dir = 2;
- if ( xpos < ( half_gauge_length - gr_screen.clip_left_unscaled) )
+ if ( xpos < ( half_gauge_length - gr_screen.clip_left) )
xpos = half_gauge_length;
- if ( xpos > (gr_screen.clip_right_unscaled - half_gauge_length) )
- xpos = gr_screen.clip_right_unscaled - half_gauge_length;
+ if ( xpos > (gr_screen.clip_right - half_gauge_length) )
+ xpos = gr_screen.clip_right - half_gauge_length;
- } else if ( ypos >= gr_screen.clip_bottom_unscaled ) {
- ypos = i2fl(gr_screen.clip_bottom_unscaled);
+ } else if ( ypos >= gr_screen.clip_bottom ) {
+ ypos = i2fl(gr_screen.clip_bottom);
*dir = 3;
- if ( xpos < ( half_gauge_length - gr_screen.clip_left_unscaled) )
+ if ( xpos < ( half_gauge_length - gr_screen.clip_left) )
xpos = half_gauge_length;
- if ( xpos > (gr_screen.clip_right_unscaled - half_gauge_length) )
- xpos = gr_screen.clip_right_unscaled - half_gauge_length;
+ if ( xpos > (gr_screen.clip_right - half_gauge_length) )
+ xpos = gr_screen.clip_right - half_gauge_length;
} else {
- Int3();
+ Assertion(false, "xpos and ypos failed to fall outside of the screen's clipping area; get a coder!\n");
return;
}
@@ -6446,8 +6442,6 @@
outcoords->y = ypos;
}
- gr_reset_screen_scale();
-
if(!in_frame)
g3_end_frame();
}
|
|
|
Okay, so after spending some time thinking about it, I've made a patch that seems to fix this issue (I played "Surrender, Belisarius", targeted Iota 1, and rammed into it a few times without triggering the assertion). Offscreen target indicators still seem to work. |
|
|
Actually, your patch breaks off-screen indicators if the HUD is scaled and the indicator is supposed to point right or down. (In case you don't know, you can try the scaled HUD by playing without the MediaVPs.) |
|
|
Argh; of course there was a reason the thing was using scaled coordinates in the first place. So, basically, my patch is reintroducing an old bug to fix a new bug. Fun. :/ I'm going to keep thinking about this, but if anybody else comes up with a fix, go right ahead. |
|
|
mantis3070.patch (2,685 bytes)
Index: code/freespace2/freespace.cpp
===================================================================
--- code/freespace2/freespace.cpp (revision 10864)
+++ code/freespace2/freespace.cpp (working copy)
@@ -1178,7 +1178,7 @@
if (Processing_filename[0] != '\0') {
gr_set_shader(&busy_shader);
- gr_shade(0, 0, gr_screen.clip_width_unscaled, 17, GR_RESIZE_MENU); // make sure it goes across the entire width
+ gr_shade(0, 0, gr_screen.max_w_unscaled, 17, GR_RESIZE_MENU); // make sure it goes across the entire width
gr_set_color_fast(&Color_white);
gr_string(5, 5, Processing_filename, GR_RESIZE_MENU);
Index: code/graphics/2d.cpp
===================================================================
--- code/graphics/2d.cpp (revision 10864)
+++ code/graphics/2d.cpp (working copy)
@@ -167,16 +167,6 @@
gr_screen.max_w_unscaled_zoomed = gr_screen.max_w_unscaled;
gr_screen.max_h_unscaled_zoomed = gr_screen.max_h_unscaled;
}
-
- gr_screen.offset_x_unscaled = 0;
- gr_screen.offset_y_unscaled = 0;
-
- gr_screen.clip_left_unscaled = 0;
- gr_screen.clip_top_unscaled = 0;
- gr_screen.clip_right_unscaled = w - 1;
- gr_screen.clip_bottom_unscaled = h - 1;
- gr_screen.clip_width_unscaled = w;
- gr_screen.clip_height_unscaled = h;
}
void gr_reset_screen_scale()
@@ -197,16 +187,6 @@
gr_screen.max_w_unscaled = gr_screen.max_w_unscaled_zoomed = (gr_screen.res == GR_1024) ? 1024 : 640;
gr_screen.max_h_unscaled = gr_screen.max_h_unscaled_zoomed = (gr_screen.res == GR_1024) ? 768 : 480;
-
- gr_screen.offset_x_unscaled = 0;
- gr_screen.offset_y_unscaled = 0;
-
- gr_screen.clip_left_unscaled = 0;
- gr_screen.clip_top_unscaled = 0;
- gr_screen.clip_right_unscaled = gr_screen.max_w_unscaled - 1;
- gr_screen.clip_bottom_unscaled = gr_screen.max_h_unscaled - 1;
- gr_screen.clip_width_unscaled = gr_screen.max_w_unscaled;
- gr_screen.clip_height_unscaled = gr_screen.max_h_unscaled;
}
/**
Index: code/menuui/mainhallmenu.cpp
===================================================================
--- code/menuui/mainhallmenu.cpp (revision 10864)
+++ code/menuui/mainhallmenu.cpp (working copy)
@@ -1613,7 +1613,7 @@
int shader_y = text_y - (Main_hall->tooltip_padding); // subtract more to pull higher
gr_set_shader(&Main_hall_tooltip_shader);
- gr_shade(0, shader_y, gr_screen.clip_width_unscaled, (gr_screen.clip_height_unscaled - shader_y), GR_RESIZE_MENU);
+ gr_shade(0, shader_y, gr_screen.max_w_unscaled, (gr_screen.max_h_unscaled - shader_y), GR_RESIZE_MENU);
gr_set_color_fast(&Color_bright_white);
gr_string((gr_screen.max_w_unscaled - w)/2, text_y, Main_hall->region_descript.at(text_index), GR_RESIZE_MENU);
|
|
|
I now think that I was mistaken in having gr_set_screen_scale() and gr_reset_screen_scale() reset the unscaled clip variables. Removing those lines appeared to fix this problem without introducing other problems; in fact, it also fixed a bug where message text would be rendered at (0, 0) instead of where the gauge was actually located, causing some or all of the text to be cut off. I've attached a patch that makes this change. I also noticed that the shaders for the main hall tooltips and the loading status (which appears in the mission loading screen in debug builds) use the unscaled clip bounds; the patch also changes those to use max_w_unscaled and max_h_unscaled instead. |
|
|
Oh wow. This is encouraging. :) Yarn, bug 0002362 also involves a scaling problem. Would you be willing to take a look at that? |
|
|
Yes, I'll take a look. |
|
|
Patch looks good; tested, seems to fix the issue while still working with scaled HUDs. My one nitpick would be that this leaves that unhelpful Int3(), but I can understand not wanting to touch more code than you need to. |
|
|
Fix committed to trunk@10868. |
|
fs2open: trunk r10868 2014-07-01 13:10 Ported: N/A Details Diff |
Fix Mantis 0003070, courtesy of Yarn. |
Affected Issues 0003070 |
|
| mod - /trunk/fs2_open/code/freespace2/freespace.cpp | Diff File | ||
| mod - /trunk/fs2_open/code/menuui/mainhallmenu.cpp | Diff File | ||
| mod - /trunk/fs2_open/code/graphics/2d.cpp | Diff File | ||
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2014-06-30 04:55 | Axem | New Issue | |
| 2014-06-30 05:52 | MageKing17 | Note Added: 0015958 | |
| 2014-06-30 15:41 | MjnMixael | Assigned To | => Yarn |
| 2014-06-30 15:41 | MjnMixael | Status | new => assigned |
| 2014-06-30 15:42 | MjnMixael | Note Added: 0015969 | |
| 2014-06-30 15:43 | MjnMixael | Note Edited: 0015969 | |
| 2014-06-30 15:49 | MageKing17 | Note Edited: 0015958 | |
| 2014-06-30 18:14 | Yarn | Note Added: 0015973 | |
| 2014-06-30 18:15 | Yarn | Severity | minor => major |
| 2014-06-30 18:15 | Yarn | Target Version | => 3.7.2 |
| 2014-07-01 00:05 | MageKing17 | File Added: hudtarget.cpp.patch | |
| 2014-07-01 00:05 | MageKing17 | Assigned To | Yarn => MageKing17 |
| 2014-07-01 00:07 | MageKing17 | Note Added: 0015979 | |
| 2014-07-01 00:07 | MageKing17 | Status | assigned => code review |
| 2014-07-01 00:32 | Yarn | Note Added: 0015982 | |
| 2014-07-01 01:19 | MageKing17 | Note Added: 0015985 | |
| 2014-07-01 01:19 | MageKing17 | Status | code review => feedback |
| 2014-07-01 01:57 | Yarn | File Added: mantis3070.patch | |
| 2014-07-01 01:58 | Yarn | Note Added: 0015987 | |
| 2014-07-01 02:02 | MageKing17 | Assigned To | MageKing17 => Yarn |
| 2014-07-01 02:02 | MageKing17 | Status | feedback => code review |
| 2014-07-01 02:03 | Goober5000 | Note Added: 0015988 | |
| 2014-07-01 02:29 | Yarn | Note Added: 0015991 | |
| 2014-07-01 02:29 | MageKing17 | Note Added: 0015992 | |
| 2014-07-01 16:49 | chief1983 | Changeset attached | => fs2open trunk r10868 |
| 2014-07-01 16:49 | chief1983 | Note Added: 0016006 | |
| 2014-07-01 16:49 | chief1983 | Status | code review => resolved |
| 2014-07-01 16:49 | chief1983 | Resolution | open => fixed |
