Fix CVertexBuffer::setType when switching no empty vertex arrays. IVertexBuffer interface changes.

Previously vertex buffer did some invalid casts to references which could cause it to copy whatever was in memory into the vertex-arrays.
Generally it worked up to S3DVertex - but switch from another type to S3DVertex2TCoords or S3DVertexTangents caused it to be filled with whatever was in memory behind it.
Setter functions in IVertexBuffer have now overloads for all known vertex types.
Also adding const version of IVertexBuffer::getData.
And some warnings in comments about using the array functions (if even we mess it up...)
Also de-deprecate IIndexBuffer::pointer() again. I don't like having 2 functions for same stuff, but in the end it doesn't really hurt keeping it around.

git-svn-id: svn://svn.code.sf.net/p/irrlicht/code/trunk@6360 dfc29bdd-3216-0410-991c-e03cc46cb475
This commit is contained in:
cutealien 2022-04-25 16:19:20 +00:00
parent f77ae3aaf5
commit 2fa3b0d1ee
5 changed files with 87 additions and 27 deletions

View File

@ -1,8 +1,12 @@
-------------------------- --------------------------
Changes in 1.9 (not yet released) Changes in 1.9 (not yet released)
- IIndexBuffer has some interface changes: pointer() deprecated (was same as getData()). Adding a const version of getData(). - Fix CVertexBuffer::setType switching types for non-empty arrays. Before we had some bad casts which could result in random initializing of some vertex data.
Also some functions no longer use references but copy by value which shouldn't be slower for this and is safer to use. - IVertexBuffer interface changes: pointer() now returns void*. Adding a const version of getData(). Some set functions have now overload for all vertex types.
- S3DVertex now always initializes color to 0xffffff. That was previously the only uninitialized variable in there. Also both derived classes have now a constructor taking a const S3DVertex&.
- Fix deserialization in CGUITable. Use now getAttributeAsStringW instead of getAttributeAsString.
Thanks @chronologicaldot for report and patch: https://irrlicht.sourceforge.io/forum/viewtopic.php?f=7&t=52821
- IIndexBuffer interface changes: Adding a const version of getData(). Some set functions now copy by value instead of per reference, which shouldn't be slower for this and is safer to use.
- Fix CIndexBuffer::setType going from 16 to 32 bit. This had some casts which simply could mess up the results. - Fix CIndexBuffer::setType going from 16 to 32 bit. This had some casts which simply could mess up the results.
- Fix OSX nor resizing properly. Thanks @torleif, Jordach and sfan5 for patch and report: https://irrlicht.sourceforge.io/forum/viewtopic.php?f=2&t=52819 - Fix OSX nor resizing properly. Thanks @torleif, Jordach and sfan5 for patch and report: https://irrlicht.sourceforge.io/forum/viewtopic.php?f=2&t=52819
- X meshloader fixes bug with uninitialized normals. Thanks @sfan5 for patch: https://irrlicht.sourceforge.io/forum/viewtopic.php?f=2&t=52819 - X meshloader fixes bug with uninitialized normals. Thanks @sfan5 for patch: https://irrlicht.sourceforge.io/forum/viewtopic.php?f=2&t=52819

View File

@ -25,14 +25,20 @@ namespace scene
virtual u32 size() const =0; virtual u32 size() const =0;
virtual void push_back (const video::S3DVertex &element) =0; virtual void push_back (const video::S3DVertex &element) =0;
virtual void push_back(const video::S3DVertex2TCoords &element) =0;
virtual void push_back(const video::S3DVertexTangents &element) =0;
virtual void setValue(u32 index, const video::S3DVertex &value) =0; virtual void setValue(u32 index, const video::S3DVertex &value) =0;
virtual void setValue(u32 index, const video::S3DVertex2TCoords &value) =0;
virtual void setValue(u32 index, const video::S3DVertexTangents &value) =0;
virtual video::S3DVertex& operator [](u32 index) = 0; virtual video::S3DVertex& operator [](u32 index) = 0;
virtual video::S3DVertex& operator [](const u32 index) const =0; virtual video::S3DVertex& operator [](const u32 index) const =0;
virtual video::S3DVertex& getLast() =0; virtual video::S3DVertex& getLast() =0;
virtual void set_used(u32 usedNow) =0; virtual void set_used(u32 usedNow) =0;
virtual void reallocate(u32 new_size, bool canShrink=true) =0; virtual void reallocate(u32 new_size, bool canShrink=true) =0;
virtual u32 allocated_size() const =0; virtual u32 allocated_size() const =0;
virtual video::S3DVertex* pointer() =0; virtual void* pointer() =0;
virtual const void* const_pointer() const =0;
virtual video::E_VERTEX_TYPE getType() const =0; virtual video::E_VERTEX_TYPE getType() const =0;
}; };
@ -47,10 +53,18 @@ namespace scene
virtual u32 size() const IRR_OVERRIDE {return Vertices.size();} virtual u32 size() const IRR_OVERRIDE {return Vertices.size();}
virtual void push_back (const video::S3DVertex &element) IRR_OVERRIDE virtual void push_back (const video::S3DVertex &element) IRR_OVERRIDE
{Vertices.push_back((T&)element);} {Vertices.push_back(element);}
virtual void push_back(const video::S3DVertex2TCoords &element) IRR_OVERRIDE
{Vertices.push_back(element);}
virtual void push_back(const video::S3DVertexTangents &element) IRR_OVERRIDE
{Vertices.push_back(element);}
virtual void setValue(u32 index, const video::S3DVertex &value) IRR_OVERRIDE virtual void setValue(u32 index, const video::S3DVertex &value) IRR_OVERRIDE
{Vertices[index] = (T&)value;} {Vertices[index] = value;}
virtual void setValue(u32 index, const video::S3DVertex2TCoords &value) IRR_OVERRIDE
{Vertices[index] = value;}
virtual void setValue(u32 index, const video::S3DVertexTangents &value) IRR_OVERRIDE
{Vertices[index] = value;}
virtual video::S3DVertex& operator [](u32 index) IRR_OVERRIDE virtual video::S3DVertex& operator [](u32 index) IRR_OVERRIDE
{return (video::S3DVertex&)Vertices[index];} {return (video::S3DVertex&)Vertices[index];}
@ -72,7 +86,8 @@ namespace scene
return Vertices.allocated_size(); return Vertices.allocated_size();
} }
virtual video::S3DVertex* pointer() IRR_OVERRIDE {return Vertices.pointer();} virtual void* pointer() IRR_OVERRIDE {return Vertices.pointer();}
virtual const void* const_pointer() const IRR_OVERRIDE {return Vertices.const_pointer();}
virtual video::E_VERTEX_TYPE getType() const IRR_OVERRIDE {return T::getType();} virtual video::E_VERTEX_TYPE getType() const IRR_OVERRIDE {return T::getType();}
}; };
@ -102,7 +117,6 @@ namespace scene
delete Vertices; delete Vertices;
} }
virtual void setType(video::E_VERTEX_TYPE vertexType) IRR_OVERRIDE virtual void setType(video::E_VERTEX_TYPE vertexType) IRR_OVERRIDE
{ {
if ( Vertices && Vertices->getType() == vertexType ) if ( Vertices && Vertices->getType() == vertexType )
@ -132,8 +146,27 @@ namespace scene
{ {
NewVertices->reallocate( Vertices->size() ); NewVertices->reallocate( Vertices->size() );
for(u32 n=0;n<Vertices->size();++n) switch (Vertices->getType()) // old type
NewVertices->push_back((*Vertices)[n]); {
case video::EVT_STANDARD:
{
for(u32 n=0;n<Vertices->size();++n)
NewVertices->push_back((*Vertices)[n]);
break;
}
case video::EVT_2TCOORDS:
{
for(u32 n=0;n<Vertices->size();++n)
NewVertices->push_back((video::S3DVertex2TCoords&)(*Vertices)[n]);
break;
}
case video::EVT_TANGENTS:
{
for(u32 n=0;n<Vertices->size();++n)
NewVertices->push_back((video::S3DVertexTangents&)(*Vertices)[n]);
break;
}
}
delete Vertices; delete Vertices;
} }
@ -142,6 +175,7 @@ namespace scene
} }
virtual void* getData() IRR_OVERRIDE {return Vertices->pointer();} virtual void* getData() IRR_OVERRIDE {return Vertices->pointer();}
virtual const void* getData() const IRR_OVERRIDE {return Vertices->const_pointer();}
virtual video::E_VERTEX_TYPE getType() const IRR_OVERRIDE {return Vertices->getType();} virtual video::E_VERTEX_TYPE getType() const IRR_OVERRIDE {return Vertices->getType();}
@ -157,11 +191,31 @@ namespace scene
Vertices->push_back(element); Vertices->push_back(element);
} }
virtual void push_back(const video::S3DVertex2TCoords &element) IRR_OVERRIDE
{
Vertices->push_back(element);
}
virtual void push_back(const video::S3DVertexTangents &element) IRR_OVERRIDE
{
Vertices->push_back(element);
}
virtual void setValue(u32 index, const video::S3DVertex &value) IRR_OVERRIDE virtual void setValue(u32 index, const video::S3DVertex &value) IRR_OVERRIDE
{ {
Vertices->setValue(index, value); Vertices->setValue(index, value);
} }
virtual void setValue(u32 index, const video::S3DVertex2TCoords &value) IRR_OVERRIDE
{
Vertices->setValue(index, value);
}
virtual void setValue(u32 index, const video::S3DVertexTangents &value) IRR_OVERRIDE
{
Vertices->setValue(index, value);
}
virtual video::S3DVertex& operator [](u32 index) IRR_OVERRIDE virtual video::S3DVertex& operator [](u32 index) IRR_OVERRIDE
{ {
return (*Vertices)[index]; return (*Vertices)[index];
@ -192,11 +246,6 @@ namespace scene
return Vertices->allocated_size(); return Vertices->allocated_size();
} }
virtual video::S3DVertex* pointer() IRR_OVERRIDE
{
return Vertices->pointer();
}
//! get the current hardware mapping hint //! get the current hardware mapping hint
virtual E_HARDWARE_MAPPING getHardwareMappingHint() const IRR_OVERRIDE virtual E_HARDWARE_MAPPING getHardwareMappingHint() const IRR_OVERRIDE
{ {

View File

@ -26,8 +26,8 @@ namespace scene
//! Const pointer to first element //! Const pointer to first element
virtual const void* getData() const =0; virtual const void* getData() const =0;
//! Deprecated: Was same as getData(), just closer to core::array interface //! Same as getData(), just closer to core::array interface
IRR_DEPRECATED void* pointer() { return getData(); } void* pointer() { return getData(); }
virtual video::E_INDEX_TYPE getType() const =0; virtual video::E_INDEX_TYPE getType() const =0;

View File

@ -21,8 +21,11 @@ namespace scene
//! Pointer to first element of vertex data //! Pointer to first element of vertex data
virtual void* getData() =0; virtual void* getData() =0;
//! Same as getData() and real type returned is not always video::S3DVertex* but depends on type //! Const pointer to first element
virtual video::S3DVertex* pointer() =0; virtual const void* getData() const =0;
//! Same as getData.
virtual void* pointer() { return getData(); }
virtual video::E_VERTEX_TYPE getType() const =0; virtual video::E_VERTEX_TYPE getType() const =0;
virtual void setType(video::E_VERTEX_TYPE vertexType) =0; virtual void setType(video::E_VERTEX_TYPE vertexType) =0;
@ -34,16 +37,20 @@ namespace scene
virtual u32 size() const =0; virtual u32 size() const =0;
//! Add vertex to end. //! Add vertex to end.
//* Note that depending on vertex type reference has to be one of the types derived from video::S3DVertex. */ //* Note that if you pass another type than the currently used vertex type then information can be lost */
virtual void push_back(const video::S3DVertex &element) =0; virtual void push_back(const video::S3DVertex &element) =0;
virtual void push_back(const video::S3DVertex2TCoords &element) =0;
virtual void push_back(const video::S3DVertexTangents &element) =0;
//! Set value at index. Buffer must be already large enough that element exists. //! Set value at index. Buffer must be already large enough that element exists.
/** Depending on vertex type reference has to be one of the types derived from video::S3DVertex */ //* Note that if you pass another type than the currently used vertex type then information can be lost */
virtual void setValue(u32 index, const video::S3DVertex &value) =0; virtual void setValue(u32 index, const video::S3DVertex &value) =0;
virtual void setValue(u32 index, const video::S3DVertex2TCoords &value) =0;
virtual void setValue(u32 index, const video::S3DVertexTangents &value) =0;
// Note that the reference can also be to one of the types derived from video::S3DVertex. //! Direct access to elements. Risky to use!
// This is especially important for the non const access version - you need to have the correct type and do some casting /** The reference _must_ be cast to the correct type before use. It's only video::S3DVertex if getType is EVT_STANDARD.
// if you write vertices. otherwise cast it first to a reference type derived from S3DVertex like S3DVertex2TCoords& or S3DVertexTangents&. */
virtual video::S3DVertex& operator [](u32 index) = 0; virtual video::S3DVertex& operator [](u32 index) = 0;
virtual video::S3DVertex& operator [](const u32 index) const =0; virtual video::S3DVertex& operator [](const u32 index) const =0;
virtual video::S3DVertex& getLast() =0; virtual video::S3DVertex& getLast() =0;

View File

@ -1,4 +1,4 @@
Tests finished. 72 tests of 72 passed. Tests finished. 72 tests of 72 passed.
Compiled as DEBUG Compiled as DEBUG
Test suite pass at GMT Thu Apr 21 21:46:37 2022 Test suite pass at GMT Mon Apr 25 14:20:20 2022