cppcheck:
[apps/esmtool/record.cpp:697]: (performance) Prefer prefix ++/-- operators for non-primitive types.
[apps/esmtool/record.cpp:1126]: (performance) Prefer prefix ++/-- operators for non-primitive types.
[apps/esmtool/record.cpp:1138]: (performance) Prefer prefix ++/-- operators for non-primitive types.
[apps/niftest/niftest.cpp:36]: (performance) Function parameter 'filename' should be passed by reference.
[apps/niftest/niftest.cpp:41]: (performance) Function parameter 'filename' should be passed by reference.
[apps/opencs/model/prefs/boolsetting.cpp:25]: (warning) Possible leak in public function. The pointer 'mWidget' is not deallocated before it is allocated.
[apps/opencs/model/prefs/shortcuteventhandler.cpp:52]: (warning) Return value of std::remove() ignored. Elements remain in container.
[apps/openmw/mwstate/quicksavemanager.cpp:5]: (performance) Variable 'mSaveName' is assigned in constructor body. Consider performing initialization in initialization list.
PVS-Studio:
apps/opencs/model/filter/parser.cpp 582 warn V560 A part of conditional expression is always true: allowPredefined.
apps/opencs/view/world/referencecreator.cpp 67 warn V547 Expression '!errors.empty()' is always false.
apps/opencs/view/world/referencecreator.cpp 74 warn V547 Expression '!errors.empty()' is always false.
apps/opencs/view/doc/loader.cpp 170 warn V560 A part of conditional expression is always true: !completed.
apps/opencs/view/doc/loader.cpp 170 warn V560 A part of conditional expression is always true: !error.empty().
apps/opencs/model/tools/pathgridcheck.cpp 32 err V517 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. Check lines: 32, 34.
apps/opencs/model/world/refidadapterimp.cpp 1376 err V547 Expression 'subColIndex < 3' is always true.
apps/openmw/mwgui/widgets.hpp 318 warn V703 It is odd that the 'mEnableRepeat' field in derived class 'MWScrollBar' overwrites field in base class 'ScrollBar'. Check lines: widgets.hpp:318, MyGUI_ScrollBar.h:179.
apps/openmw/mwgui/widgets.hpp 319 warn V703 It is odd that the 'mRepeatTriggerTime' field in derived class 'MWScrollBar' overwrites field in base class 'ScrollBar'. Check lines: widgets.hpp:319, MyGUI_ScrollBar.h:180.
apps/openmw/mwgui/widgets.hpp 320 warn V703 It is odd that the 'mRepeatStepTime' field in derived class 'MWScrollBar' overwrites field in base class 'ScrollBar'. Check lines: widgets.hpp:320, MyGUI_ScrollBar.h:181
apps/openmw/mwmechanics/actors.cpp 1425 warn V547 Expression '!detected' is always true.
apps/openmw/mwmechanics/character.cpp 2155 err V547 Expression 'mode == 0' is always true.
apps/openmw/mwmechanics/character.cpp 1192 warn V592 The expression was enclosed by parentheses twice: ((expression)). One pair of parentheses is unnecessary or misprint is present.
apps/openmw/mwmechanics/character.cpp 521 warn V560 A part of conditional expression is always true: (idle == mIdleState).
apps/openmw/mwmechanics/pathfinding.cpp 317 err V547 Expression 'mPath.size() >= 2' is always true.
apps/openmw/mwscript/interpretercontext.cpp 409 warn V560 A part of conditional expression is always false: rank > 9.
apps/openmw/mwgui/windowbase.cpp 28 warn V560 A part of conditional expression is always true: !visible.
apps/openmw/mwgui/journalwindow.cpp 561 warn V547 Expression '!mAllQuests' is always false.
apps/openmw/mwgui/referenceinterface.cpp 18 warn V571 Recurring check. The '!mPtr.isEmpty()' condition was already verified in line 16.
apps/openmw/mwworld/scene.cpp 463 warn V547 Expression 'adjustPlayerPos' is always true.
apps/openmw/mwworld/worldimp.cpp 409 err V766 An item with the same key '"sCompanionShare"' has already been added.
apps/openmw/mwworld/cellstore.cpp 691 warn V519 The 'state.mWaterLevel' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 689, 691.
apps/openmw/mwworld/weather.cpp 1125 warn V519 The 'mResult.mParticleEffect' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 1123, 1125.
apps/openmw/mwworld/weather.cpp 1137 warn V519 The 'mResult.mParticleEffect' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 1135, 1137.
apps/wizard/unshield/unshieldworker.cpp 475 warn V728 An excessive check can be simplified. The '(A && B) || (!A && !B)' expression is equivalent to the 'bool(A) == bool(B)' expression.
apps/wizard/installationpage.cpp 163 warn V735 Possibly an incorrect HTML. The "</p" closing tag was encountered, while the "</span" tag was expected.
components/fontloader/fontloader.cpp 427 err V547 Expression 'i == 1' is always true.
components/nifosg/nifloader.cpp 282 warn V519 The 'created' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 278, 282.
components/esm/loadregn.cpp 119 err V586 The 'clear' function is called twice for deallocation of the same resource. Check lines: 112, 119.
components/esm/cellref.cpp 178 warn V581 The conditional expressions of the 'if' statements situated alongside each other are identical. Check lines: 175, 178.
components/esmterrain/storage.cpp 235 warn V560 A part of conditional expression is always true: colStart == 0.
components/esmterrain/storage.cpp 237 warn V560 A part of conditional expression is always true: rowStart == 0.
Double buffering, custom bounding box and the update in the cull visitor (instead of update) are now all handled internally rather than needing hacks and/or callbacks.
The double buffering is an implementation detail so it should be handled as such, rather than mandating the scene graph to be structured in a certain way.
Override accept(NodeVisitor&) instead of using callbacks.
A Warning indicates a potential problem in the content file(s) that the user told OpenMW to load. E.g. this might cause an object to not display at all or as intended, however the rest of the game will run fine.
An Error, however, is more likely to be a bug with the engine itself - it means that basic assumptions have been violated and the engine might not run correctly anymore.
The above mostly applies to errors/warnings during game-play; startup issues are handled differently: when a file is completely invalid/corrupted to the point that the engine can not start, that might cause messages that are worded as Error due to the severity of the issue but are not necessarily the engine's fault.
Hopefully, being a little more consistent here will alleviate confusion among users as to when a log message should be reported and to whom.
The first part of the fix is to assign VBO/EBO's upon loading the array in the Nif reader. This avoids triggering the 'addVertexBufferObjectIfRequired' code path in osg::Geometry which has the race condition when two threads add the same Array at the same time. Essentially, we want the Arrays to be 'const' when they come out of the Nif reader.
The second part of the fix is to make sure not to create empty arrays in the Nif reader (importantly, not assigning a VBO to the empty array). This empty array would be deleted when the NIFFile is cleaned up, and the detachment of the VBO assigned to it (which is still in use by other arrays) would cause threading issues.
This rare crash bug was first introduced with commit a7c5beb7c5. When using OSG dev version 3.5 the crashes were a little more prevalent, because 'addVertexBufferObjectIfRequired' in osg::Geometry is now used even when VBO's are disabled (as part of the VAO support changes).
The previous commit broke some magic effects (e.g. shield). The controller's emit start/stop times should control the emitter, while the absence of the ParticleSystem's autoplay flag should freeze the whole particle system if no valid controller source gets assigned. This revised version works for both the Shield effect and the "dagoth ur death sparkles" effect.
Since commit e8662bea31, we're using OSG functionality that contains an unfixed crash bug in version 3.2. The bug is fixed in version 3.4 (OSG commit 6351e5020371b0b72b300088a5c6772f58379b84)
Instead use getImage and let the caller create the Texture. Sharing of textures is then handled in post by the SharedStateManager.
This is closer to what the OSG serializer does.
Streamlines the TextureManager and will make it easier to multithread.
Practical benefits:
- Filter settings are now applied to native OSG format models. These models do not use TextureManager::getTexture2D since the model itself specifies a Texture.
- The GUI render manager will be able to use its own separate textures, making it easier to turn off filtering for them.
Cache the current position in the animation track and attempt to reuse it in the next frame.
Decent speed up for the Update phase, about 0.3 ms faster in Balmora.
Unlike what I expected, the osgUtil::UpdateVisitor is set to traverse all children (not only active children). The FrameSwitch was thus traversing both RigGeometries part of the double-buffering scheme, rather than only the one active in the current frame.
Seems wrong to me, but MW appears to do it that way. Without this fix, the light_de_candle_08_64 from http://www.nexusmods.com/morrowind/mods/41654/ has flame particles in the wrong spot.
OSG 3.4 adds the ability to place Drawables directly in the scene graph, without a Geode decorating them. Leveraging this should give a small performance boost, because the redundant Geodes increase culling overhead.
There is still an oustanding issue with the RemoveDrawableVisitor no longer working correctly, because Drawables can have multiple parents.
Works around a compiler warning with OSG 3.4:
warning: base class 'class osg::Callback' should be explicitly initialized in the copy constructor [-Wextra]
With no default argument for osg::CopyOp&, the compiler no longer sees the function as a real copy constructor and stops warning about the missing virtual initializations.
We don't care about this warning because there is nothing interesting to initialize in the osg::NodeCallback base anyway.
A proper fix for the warning would require to inserting OSG_VERSION conditional compiling all over the place, that is as long as we are still supporting OSG 3.2.
Nvidia drivers only support a maximum of 4 fixed function texture units. To resolve this problem, bind texture units in order instead of binding to the NiTexturingProperty::TextureType unit.
The Material state is now set on the NiTriShape's node rather than the Geode, thus merged with other state like NiTexturingProperties, etc that are typically attached to the NiTriShape.
Effectively cuts in half the number of StatSets for a NIF file, resulting in big speedup (~10%) in the Cull and Draw phases.
Goals:
- get rid of the mesh pre-transform (this requires supporting different bind matrices for each mesh)
- bounding box should be relative to the bone the mesh is attached to, ideally we can then get rid of the expensive skeleton-based bounding boxes
- update bone matrices in CullCallback instead of UpdateCallback
Works OK, though the bounding boxes are not correct yet.
Practical benefits:
- The controller update is now run automatically
- Creating an instance of a scene graph should now work properly using the defined copy constructors