Choose a parent base on which node is used to iterate over children nodes.
This leads to duplicate handing of child nodes. A node will be handled so many
times how many parents it has.
For example:
p1 p2
\ /
c
Will be handled as:
p1 p2
| |
c c
If c has children they will be handled X times c is handled.
> warning: the variable 'key' is copy-constructed from a const reference but is
only used as const reference; consider making it a const reference
[performance-unnecessary-copy-initialization]
Found by clang-tidy.
- properly initialize mSimulationPosition in the constructor. Unlucky thread scheduling can cause processHits to be called before the first simulation run, causing the projectile to vanish to whatever value the variable happens to contains.
- don't continue moving the projectile after a hit. The position would continue to be updated to some senseless value.
=================================================================
==20931==ERROR: AddressSanitizer: heap-use-after-free on address 0x607000206030 at pc 0x7fc8b0f3a72b bp 0x7ffcee176860 sp 0x7ffcee176008
READ of size 13 at 0x607000206030 thread T0
#0 0x7fc8b0f3a72a in __interceptor_strlen /build/gcc/src/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:389
#1 0x562e069a0af7 in QString::fromUtf8(char const*, int) /usr/include/qt/QtCore/qstring.h:706
#2 0x562e069a0af7 in Launcher::AdvancedPage::AdvancedPage(Config::GameSettings&, QWidget*) /home/elsid/dev/openmw/apps/launcher/advancedpage.cpp:29
#3 0x562e06959613 in Launcher::MainDialog::createPages() /home/elsid/dev/openmw/apps/launcher/maindialog.cpp:127
#4 0x562e069691d2 in Launcher::MainDialog::setup() /home/elsid/dev/openmw/apps/launcher/maindialog.cpp:228
#5 0x562e06969d88 in Launcher::MainDialog::showFirstRunDialog() /home/elsid/dev/openmw/apps/launcher/maindialog.cpp:188
#6 0x562e06957025 in main /home/elsid/dev/openmw/apps/launcher/main.cpp:35
#7 0x7fc8ad0d9b24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)
#8 0x562e0690fced in _start (/home/elsid/dev/openmw/build/gcc/asan/openmw-launcher+0x56ced)
0x607000206030 is located 16 bytes inside of 64-byte region [0x607000206020,0x607000206060)
freed by thread T0 here:
#0 0x7fc8b0fb3f19 in __interceptor_free /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:127
#1 0x7fc8b0de3388 (/usr/lib/libopenal.so.1+0x40388)
previously allocated by thread T0 here:
#0 0x7fc8b0fb4fd6 in __interceptor_posix_memalign /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:226
#1 0x7fc8b0e379cb (/usr/lib/libopenal.so.1+0x949cb)
SUMMARY: AddressSanitizer: heap-use-after-free /build/gcc/src/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:389 in __interceptor_strlen
Shadow bytes around the buggy address:
0x0c0e80038bb0: 00 00 00 00 00 00 00 00 00 fa fa fa fa fa 00 00
0x0c0e80038bc0: 00 00 00 00 00 00 00 fa fa fa fa fa 00 00 00 00
0x0c0e80038bd0: 00 00 00 00 00 fa fa fa fa fa 00 00 00 00 00 00
0x0c0e80038be0: 00 00 02 fa fa fa fa fa 00 00 00 00 00 00 00 00
0x0c0e80038bf0: 02 fa fa fa fa fa fd fd fd fd fd fd fd fd fa fa
=>0x0c0e80038c00: fa fa fa fa fd fd[fd]fd fd fd fd fd fa fa fa fa
0x0c0e80038c10: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c0e80038c20: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c0e80038c30: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c0e80038c40: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c0e80038c50: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone: fa
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user: f7
Container overflow: fc
Array cookie: ac
Intra object redzone: bb
ASan internal: fe
Left alloca redzone: ca
Right alloca redzone: cb
Shadow gap: cc
==20931==ABORTING
/<<BUILDDIR>>/openmw-0.47.0+git202112160802~ubuntu18.04.1/apps/navmeshtool/worldspacedata.cpp:213:109: error: class template argument deduction failed:
heightfields.emplace_back(std::vector(std::begin(landData.mHeights), std::end(landData.mHeights)));
^
/<<BUILDDIR>>/openmw-0.47.0+git202112160802~ubuntu18.04.1/apps/navmeshtool/worldspacedata.cpp:213:109: error: no matching function for call to ‘vector(float*, float*)’
In file included from /usr/include/c++/7/vector:64:0,
from /<<BUILDDIR>>/openmw-0.47.0+git202112160802~ubuntu18.04.1/components/esm/loadpgrd.hpp:5,
from /<<BUILDDIR>>/openmw-0.47.0+git202112160802~ubuntu18.04.1/components/misc/convert.hpp:5,
from /<<BUILDDIR>>/openmw-0.47.0+git202112160802~ubuntu18.04.1/components/bullethelpers/collisionobject.hpp:4,
from /<<BUILDDIR>>/openmw-0.47.0+git202112160802~ubuntu18.04.1/apps/navmeshtool/worldspacedata.hpp:4,
from /<<BUILDDIR>>/openmw-0.47.0+git202112160802~ubuntu18.04.1/apps/navmeshtool/worldspacedata.cpp:1:
/usr/include/c++/7/bits/stl_vector.h:411:2: note: candidate: template<class _Tp, class _Alloc, class _InputIterator, class> vector(_InputIterator, _InputIterator, const _Alloc&)-> std::vector<_Tp, _Alloc>
vector(_InputIterator __first, _InputIterator __last,
^~~~~~
Perform all request to db in a single thread to avoid blocking navmesh
generator threads due to slow write operations.
Write to db navmesh for all changes except update as it done for memory cache.
Batch multiple db operations into a single transaction to speed up writing by
not executing fsync after each insert/update query. All reads are performed in
the same transaction so they see uncommited data.
Get rid of EscapeHashX classes option 5 (attempt 2): Use boost::filesystem::path rules if the path starts with ", and consume the whole thing verbatim otherwise
Closes#5804
See merge request OpenMW/openmw!1436
Merge conflicts included:
* One setting being removed (branch had changed its type).
* One setting's description being changed (branch had changed its type).
* List of files in components/files was changed both upstream and on the
branch.
* Upstream had changed something in a file the branch deletes.
Check if a leveled creature is in an unloaded cell before deciding it doesn't exist
Closes#4376
See merge request OpenMW/openmw!1420
(cherry picked from commit 782371cb2e7f6653d72305090033557b53bbcf3a)
6d945da7 Check if a leveled creature is in an unloaded cell before deciding it doesn't exist
Lua binding for SDL_GetKeyName, two missing scan codes
See merge request OpenMW/openmw!1450
(cherry picked from commit d86e7d4c9a28bc96af0a5638b26879fa49b8a847)
9a073baa Add Apostrophe and Period scan codes
d66f3a35 Add getKeyName to Lua input API
ed64add9 Replace mentions of KeyEvent with KEY
1. Reorder unlock and notify_all calls to avoid notifying when not all worker
threads are waiting.
2. Make sure main thread does not attempt to exclusively lock mSimulationMutex
while not all workers are done with previous frame.
3. Replace mNewFrame flag by counter to avoid modification from multiple
threads.
Specifications developed in PR #3206 require that groundcover content files must not be allowed to corrupt normal content files. With this PR we simply isolate our existing loading logic by instantiating a separate `ESMStore` for `Groundcover`. In addition, we remove some outdated workarounds.
This PR aims to solve all issues with `Groundcover` view distance handling in a satisfying way while preserving other optimisations that benefit other features. The main idea here is not to rely on `ViewData` updates for distance culling calculations because we can not accurately determine distance against lazily updated views. Instead, we perform an accurate measurement in a cull callback we can run every frame in `Groundcover` itself. While we do have to add some code to handle this feature, it is quite loosely coupled code that could be useful elsewhere in the future. These changes should address a performance regression @akortunov experienced.
This PR aims to solve `uniform block LightBufferBinding has no binding` messages @glassmancody has reportedly encountered since PR #3110 due to an apparent bug in OSG. While we do have to add a workaround here that adds a bit of clunkiness, #3216 should allow us to clean up these interactions a bit in the future.
1) As much as I dislike it, upping the collision margin from 0.1 to 0.2
fixes bugs, particularly involving walking into upwards-slanted walls.
2) There were still some problems involving acute crevices/seams; they
were using the adjusted instead of unadjusted normal, and also they need
to bypass the don't-slide-upwards check to prevent (see #6379)
3) The move-away-from-what-we-just-hit code needs to run always, not
just on non-initial iterations. No idea why I did it this way before.
4) Force bullet to give actor boxes a tiny collision margin of 0.001
instead of the default 0.04. I can't tell whether this is actually
working or not, but it should reduce unexplained weirdness.
5) A piece of code that was meant to prevent bugs by short-circuiting
the movement solver if its direction changed more than 180 degrees
actually caused problems instead of preventing them, so I deleted it.
With this PR we refactor `StringUtils::replaceAll` to accept `string_view` as suggested in a code comment. In addition, while we are touching this rebuild happy file, we slim it down a bit by moving a few sparingly used functions elsewhere.
This PR removes unneeded `lowerCaseInPlace` calls in in a hot path of `objectpaging.cpp` that are no longer necessary after PR #3197. In addition, I have been informed that these changes should by coincidence address a compiler specific compilation error we currently experience.
* Handle culling of all sound sources in a separate function cull3DSound
* Add two new settings Sound::sfx fade in duration and Sound::sfx fade out duration
* Implement a more general SoundBase::setFade that can be used to fade to any desired
volume and not just fading out
* Implement SoundBase::setFadeout by using SoundBase::setFade
* Implement an exponential fade mode
* resets state updater to apply light settings (#3141)
resets state updater to apply light settings
With this PR we achieve the same effect with fewer lines of code.
* fixes LightSource logic errors
We currently update `LightSource::setActorFade` in `TransparencyUpdater`. There are several logic errors inherent in this approach:
1. We fail to update `LightSource::setActorFade` for off screen actors because their `TransparencyUpdater` cull callback is not invoked.
2. We fail to update `LightSource::setActorFade` in the instant that a `TransparencyUpdater` is removed.
3. We fail to update `setActorFade` when an `mExtraLightSource` is created after calling `Animation::setAlpha`.
With this PR we avoid such issues by updating `LightSource::setActorFade` in `Animation::setAlpha` and `Animation::addExtraLightSource` instead.
This PR fixes an assertion introduced by #3211. For some reason my build originally did not contain assertions despite passing `DEBUG` into cmake, but after deleting `CMakeCache.txt` I have now hit the assertion @glassmancody reported as well.
With this PR we restore the previous order of `ESM::Dialogue` entries implicitly changed by PR #3197. In the future we may want to consider additional verification and documentation of `mShared` order inconsistencies. We might additionally consider applying this sorting in the particular code that requires it.
This PR aims to start addressing `ESM` design issues that have silenced errors we incorporated into groundcover `ESM` loading approaches.
- We move the resolution of `parentFileIndices` from `ESMStore` to `ESMReader` as suggested in a `TODO` comment.
- We improve a highly misleading comment which downplayed the significance of `parentFileIndices`.
- We document important preconditions.
- We move a user facing error message to the highest level and improve its context.
- We remove an inappropriate `setGlobalReaderList` method. We now pass this reader list into the method that requires it.
- We remove a thoroughly pointless optimisation of `Store<ESM::LandTexture>`'s construction that has unnecessarily depended on `getGlobalReaderList`.
There should be no functional changes for `master`, but this PR should remove an issue blocking PR #3208.
With this PR we restore @elsid 's optimisations of countRecords we have unintentionally discarded in PR #3197. In addition, we give it a more appropriate name and add comments concerning its peculiar background.
With this PR we refactor `SceneUtil::KeyframeController` not to require `virtual osg::Callback` inheritance. I suppose such `virtual` overhead is not justified here because it negatively impacts many other classes we derive from `osg::Callback`.
Previous version skipped collision the frame immediately after a call to SetPos. It worked for one-off calls (teleports for instance) and continuous call along a pre-defined path (scenic travel). However, in the case of mod which uses SetPos to simulate a player-controlled movement, it is equivalent to using tcl.
Solution:
1/ skip update of mPosition and mPreviousPosition to avoid janky interpolation
2/ use back plain moveObject() instead of moveObjectBy() since we don't want physics simulation
3/ rework a little bit waterwalking influence on coordinate because of 1/
With this PR we refactor a `premultiplied alpha` user string set by `characterpreview.cpp` into a more flexible mechanism allowing us to assign any state to GUI textures. We can consider these changes more future proof than the previous approach.
This PR aims to spark the retirement of a questionable pattern I have found all over our code base. I will illustrate how this pattern encourages code duplication, lacks type safety, requires documentation and can be prone to bugs.
```
std::map<std::string, Object> mMap; // Stored in all lowercase for a case-insensitive lookup
std::string lowerKey = Misc::StringUtils::lowerCase(key);
mMap.emplace(lowerKey, object);
std::string lowerKey = Misc::StringUtils::lowerCase(key);
mMap.find(lowerKey);
mMap.find(key); // Not found. Oops!
```
An alternative approach produces no such issues.
```
std::unordered_map<std::string, Object, Misc::StringUtils::CiHash, Misc::StringUtils::CiEqual> mMap;
mMap.emplace(key, object);
mMap.find(key);
```
Of course, such an alternative will work for a `map` as well, but an `unordered_map` is generally preferable over a `map` with these changes because we have moved `lowerCase` into the comparison operator.
In this PR I have refactored `Animation::mNodeMap` accordingly. I have reviewed and adapted all direct and indirect usage of `Animation::mNodeMap` to ensure we do not change behaviour with this PR.
To construct serializer from given entities:
* Data source/destination - any value that has to be serialized/deserialized,
usually already existing type.
* Format - functional object to define high level serialization logic to
define specific format and data schema. Like order of fields, allocation.
* Visitor - functional object to define low level serialization logic to
operator on given data part.
* BinaryWriter - copies given value into provided buffer.
* BinaryReader - copies value into given destination from provided buffer.
* SizeAccumulator - calculates required buffer size for given data.
This PR fixes a crash caused by the improperly ensured lifetime of RigGeometry::mSourceGeometry. mSourceGeometry was not adequate to ensure mSourceGeometry would outlive mGeometry because we extend mGeometrys lifetime beyond this lifetime by passing mGeometry to the draw traversal instead of this.
In addition,
We add important comments.
We detect and prevent generally unsafe operations in high level code.
We add a sprinkling of const to help clarify intentions.
Currently, we use an `UnrefQueue` which supposedly aims to transfer destruction costs to another thread. The implications of this unusual pattern can not be well understood because some allocators might free resources more efficiently if they are freed by the same thread that allocated them. In addition, `UnrefQueue` complicates the validation of thread safety in our engine. Lastly, our current usage of `UnrefQueue` triggers `ref()`, `unref()` atomic operations as objects are passed into the queue. These operations could be more expensive than the actual destruction.
With this PR we thus remove `UnrefQueue`. We can expect these changes to have a minor impact at most because we free most resources elsewhere in `ResourceSystem::updateCache`.
It can fail due to float arithmetic precision with error:
Expected equality of these values:
getAsString(lua, "moveAndScale")
Which is: "TransformM{ move(6, 22, 18) scale(0.5, 1, 0.5) rotation(angle=8.53284e-17, axis=(1, 0, 0)) }"
"TransformM{ move(6, 22, 18) scale(0.5, 1, 0.5) }"
A component to load ESM content files with limited support for record types and
selection which of them to load. Supported record types are:
ACTI, CELL, CONT, DOOR, GMST, LAND, STAT.
==16218== Conditional jump or move depends on uninitialised value(s)
==16218== at 0xCDBC27: MWPhysics::Object::commitPositionChange() (object.cpp:68)
==16218== by 0xCE5B64: MWPhysics::PhysicsTaskScheduler::updatePtrAabb(std::shared_ptr<MWPhysics::PtrHolder> const&) (mtphysics.cpp:446)
==16218== by 0xCE5CC3: operator() (mtphysics.cpp:432)
==16218== by 0xCE5CC3: for_each<std::_Rb_tree_const_iterator<std::shared_ptr<MWPhysics::PtrHolder> >, MWPhysics::PhysicsTaskScheduler::updateAabbs()::<lambda(const std::shared_ptr<MWPhysics::PtrHolder>&)> > (stl_algo.h:3820)
==16218== by 0xCE5CC3: MWPhysics::PhysicsTaskScheduler::updateAabbs() (mtphysics.cpp:431)
==16218== by 0xCE5E35: MWPhysics::PhysicsTaskScheduler::afterPreStep() (mtphysics.cpp:586)
==16218== by 0xCE6238: operator() (mtphysics.cpp:533)
==16218== by 0xCE6238: wait<MWPhysics::PhysicsTaskScheduler::doSimulation()::<lambda()> > (barrier.hpp:33)
==16218== by 0xCE6238: MWPhysics::PhysicsTaskScheduler::doSimulation() (mtphysics.cpp:533)
==16218== by 0xCE6377: MWPhysics::PhysicsTaskScheduler::worker() (mtphysics.cpp:464)
==16218== by 0x74523C3: execute_native_thread_routine (thread.cc:82)
==16218== by 0x76FF258: start_thread (in /usr/lib/libpthread-2.33.so)
==16218== by 0x78155E2: clone (in /usr/lib/libc-2.33.so)
==16218==
==16218== Conditional jump or move depends on uninitialised value(s)
==16218== at 0xCDBC30: MWPhysics::Object::commitPositionChange() (object.cpp:73)
==16218== by 0xCE5B64: MWPhysics::PhysicsTaskScheduler::updatePtrAabb(std::shared_ptr<MWPhysics::PtrHolder> const&) (mtphysics.cpp:446)
==16218== by 0xCE5CC3: operator() (mtphysics.cpp:432)
==16218== by 0xCE5CC3: for_each<std::_Rb_tree_const_iterator<std::shared_ptr<MWPhysics::PtrHolder> >, MWPhysics::PhysicsTaskScheduler::updateAabbs()::<lambda(const std::shared_ptr<MWPhysics::PtrHolder>&)> > (stl_algo.h:3820)
==16218== by 0xCE5CC3: MWPhysics::PhysicsTaskScheduler::updateAabbs() (mtphysics.cpp:431)
==16218== by 0xCE5E35: MWPhysics::PhysicsTaskScheduler::afterPreStep() (mtphysics.cpp:586)
==16218== by 0xCE6238: operator() (mtphysics.cpp:533)
==16218== by 0xCE6238: wait<MWPhysics::PhysicsTaskScheduler::doSimulation()::<lambda()> > (barrier.hpp:33)
==16218== by 0xCE6238: MWPhysics::PhysicsTaskScheduler::doSimulation() (mtphysics.cpp:533)
==16218== by 0xCE6377: MWPhysics::PhysicsTaskScheduler::worker() (mtphysics.cpp:464)
==16218== by 0x74523C3: execute_native_thread_routine (thread.cc:82)
==16218== by 0x76FF258: start_thread (in /usr/lib/libpthread-2.33.so)
==16218== by 0x78155E2: clone (in /usr/lib/libc-2.33.so)
==16218==
We currently build a large map of a BSAFile's contents unused by Open MW. We already map archive contents in VFS. With this PR we remove the map from BSAFile and reimplement its only current use in BSATool.
To reduce amount of computations on the caller side and restrict possible
values.
* verts can't be non-int because it's a number of things.
* worldsize is initially defined as int by ESM::Land::REAL_SIZE.
* Put function to calculate heightfied shift into components to be able to
reuse by other binaries.
We just can no longer use the former ptr.getTypeName() as a fallback, but I do not suppose it matters. This value was an implementation defined string that usually contained a garbled mess.
Probably we should additionally refactor Lua types completely now that we have Ptr::getType(). I will leave this up to MWLua developers.
Currently, we use a peculiar mapping of ESM classes by their std::type_info::name. This mapping is an undefined behaviour because std::type_info::name is strictly implementation defined. It could return a non-unique value on some platforms. With this PR we use the unsigned int sRecordId of the ESM class as a more efficient lookup type that does not build on undefined behaviour. We can expect marginally faster save-game loading with these changes as well.
Currently, we always traverse the scene graph an additional time with a ComputeLightSpaceBounds visitor during shadow casting. ComputeLightSpaceBounds is only useful when the shadow casting mask allows us to shrink the bounds of the rendered scene, so we guard its traversal with a check against getCastsShadowTraversalMask. In practice, this guard never works because we build the traversal mask inclusively.
With this PR we limit the getCastsShadowTraversalMask check to relevant masks. This new check allows us to skip a superfluous ComputeLightSpaceBounds traversal with most settings.
We now use PositionAttitudeTransform for unmerged nodes because I have been informed PositionAttitudeTransform's scene graph performance is measurably faster than MatrixTransform's. We still need to use MatrixTransform for merged nodes because the Optimizer has limited support for non-MatrixTransform nodes. These MatrixTransforms will be removed in the merging process anyway.
Tests which no longer work are commented out.
Some of these don't work because they're effectively testing for the
presence of bugs in the old implementation.
Others don't work because we're no longer accidentally disabling the
boost::program_options feature where it generates an error if only part
of a token gets consumed.
These will be fixed by later commits.
This is the value used by vanilla Morrowind engine. At this angle player
character starts sliding down the slope.
This fixes navigation and movement issue in Mournhold, Godsreach.
For some reason we have commented std::move keywords in keywordsearch.hpp while they are quite beneficial. openmw_test_suite for keywordsearch.hpp takes 30% less time with these changes.
As we discovered in #3148, `Transform` nodes and their low level equivalence `pushModelViewMatrix` are somewhat costly involving a `Matrix::invert` operation per frame.
With this PR we avoid one `Transform` node for sun flashes and avoid another `pushModelViewMatrix` call in case the sun is fully visible.
With this PR we test out osg's shader define system for a somewhat harmless feature. As we can see, our code becomes more concise and efficient in this case. Most importantly, we no longer create unneeded vertex shader objects.
* uses a bitfield in refdata.hpp
With this simple change we pack boolean values to reduce the memory requirements of game objects.
* refdata.hpp [ci skip]
* refdata.cpp
