View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0002395 | FSSCP | OpenGL | public | 2011-02-09 19:42 | 2012-05-02 11:35 |
| Reporter | casual pilot | Assigned To | The_E | ||
| Priority | high | Severity | major | Reproducibility | always |
| Status | resolved | Resolution | reopened | ||
| Product Version | 3.6.14 | ||||
| Target Version | 3.6.14 | ||||
| Summary | 0002395: Some models partially invisible | ||||
| Description | Some models such as the Deimos and Sathanas are only partially rendered with many (most?) polygons missing. | ||||
| Additional Information | Linux, r200 DRI driver (Radeon 9250). Redefining DO_RENDER in code/graphics/gropengltnl.cpp to use glDrawElements instead of glDrawRangeElements appears to solve the problem (see attached patch). Hence I suspect that incorrect start and end values were given to glDrawRangeElements. That is an error in OpenGL with no diagnostic required, so some implementations may render as intended while others don't. | ||||
| Tags | No tags attached. | ||||
|
2011-02-09 19:42
|
fso-rendering.patch (576 bytes)
diff --git a/code/graphics/gropengltnl.cpp b/code/graphics/gropengltnl.cpp index cc93dca..622d2c3 100644 --- a/code/graphics/gropengltnl.cpp +++ b/code/graphics/gropengltnl.cpp @@ -496,7 +496,7 @@ static void opengl_init_arrays(opengl_vertex_buffer *vbp, const vertex_buffer *b } #define DO_RENDER() \ - vglDrawRangeElements(GL_TRIANGLES, start, end, count, element_type, ibuffer + (datap->index_offset + start)) + glDrawElements(GL_TRIANGLES, count, element_type, ibuffer + (datap->index_offset + start)) int GL_last_shader_flags = -1; int GL_last_shader_index = -1; |
|
|
Fixed in revision 7289, by introducing a commandline flag (-use_gldrawelements) to switch between using glDrawElements and glDrawRangeElements, since the latter is noticeably faster. |
|
|
I have since verified that incorrect start and end values are indeed passed to glDrawRangeElements. Being unfamiliar with the model data structures I did not find an immediate way to obtain the actual range of vertex indices present in the array being drawn. Nevertheless, I did manage a crude fix by modifying buffer_data to keep track of the index range. |
|
|
fso-vertex-range.patch (3,678 bytes)
commit ff948a089ca61a60d8aabf225ae70993e65ba852
Author: Timo Korvola <tkorvola@iki.fi>
Date: Tue Feb 21 23:11:48 2012 +0200
Record the minimum and maximum indices when creating index buffers.
The bounds can then be used in glDrawRangeElements, which appears
somewhat faster than glDrawElements.
Conflicts:
code/graphics/gropengltnl.cpp
diff --git a/code/graphics/2d.h b/code/graphics/2d.h
index e55f65e..92eebbf 100644
--- a/code/graphics/2d.h
+++ b/code/graphics/2d.h
@@ -109,7 +109,12 @@ struct buffer_data {
size_t index_offset;
- uint *index;
+ const uint *get_index() const
+ {
+ return index;
+ }
+
+ uint i_first, i_last;
void release()
{
@@ -119,10 +124,26 @@ struct buffer_data {
}
}
- buffer_data() :
- flags(0), texture(-1), n_verts(0), index_offset(0), index(NULL)
+ void assign(int i, uint j)
+ {
+ const_cast<uint *>(index)[i] = j;
+ if (i_first > i_last)
+ i_first = i_last = j;
+ else if (i_first > j)
+ i_first = j;
+ else if (i_last < j)
+ i_last = j;
+ }
+
+ buffer_data(int n_vrts) :
+ flags(0), texture(-1), n_verts(n_vrts), index_offset(0),
+ i_first(1), i_last(0)
{
+ index = new(std::nothrow) uint[n_verts];
}
+
+private:
+ uint *index;
};
struct vertex_buffer {
diff --git a/code/graphics/gropengltnl.cpp b/code/graphics/gropengltnl.cpp
index 6a07156..eeed02c 100644
--- a/code/graphics/gropengltnl.cpp
+++ b/code/graphics/gropengltnl.cpp
@@ -357,7 +357,7 @@ bool gr_opengl_pack_buffer(const int buffer_id, vertex_buffer *vb)
for (j = 0; j < vb->tex_buf.size(); j++) {
n_verts = vb->tex_buf[j].n_verts;
uint offset = vb->tex_buf[j].index_offset;
- uint *index = vb->tex_buf[j].index;
+ const uint *index = vb->tex_buf[j].get_index();
// bump to our spot in the buffer
GLubyte *ibuf = m_vbp->index_list + offset;
@@ -494,7 +494,7 @@ static void opengl_init_arrays(opengl_vertex_buffer *vbp, const vertex_buffer *b
if (Cmdline_drawelements) \
glDrawElements(GL_TRIANGLES, count, element_type, ibuffer + (datap->index_offset + start)); \
else \
- vglDrawRangeElements(GL_TRIANGLES, start, end, count, element_type, ibuffer + (datap->index_offset + start));
+ vglDrawRangeElements(GL_TRIANGLES, datap->i_first, datap->i_last, count, element_type, ibuffer + (datap->index_offset + start));
int GL_last_shader_flags = -1;
int GL_last_shader_index = -1;
diff --git a/code/model/modelinterp.cpp b/code/model/modelinterp.cpp
index 92c073f..d9aa672 100644
--- a/code/model/modelinterp.cpp
+++ b/code/model/modelinterp.cpp
@@ -4245,22 +4245,21 @@ void interp_configure_vertex_buffers(polymodel *pm, int mn)
if ( !polygon_list[i].n_verts )
continue;
- buffer_data new_buffer;
+ buffer_data new_buffer(polygon_list[i].n_verts);
- new_buffer.index = new(std::nothrow) uint[polygon_list[i].n_verts];
- Verify( new_buffer.index != NULL );
+ Verify( new_buffer.get_index() != NULL );
for (j = 0; j < polygon_list[i].n_verts; j++) {
if (ibuffer_info.read != NULL) {
first_index = cfread_int(ibuffer_info.read);
Assert( first_index >= 0 );
- new_buffer.index[j] = (uint)first_index;
+ new_buffer.assign(j, first_index);
} else {
first_index = model_list->find_index(&polygon_list[i], j);
Assert(first_index != -1);
- new_buffer.index[j] = (uint)first_index;
+ new_buffer.assign(j, first_index);
if (ibuffer_info.write != NULL) {
cfwrite_int(first_index, ibuffer_info.write);
@@ -4268,7 +4267,6 @@ void interp_configure_vertex_buffers(polymodel *pm, int mn)
}
}
- new_buffer.n_verts = polygon_list[i].n_verts;
new_buffer.texture = i;
new_buffer.flags = 0;
|
|
|
This is a big issue, thanks for debugging. Can we get that patch as a standard diff? I'd like to look into this more. |
|
|
Sorry, I don't understand what you mean by standard diff. patch -p1 seems to work fine on the attached patch. |
|
|
The problem is, you are attaching a git patch when we are using svn. |
|
|
fso-vertex-range-svn.patch (3,087 bytes)
Index: code/graphics/gropengltnl.cpp
===================================================================
--- code/graphics/gropengltnl.cpp (revision 8555)
+++ code/graphics/gropengltnl.cpp (working copy)
@@ -357,7 +357,7 @@
for (j = 0; j < vb->tex_buf.size(); j++) {
n_verts = vb->tex_buf[j].n_verts;
uint offset = vb->tex_buf[j].index_offset;
- uint *index = vb->tex_buf[j].index;
+ const uint *index = vb->tex_buf[j].get_index();
// bump to our spot in the buffer
GLubyte *ibuf = m_vbp->index_list + offset;
@@ -494,7 +494,7 @@
if (Cmdline_drawelements) \
glDrawElements(GL_TRIANGLES, count, element_type, ibuffer + (datap->index_offset + start)); \
else \
- vglDrawRangeElements(GL_TRIANGLES, start, end, count, element_type, ibuffer + (datap->index_offset + start));
+ vglDrawRangeElements(GL_TRIANGLES, datap->i_first, datap->i_last, count, element_type, ibuffer + (datap->index_offset + start));
int GL_last_shader_flags = -1;
int GL_last_shader_index = -1;
Index: code/graphics/2d.h
===================================================================
--- code/graphics/2d.h (revision 8555)
+++ code/graphics/2d.h (working copy)
@@ -109,7 +109,12 @@
size_t index_offset;
- uint *index;
+ const uint *get_index() const
+ {
+ return index;
+ }
+
+ uint i_first, i_last;
void release()
{
@@ -119,10 +124,26 @@
}
}
- buffer_data() :
- flags(0), texture(-1), n_verts(0), index_offset(0), index(NULL)
+ void assign(int i, uint j)
{
+ const_cast<uint *>(index)[i] = j;
+ if (i_first > i_last)
+ i_first = i_last = j;
+ else if (i_first > j)
+ i_first = j;
+ else if (i_last < j)
+ i_last = j;
}
+
+ buffer_data(int n_vrts) :
+ flags(0), texture(-1), n_verts(n_vrts), index_offset(0),
+ i_first(1), i_last(0)
+ {
+ index = new(std::nothrow) uint[n_verts];
+ }
+
+private:
+ uint *index;
};
struct vertex_buffer {
Index: code/model/modelinterp.cpp
===================================================================
--- code/model/modelinterp.cpp (revision 8555)
+++ code/model/modelinterp.cpp (working copy)
@@ -4244,22 +4244,21 @@
if ( !polygon_list[i].n_verts )
continue;
- buffer_data new_buffer;
+ buffer_data new_buffer(polygon_list[i].n_verts);
- new_buffer.index = new(std::nothrow) uint[polygon_list[i].n_verts];
- Verify( new_buffer.index != NULL );
+ Verify( new_buffer.get_index() != NULL );
for (j = 0; j < polygon_list[i].n_verts; j++) {
if (ibuffer_info.read != NULL) {
first_index = cfread_int(ibuffer_info.read);
Assert( first_index >= 0 );
- new_buffer.index[j] = (uint)first_index;
+ new_buffer.assign(j, first_index);
} else {
first_index = model_list->find_index(&polygon_list[i], j);
Assert(first_index != -1);
- new_buffer.index[j] = (uint)first_index;
+ new_buffer.assign(j, first_index);
if (ibuffer_info.write != NULL) {
cfwrite_int(first_index, ibuffer_info.write);
@@ -4267,7 +4266,6 @@
}
}
- new_buffer.n_verts = polygon_list[i].n_verts;
new_buffer.texture = i;
new_buffer.flags = 0;
|
|
|
That should not be a problem, because the unified diff format used by both is well established. Nevertheless, here is a version computed with svn diff against the current trunk. |
|
|
Fix committed to trunk@8577. |
|
|
Fix committed to fs2_open_3_6_14@8716. |
|
fs2open: trunk r8577 2012-03-05 07:01 Ported: N/A Details Diff |
From casual_pilot: Fix for Mantis 2395 (incorrect index values passed to OpenGL) |
Affected Issues 0002395 |
|
| mod - /trunk/fs2_open/code/model/modelinterp.cpp | Diff File | ||
| mod - /trunk/fs2_open/code/graphics/gropengltnl.cpp | Diff File | ||
| mod - /trunk/fs2_open/code/graphics/2d.h | Diff File | ||
|
fs2open: fs2_open_3_6_14 r8716 2012-05-02 07:36 Ported: N/A Details Diff |
Backport: Trunk r8577; From casual_pilot: Fix for Mantis 2395 (incorrect index values passed to OpenGL) |
Affected Issues 0002395 |
|
| mod - /branches/fs2_open_3_6_14/code/model/modelinterp.cpp | Diff File | ||
| mod - /branches/fs2_open_3_6_14/code/graphics/gropengltnl.cpp | Diff File | ||
| mod - /branches/fs2_open_3_6_14/code/graphics/2d.h | Diff File | ||
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2011-02-09 19:42 | casual pilot | New Issue | |
| 2011-02-09 19:42 | casual pilot | File Added: fso-rendering.patch | |
| 2011-06-28 20:21 | The_E | Note Added: 0012736 | |
| 2011-06-28 20:21 | The_E | Status | new => resolved |
| 2011-06-28 20:21 | The_E | Fixed in Version | => 3.6.13 |
| 2011-06-28 20:21 | The_E | Resolution | open => fixed |
| 2011-06-28 20:21 | The_E | Assigned To | => The_E |
| 2012-03-02 22:36 | casual pilot | Note Added: 0013372 | |
| 2012-03-02 22:36 | casual pilot | Status | resolved => feedback |
| 2012-03-02 22:36 | casual pilot | Resolution | fixed => reopened |
| 2012-03-02 22:38 | casual pilot | File Added: fso-vertex-range.patch | |
| 2012-03-03 00:31 | Echelon9 | Note Added: 0013374 | |
| 2012-03-03 00:32 | Echelon9 | Priority | normal => high |
| 2012-03-03 00:32 | Echelon9 | Product Version | 3.6.12 => 3.6.14 |
| 2012-03-03 00:32 | Echelon9 | Fixed in Version | 3.6.13 => |
| 2012-03-03 00:32 | Echelon9 | Target Version | => 3.6.14 |
| 2012-03-03 08:55 | casual pilot | Note Added: 0013375 | |
| 2012-03-03 08:55 | casual pilot | Status | feedback => assigned |
| 2012-03-03 08:57 | The_E | Note Added: 0013376 | |
| 2012-03-03 09:37 | casual pilot | File Added: fso-vertex-range-svn.patch | |
| 2012-03-03 09:38 | casual pilot | Note Added: 0013377 | |
| 2012-03-05 12:01 | The_E | Changeset attached | => fs2open trunk r8577 |
| 2012-03-05 12:01 | The_E | Note Added: 0013389 | |
| 2012-03-05 12:01 | The_E | Status | assigned => resolved |
| 2012-05-02 11:35 | niffiwan | Changeset attached | => fs2open fs2_open_3_6_14 r8716 |
| 2012-05-02 11:35 | niffiwan | Note Added: 0013500 |
