View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0002404 | FSSCP | tables | public | 2011-02-25 23:12 | 2014-11-21 17:56 |
Reporter | FUBAR-BDHR | Assigned To | The_E | ||
Priority | normal | Severity | feature | Reproducibility | always |
Status | resolved | Resolution | fixed | ||
Product Version | 3.6.13 | ||||
Target Version | 3.7.2 | ||||
Summary | 0002404: $Weapon Model Draw Distance: does nothing | ||||
Description | It's in the wiki, defined and parsed in ship.h and ship.cpp but never used anywhere. Basically it defaults it to 200 and stuffs the value if it exists in ships.tbl but that's it. If this is going to be implimented it would be nice to be able to tie it to the LOD system as well. So a value of 200 would be multiplied by the LOD factors (default .125 .25, 1, 4, 8). An alternate option would be specifing what LODs the models are visible on. | ||||
Tags | No tags attached. | ||||
|
Feature request. Targeted for 3.7.2. |
|
Added a draft patch for this. Please review. |
|
So if I'm reading this correctly it will take whatever you have tabled or the default of 200 and scale it by the defined detail distances and not draw if beyond that. So using retail defaults it would draw weapons out to 25, 50, 200, 800, 1600 based on your detail setting. I see 2 possible issues here. First is it appears to only work for primaries unless that info is set somewhere else. When I found this what I was trying to do was cut off rendering of models displayed with show secondaries at distances where they really couldn't be seen attached to the ship anymore. Second is do we really want to change the behavior if this value isn't specified? I don't have a problem with it but I can see where others might. Maybe it would be better to just fall back to the old behavior if the entry does not exist in the table. Possibly change the default to -1 (it appears the 200 was never used) and if the value is -1 just skip that check and render as it used to. |
|
2404.patch (5,727 bytes)
Index: code/model/model.h =================================================================== --- code/model/model.h (revision 9461) +++ code/model/model.h (working copy) @@ -824,6 +824,7 @@ #define MR_FULL_DETAIL (1<<28) // render all valid objects, particularly ones that are otherwise in/out of render boxes - taylor #define MR_FORCE_CLAMP (1<<29) // force clamp - Hery #define MR_ANIMATED_SHADER (1<<30) // Use a animated Shader - Valathil +#define MR_ATTACHED_MODEL (1<<31) // Used for attached weapon model lodding // Renders a model and all it's submodels. // See MR_? defines for values for flags Index: code/model/modelinterp.cpp =================================================================== --- code/model/modelinterp.cpp (revision 9461) +++ code/model/modelinterp.cpp (working copy) @@ -1993,7 +1993,10 @@ cull = gr_set_cull(0); } - Interp_objnum = objnum; + if (flags & MR_ATTACHED_MODEL) + Interp_objnum = -1; + else + Interp_objnum = objnum; if ( flags & MR_NO_LIGHTING ) { Interp_light = 1.0f; @@ -2684,9 +2687,11 @@ bool draw_thrusters = false; // just to be on the safe side - Assert( Interp_objnum == objnum ); + // If we're dealing with an attached model (i.e. an external weapon model) we need the object number + // of the ship that the weapon is attached to, but nothing else + Assert((objnum == Interp_objnum && !(flags & MR_ATTACHED_MODEL))); - if (objnum >= 0) { + if (objnum >= 0 && !(flags & MR_ATTACHED_MODEL)) { objp = &Objects[objnum]; if (objp->type == OBJ_SHIP) { @@ -2774,34 +2779,40 @@ vec3d closest_pos; float depth = model_find_closest_point( &closest_pos, model_num, -1, orient, pos, &Eye_position ); + + #if MAX_DETAIL_LEVEL != 4 + #error Code in modelInterp.cpp assumes MAX_DETAIL_LEVEL == 4 + #endif + + switch( Detail.detail_distance ) { + case 0: // lowest + depth /= The_mission.ai_profile->detail_distance_mult[0]; + break; + case 1: // lower than normal + depth /= The_mission.ai_profile->detail_distance_mult[1]; + break; + case 2: // default + depth /= The_mission.ai_profile->detail_distance_mult[2]; + break; + case 3: // above normal + depth /= The_mission.ai_profile->detail_distance_mult[3]; + break; + case 4: // even more normal + depth /= The_mission.ai_profile->detail_distance_mult[4]; + break; + } + + // If we're rendering attached weapon models, check against the ships' tabled Weapon Model Draw Distance (which defaults to 200) + if (Interp_flags & MR_ATTACHED_MODEL) { + if (depth > Ship_info[Ships[Objects[objnum].instance].ship_info_index].weapon_model_draw_distance) + return; + } + if ( pm->n_detail_levels > 1 ) { if ( Interp_flags & MR_LOCK_DETAIL ) { i = Interp_detail_level_locked+1; } else { - - #if MAX_DETAIL_LEVEL != 4 - #error Code in modelInterp.cpp assumes MAX_DETAIL_LEVEL == 4 - #endif - - switch( Detail.detail_distance ) { - case 0: // lowest - depth /= The_mission.ai_profile->detail_distance_mult[0]; - break; - case 1: // lower than normal - depth /= The_mission.ai_profile->detail_distance_mult[1]; - break; - case 2: // default - depth /= The_mission.ai_profile->detail_distance_mult[2]; - break; - case 3: // above normal - depth /= The_mission.ai_profile->detail_distance_mult[3]; - break; - case 4: // even more normal - depth /= The_mission.ai_profile->detail_distance_mult[4]; - break; - } - // nebula ? if(The_mission.flags & MISSION_FLAG_FULLNEB){ depth *= neb2_get_lod_scale(Interp_objnum); Index: code/ship/ship.cpp =================================================================== --- code/ship/ship.cpp (revision 9461) +++ code/ship/ship.cpp (working copy) @@ -6127,6 +6127,7 @@ int save_flags = render_flags; render_flags &= ~MR_SHOW_THRUSTERS; + render_flags |= MR_ATTACHED_MODEL; //primary weapons for (i = 0; i < swp->num_primary_banks; i++) { @@ -6137,7 +6138,7 @@ for(k = 0; k < bank->num_slots; k++) { polymodel* pm = model_get(Weapon_info[swp->primary_bank_weapons[i]].external_model_num); pm->gun_submodel_rotation = shipp->primary_rotate_ang[i]; - model_render(Weapon_info[swp->primary_bank_weapons[i]].external_model_num, &vmd_identity_matrix, &bank->pnt[k], render_flags); + model_render(Weapon_info[swp->primary_bank_weapons[i]].external_model_num, &vmd_identity_matrix, &bank->pnt[k], render_flags, shipp->objnum); pm->gun_submodel_rotation = 0.0f; } } @@ -6155,7 +6156,7 @@ if (Weapon_info[swp->secondary_bank_weapons[i]].wi_flags2 & WIF2_EXTERNAL_WEAPON_LNCH) { for(k = 0; k < bank->num_slots; k++) { - model_render(Weapon_info[swp->secondary_bank_weapons[i]].external_model_num, &vmd_identity_matrix, &bank->pnt[k], render_flags); + model_render(Weapon_info[swp->secondary_bank_weapons[i]].external_model_num, &vmd_identity_matrix, &bank->pnt[k], render_flags, shipp->objnum); } } else { num_secondaries_rendered = 0; @@ -6174,7 +6175,7 @@ vm_vec_scale_add2(&secondary_weapon_pos, &vmd_z_vector, -(1.0f-shipp->secondary_point_reload_pct[i][k]) * model_get(Weapon_info[swp->secondary_bank_weapons[i]].external_model_num)->rad); - model_render(Weapon_info[swp->secondary_bank_weapons[i]].external_model_num, &vmd_identity_matrix, &secondary_weapon_pos, render_flags); + model_render(Weapon_info[swp->secondary_bank_weapons[i]].external_model_num, &vmd_identity_matrix, &secondary_weapon_pos, render_flags, shipp->objnum); } } } |
|
I've attached a new patch that should work for secondaries as well. As for the other issue you've raised, I believe that some form of lodding for external weapon models is a good thing to have. If the default distance value is too small, well, then we can discuss raising it, but I am of the opinion that _some_ value should be there. Rendering weapon models when they're very definitely not going to be seen is a bad idea in my opinion. |
|
I tried running a test with Diaspora (which I know displays external weapons by default) and the patch causes a crash when entering the Techroom after pilot selection. It seems that The_mission.ai_profile is a NULL pointer at this stage? Without the patch, this code prevents the case statement from being executed: if ( Interp_flags & MR_LOCK_DETAIL ) { i = Interp_detail_level_locked+1; } else { |
|
Surrounding the crashing code with if (!(Interp_flags & MR_LOCK_DETAIL)) { fixes the crash. Given that that flag says that detail scaling should not be done it seems like a good solution to this problem. |
|
Confirmed that change fixed the techroom crash. However, when trying to play Diaspora "The Strike" I hit this assert just after the Theseus jumps in: Assert((objnum == Interp_objnum && !(flags & MR_ATTACHED_MODEL))); Changing it to this (which I think matches what the comment says) lets me proceed past that point. Assert(objnum == Interp_objnum || (flags & MR_ATTACHED_MODEL)); However, I then start hitting other asserts within 30 seconds of mission start, while trying to get a look at the ships external missile models from different distances. ASSERTION FAILED: "instance_depth<MAX_INSTANCE_DEPTH" at render/3dsetup.cpp:256 or ASSERTION FAILED: "GL_modelview_matrix_depth == 2" at graphics/gropengltnl.cpp:1727 (FYI, I applied the patch to r9461 from SVN; I haven't updated it for current trunk yet) |
|
2404_r11171.patch (5,673 bytes)
Index: code/model/model.h =================================================================== --- code/model/model.h (revision 11170) +++ code/model/model.h (working copy) @@ -842,6 +842,7 @@ #define MR_FULL_DETAIL (1<<28) // render all valid objects, particularly ones that are otherwise in/out of render boxes - taylor #define MR_FORCE_CLAMP (1<<29) // force clamp - Hery #define MR_ANIMATED_SHADER (1<<30) // Use a animated Shader - Valathil +#define MR_ATTACHED_MODEL (1<<31) // Used for attached weapon model lodding // Renders a model and all it's submodels. // See MR_? defines for values for flags Index: code/model/modelinterp.cpp =================================================================== --- code/model/modelinterp.cpp (revision 11170) +++ code/model/modelinterp.cpp (working copy) @@ -2003,7 +2003,10 @@ } - Interp_objnum = objnum; + if (flags & MR_ATTACHED_MODEL) + Interp_objnum = -1; + else + Interp_objnum = objnum; if ( flags & MR_NO_LIGHTING ) { Interp_light = 1.0f; @@ -2730,9 +2733,11 @@ bool draw_thrusters = false; // just to be on the safe side - Assert( Interp_objnum == objnum ); + // If we're dealing with an attached model (i.e. an external weapon model) we need the object number + // of the ship that the weapon is attached to, but nothing else + Assert( (Interp_objnum == objnum) || (flags & MR_ATTACHED_MODEL) ); - if (objnum >= 0) { + if (objnum >= 0 && !(flags & MR_ATTACHED_MODEL)) { objp = &Objects[objnum]; if (objp->type == OBJ_SHIP) { @@ -2820,34 +2825,43 @@ vec3d closest_pos; float depth = model_find_closest_point( &closest_pos, model_num, -1, orient, pos, &Eye_position ); + + if ( !(Interp_flags & MR_LOCK_DETAIL) ) { + #if MAX_DETAIL_LEVEL != 4 + #error Code in modelInterp.cpp assumes MAX_DETAIL_LEVEL == 4 + #endif + + switch( Detail.detail_distance ) { + case 0: // lowest + depth /= The_mission.ai_profile->detail_distance_mult[0]; + break; + case 1: // lower than normal + depth /= The_mission.ai_profile->detail_distance_mult[1]; + break; + case 2: // default + depth /= The_mission.ai_profile->detail_distance_mult[2]; + break; + case 3: // above normal + depth /= The_mission.ai_profile->detail_distance_mult[3]; + break; + case 4: // even more normal + depth /= The_mission.ai_profile->detail_distance_mult[4]; + break; + } + + // If we're rendering attached weapon models, check against the ships' tabled Weapon Model Draw Distance (which defaults to 200) + if (Interp_flags & MR_ATTACHED_MODEL) { + if (depth > Ship_info[Ships[Objects[objnum].instance].ship_info_index].weapon_model_draw_distance) { + g3_done_instance(use_api); + return; + } + } + } if ( pm->n_detail_levels > 1 ) { if ( Interp_flags & MR_LOCK_DETAIL ) { i = Interp_detail_level_locked+1; } else { - - #if MAX_DETAIL_LEVEL != 4 - #error Code in modelInterp.cpp assumes MAX_DETAIL_LEVEL == 4 - #endif - - switch( Detail.detail_distance ) { - case 0: // lowest - depth /= The_mission.ai_profile->detail_distance_mult[0]; - break; - case 1: // lower than normal - depth /= The_mission.ai_profile->detail_distance_mult[1]; - break; - case 2: // default - depth /= The_mission.ai_profile->detail_distance_mult[2]; - break; - case 3: // above normal - depth /= The_mission.ai_profile->detail_distance_mult[3]; - break; - case 4: // even more normal - depth /= The_mission.ai_profile->detail_distance_mult[4]; - break; - } - // nebula ? if(The_mission.flags & MISSION_FLAG_FULLNEB){ depth *= neb2_get_lod_scale(Interp_objnum); Index: code/ship/ship.cpp =================================================================== --- code/ship/ship.cpp (revision 11170) +++ code/ship/ship.cpp (working copy) @@ -6309,6 +6309,7 @@ int save_flags = render_flags; render_flags &= ~MR_SHOW_THRUSTERS; + render_flags |= MR_ATTACHED_MODEL; //primary weapons for (i = 0; i < swp->num_primary_banks; i++) { @@ -6319,7 +6320,7 @@ for(k = 0; k < bank->num_slots; k++) { polymodel* pm = model_get(Weapon_info[swp->primary_bank_weapons[i]].external_model_num); pm->gun_submodel_rotation = shipp->primary_rotate_ang[i]; - model_render(Weapon_info[swp->primary_bank_weapons[i]].external_model_num, &vmd_identity_matrix, &bank->pnt[k], render_flags); + model_render(Weapon_info[swp->primary_bank_weapons[i]].external_model_num, &vmd_identity_matrix, &bank->pnt[k], render_flags, shipp->objnum); pm->gun_submodel_rotation = 0.0f; } } @@ -6337,7 +6338,7 @@ if (Weapon_info[swp->secondary_bank_weapons[i]].wi_flags2 & WIF2_EXTERNAL_WEAPON_LNCH) { for(k = 0; k < bank->num_slots; k++) { - model_render(Weapon_info[swp->secondary_bank_weapons[i]].external_model_num, &vmd_identity_matrix, &bank->pnt[k], render_flags); + model_render(Weapon_info[swp->secondary_bank_weapons[i]].external_model_num, &vmd_identity_matrix, &bank->pnt[k], render_flags, shipp->objnum); } } else { num_secondaries_rendered = 0; @@ -6356,7 +6357,7 @@ vm_vec_scale_add2(&secondary_weapon_pos, &vmd_z_vector, -(1.0f-shipp->secondary_point_reload_pct[i][k]) * model_get(Weapon_info[swp->secondary_bank_weapons[i]].external_model_num)->rad); - model_render(Weapon_info[swp->secondary_bank_weapons[i]].external_model_num, &vmd_identity_matrix, &secondary_weapon_pos, render_flags); + model_render(Weapon_info[swp->secondary_bank_weapons[i]].external_model_num, &vmd_identity_matrix, &secondary_weapon_pos, render_flags, shipp->objnum); } } } |
|
I couldn't get the second assertion to trigger, but then, I tried this with latest trunk. The first assertion was caused by the return skipping a g3_done_instance() call; I flew around in "The Strike" with an executable made from the attached patch for a few minutes shooting raiders without triggering any assertions. |
|
Fix committed to trunk@11178. |
fs2open: trunk r11178 2014-11-21 13:42 Ported: N/A Details Diff |
From The_E, niffiwan, m_m, and myself: Fix Mantis 2404. Makes "$Weapon Model Draw Distance:" actually stop external weapon models from being drawn beyond that distance (modified by detail level). |
Affected Issues 0002404 |
|
mod - /trunk/fs2_open/code/model/modelinterp.cpp | Diff File | ||
mod - /trunk/fs2_open/code/model/model.h | Diff File | ||
mod - /trunk/fs2_open/code/ship/ship.cpp | Diff File |
Date Modified | Username | Field | Change |
---|---|---|---|
2011-02-25 23:12 | FUBAR-BDHR | New Issue | |
2012-12-20 06:48 | MjnMixael | Note Added: 0014551 | |
2012-12-20 06:48 | MjnMixael | Severity | minor => feature |
2012-12-20 06:48 | MjnMixael | Target Version | => 3.7.2 |
2012-12-21 11:10 | The_E | File Added: 2404.patch | |
2012-12-21 11:11 | The_E | Note Added: 0014555 | |
2012-12-21 11:11 | The_E | Assigned To | => The_E |
2012-12-21 11:11 | The_E | Status | new => code review |
2012-12-24 06:37 | FUBAR-BDHR | Note Added: 0014574 | |
2012-12-24 09:50 | The_E | File Deleted: 2404.patch | |
2012-12-24 09:50 | The_E | File Added: 2404.patch | |
2012-12-24 09:53 | The_E | Note Added: 0014575 | |
2014-07-27 10:10 | niffiwan | Note Added: 0016138 | |
2014-07-30 10:39 | m_m | Note Added: 0016149 | |
2014-08-21 10:14 | niffiwan | Note Added: 0016246 | |
2014-11-12 01:55 | MageKing17 | File Added: 2404_r11171.patch | |
2014-11-12 01:57 | MageKing17 | Note Added: 0016382 | |
2014-11-21 17:56 | MageKing17 | Changeset attached | => fs2open trunk r11178 |
2014-11-21 17:56 | MageKing17 | Note Added: 0016388 | |
2014-11-21 17:56 | MageKing17 | Status | code review => resolved |
2014-11-21 17:56 | MageKing17 | Resolution | open => fixed |