From 92564435f4d2373a6a22002b219504611d93102e Mon Sep 17 00:00:00 2001 From: cutealien Date: Fri, 22 Apr 2022 16:28:29 +0000 Subject: [PATCH] Fix bad cast in CIndexBuffer. Modify IIndexBuffer interface for cleanup and safety. CIndexBuffer::setType could end up casting a temporary u16 variable to u32& (reference!). Probably never noticed as this tended to mostly works (guess next byte on stack often 0?). To prevent this from happening again I modifed IIndexBuffer::push_back to work with copies instead of references. While breaking the interface anyway I also deprecated pointer() which is just identical to getData() anyway. I get the idea of staying similar to core::array interface, but it's just confusing (and not same due to lack of types anyway). Also added a const version for getData() On a side-note - same setType bug still in CVertexBuffer, but a bit harder to fix there. So will be an new patch. git-svn-id: svn://svn.code.sf.net/p/irrlicht/code/trunk@6357 dfc29bdd-3216-0410-991c-e03cc46cb475 --- changes.txt | 3 +++ include/CIndexBuffer.h | 24 ++++++++++-------------- include/IIndexBuffer.h | 19 +++++++++++++++---- 3 files changed, 28 insertions(+), 18 deletions(-) diff --git a/changes.txt b/changes.txt index b2c436f6..2f39012a 100644 --- a/changes.txt +++ b/changes.txt @@ -1,6 +1,9 @@ -------------------------- Changes in 1.9 (not yet released) +- IIndexBuffer has some interface changes: pointer() deprecated (was same as getData()). Adding a const version of getData(). + Also some functions no longer use references but copy by value 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 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 - stl meshloader now faster, especially with text format diff --git a/include/CIndexBuffer.h b/include/CIndexBuffer.h index 3f3381b6..a29d1379 100644 --- a/include/CIndexBuffer.h +++ b/include/CIndexBuffer.h @@ -14,7 +14,7 @@ namespace scene class CIndexBuffer : public IIndexBuffer { - + // Virtual function wrapper around irr::core::array class IIndexList { public: @@ -22,7 +22,7 @@ namespace scene virtual u32 stride() const =0; virtual u32 size() const =0; - virtual void push_back(const u32 &element) =0; + virtual void push_back(u32 value) =0; virtual u32 operator [](u32 index) const =0; virtual u32 getLast() =0; virtual void setValue(u32 index, u32 value) =0; @@ -30,6 +30,7 @@ namespace scene virtual void reallocate(u32 new_size, bool canShrink=true) =0; virtual u32 allocated_size() const =0; virtual void* pointer() =0; + virtual const void* const_pointer() const =0; virtual video::E_INDEX_TYPE getType() const =0; }; @@ -43,10 +44,9 @@ namespace scene virtual u32 size() const IRR_OVERRIDE {return Indices.size();} - virtual void push_back(const u32 &element) IRR_OVERRIDE + virtual void push_back(u32 value) IRR_OVERRIDE { - // push const ref due to compiler problem with gcc 4.6, big endian - Indices.push_back((const T&)element); + Indices.push_back((T)value); } virtual u32 operator [](u32 index) const IRR_OVERRIDE @@ -76,7 +76,8 @@ namespace scene return Indices.allocated_size(); } - virtual void* pointer() IRR_OVERRIDE {return Indices.pointer();} + virtual void* pointer() IRR_OVERRIDE { return Indices.pointer(); } + virtual const void* const_pointer() const IRR_OVERRIDE { return Indices.const_pointer(); } virtual video::E_INDEX_TYPE getType() const IRR_OVERRIDE { @@ -109,7 +110,6 @@ namespace scene delete Indices; } - //virtual void setType(video::E_INDEX_TYPE IndexType); virtual void setType(video::E_INDEX_TYPE indexType) IRR_OVERRIDE { if ( Indices && Indices->getType() == indexType ) @@ -145,6 +145,7 @@ namespace scene } virtual void* getData() IRR_OVERRIDE {return Indices->pointer();} + virtual const void* getData() const IRR_OVERRIDE { return Indices->const_pointer(); } virtual video::E_INDEX_TYPE getType() const IRR_OVERRIDE {return Indices->getType();} @@ -155,9 +156,9 @@ namespace scene return Indices->size(); } - virtual void push_back(const u32 &element) IRR_OVERRIDE + virtual void push_back(u32 value) IRR_OVERRIDE { - Indices->push_back(element); + Indices->push_back(value); } virtual u32 operator [](u32 index) const IRR_OVERRIDE @@ -190,11 +191,6 @@ namespace scene return Indices->allocated_size(); } - virtual void* pointer() IRR_OVERRIDE - { - return Indices->pointer(); - } - //! get the current hardware mapping hint virtual E_HARDWARE_MAPPING getHardwareMappingHint() const IRR_OVERRIDE { diff --git a/include/IIndexBuffer.h b/include/IIndexBuffer.h index 59749af0..96a58f16 100644 --- a/include/IIndexBuffer.h +++ b/include/IIndexBuffer.h @@ -23,10 +23,18 @@ namespace scene //! Pointer to first element virtual void* getData() =0; - //! Same as getData() - virtual void* pointer() =0; + //! Const pointer to first element + virtual const void* getData() const =0; + + //! Deprecated: Was same as getData(), just closer to core::array interface + IRR_DEPRECATED void* pointer() { return getData(); } virtual video::E_INDEX_TYPE getType() const =0; + + //! Change between 16 and 32 bit indices. + /** This copies all indices to a new buffer of corresponding type. + Be careful - going from 32 to 16 bit will only work correctly + if none of your indices is larger than 16 bit. */ virtual void setType(video::E_INDEX_TYPE IndexType) =0; //! Number of bytes per element @@ -35,14 +43,17 @@ namespace scene //! Number of elements virtual u32 size() const =0; - virtual void push_back (const u32 &element) =0; + //! Add value to end. Note that for 16 bit index types values shouldn't be larger than u16 + virtual void push_back(u32 value) =0; - //! Set value at index. + //! Set value at index. Note that for 16 bit index types values shouldn't be larger than u16 /** Buffer must be already large enough. This is basically the non const version of operator [] */ virtual void setValue(u32 index, u32 value) =0; + //! Access element value at given index virtual u32 operator [](u32 index) const =0; virtual u32 getLast() =0; + virtual void set_used(u32 usedNow) =0; virtual void reallocate(u32 new_size, bool canShrink=true) =0; virtual u32 allocated_size() const=0;