From 2fa3b0d1eed3580d5999e1ee9bb5670b455ade2e Mon Sep 17 00:00:00 2001 From: cutealien Date: Mon, 25 Apr 2022 16:19:20 +0000 Subject: [PATCH] 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 --- changes.txt | 8 +++- include/CVertexBuffer.h | 73 ++++++++++++++++++++++++++++------ include/IIndexBuffer.h | 4 +- include/IVertexBuffer.h | 21 ++++++---- tests/tests-last-passed-at.txt | 8 ++-- 5 files changed, 87 insertions(+), 27 deletions(-) diff --git a/changes.txt b/changes.txt index 2f39012..cc20ad5 100644 --- a/changes.txt +++ b/changes.txt @@ -1,8 +1,12 @@ -------------------------- 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 CVertexBuffer::setType switching types for non-empty arrays. Before we had some bad casts which could result in random initializing of some vertex data. +- 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 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 diff --git a/include/CVertexBuffer.h b/include/CVertexBuffer.h index cf569e6..5494b97 100644 --- a/include/CVertexBuffer.h +++ b/include/CVertexBuffer.h @@ -25,14 +25,20 @@ namespace scene virtual u32 size() const =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::S3DVertex2TCoords &value) =0; + virtual void setValue(u32 index, const video::S3DVertexTangents &value) =0; + virtual video::S3DVertex& operator [](u32 index) = 0; virtual video::S3DVertex& operator [](const u32 index) const =0; virtual video::S3DVertex& 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; - virtual video::S3DVertex* pointer() =0; + virtual void* pointer() =0; + virtual const void* const_pointer() 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 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 - {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 {return (video::S3DVertex&)Vertices[index];} @@ -72,7 +86,8 @@ namespace scene 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();} }; @@ -102,7 +117,6 @@ namespace scene delete Vertices; } - virtual void setType(video::E_VERTEX_TYPE vertexType) IRR_OVERRIDE { if ( Vertices && Vertices->getType() == vertexType ) @@ -132,8 +146,27 @@ namespace scene { NewVertices->reallocate( Vertices->size() ); - for(u32 n=0;nsize();++n) - NewVertices->push_back((*Vertices)[n]); + switch (Vertices->getType()) // old type + { + case video::EVT_STANDARD: + { + for(u32 n=0;nsize();++n) + NewVertices->push_back((*Vertices)[n]); + break; + } + case video::EVT_2TCOORDS: + { + for(u32 n=0;nsize();++n) + NewVertices->push_back((video::S3DVertex2TCoords&)(*Vertices)[n]); + break; + } + case video::EVT_TANGENTS: + { + for(u32 n=0;nsize();++n) + NewVertices->push_back((video::S3DVertexTangents&)(*Vertices)[n]); + break; + } + } delete Vertices; } @@ -142,6 +175,7 @@ namespace scene } 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();} @@ -157,11 +191,31 @@ namespace scene 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 { 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 { return (*Vertices)[index]; @@ -192,11 +246,6 @@ namespace scene return Vertices->allocated_size(); } - virtual video::S3DVertex* pointer() IRR_OVERRIDE - { - return Vertices->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 96a58f1..a39e306 100644 --- a/include/IIndexBuffer.h +++ b/include/IIndexBuffer.h @@ -26,8 +26,8 @@ namespace scene //! 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(); } + //! Same as getData(), just closer to core::array interface + void* pointer() { return getData(); } virtual video::E_INDEX_TYPE getType() const =0; diff --git a/include/IVertexBuffer.h b/include/IVertexBuffer.h index e201a5b..4133637 100644 --- a/include/IVertexBuffer.h +++ b/include/IVertexBuffer.h @@ -21,8 +21,11 @@ namespace scene //! Pointer to first element of vertex data virtual void* getData() =0; - //! Same as getData() and real type returned is not always video::S3DVertex* but depends on type - virtual video::S3DVertex* pointer() =0; + //! Const pointer to first element + virtual const void* getData() const =0; + + //! Same as getData. + virtual void* pointer() { return getData(); } virtual video::E_VERTEX_TYPE getType() const =0; virtual void setType(video::E_VERTEX_TYPE vertexType) =0; @@ -34,16 +37,20 @@ namespace scene virtual u32 size() const =0; //! 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::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. - /** 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::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. - // This is especially important for the non const access version - you need to have the correct type and do some casting - // if you write vertices. + //! Direct access to elements. Risky to use! + /** The reference _must_ be cast to the correct type before use. It's only video::S3DVertex if getType is EVT_STANDARD. + 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 [](const u32 index) const =0; virtual video::S3DVertex& getLast() =0; diff --git a/tests/tests-last-passed-at.txt b/tests/tests-last-passed-at.txt index cd5f885..c44f2d1 100644 --- a/tests/tests-last-passed-at.txt +++ b/tests/tests-last-passed-at.txt @@ -1,4 +1,4 @@ -Tests finished. 72 tests of 72 passed. -Compiled as DEBUG -Test suite pass at GMT Thu Apr 21 21:46:37 2022 - +Tests finished. 72 tests of 72 passed. +Compiled as DEBUG +Test suite pass at GMT Mon Apr 25 14:20:20 2022 +