I do not think there was an actual error with AtomicFreeList, however
TSAN detected a data race when concurrent pop() happened. In that case,
there is indeed a race, where we can end-up reading data that is
already corrupted by the concurrent pop. However, that situation is
corrected by the following CAS. Somehow TSAN didn't see that.
The fix is strange and consists in replacing:
```
auto pNext = storage[offset].next;
```
with
```
auto s = storage[offset];
auto pNext = s.next;
```
In this PR we also adjust the memory ordering to be less strong. i.e.
we do not need `memory_order_seq_cst`, only the appropriate acquire or
release semantic.
In addition we also make `Node* next` a non-atomic variable again. It
should have been, but was change to placate an older version of TSAN.
BUGS=[377369108]
Previously, we linked against libOSMesa through the linker and
then used dlopen to link against libGL. This is not how libOSMesa
is intended to be used. Instead, we use dlopen on libOSMesa
(via bluegl), which then will map the correct GL methods for us,
and for the OSMesa functions, we also map those functions
from libOSMesa (instead of relying on compile-time linker).
- Both Scene and IBL are holding on to a skybox reference. We
need to make sure the order they are destroyed in right order.
- Reloading IBL should trigger resetting the indrect light in
gltf_viewer.
default parameters were not initialized which could cause them to
be incorrectly evaluated in the shader. this is actually a pretty
crazy bug that has been around since forever and which we were
"lucky" to not run into sooner.
this was exposed with the specular extension that was added recently.
Print a warning in case the sterescopic type in a compiled package is
different than what's in the engine's setting. The application may
proceed, but it could end up visual glitches when enabling stereoscopic
rendering.
This requires the stereoscopic type to be written into the package,
which needs a material version bump.
this change shouldn't have any impact on ARM, however, according
to cppreference it's not safe to mix seq_cst with other memory
orders:
"as soon as atomic operations that are not tagged memory_order_seq_cst
enter the picture, the sequential consistency guarantee for the program
is lost"
Value of mCgltfBuffersLoaded is sometimes retained across creation of FAssetLoader which skips loading the buffer in AssetLoaderExtended#createPrimitive leading to null pointer crash
if it can't find a name for a node, it will revert to the config's
defaultNodeName, however, if that is nullptr also, a crash will occur.
so we provide a last-resort hardcoded name in that case.
* improve parallel_for a bit
We get about 40% performance increase. The gain comes from not having
to copy the JobData structure each time we create a job, by using a
new emplaceJob() method, we can create the structure directly into
its destination.
* avoid calling wakeAll() when possible
wakeAll() is very expensive and not always needed when a job finishes
because there may not be anyone waiting on that job.
We now maintain a waiter count per job, and use that to determine if
we need to notify or not.
And now that the JobSystem overhead is lower, we can decrease the size
of the jobs, which improves the load balancing.
* mActiveJobs fixes
some comments claimed mActiveJobs needed to be modified before or after
accessing the WorkQueue; this couldn't be correct because there were no
guaranteed global ordering with the workQueue.
- reduce the number of calls to notify_one() and notify_all().
notify_one() is not only called when running a new job, and
notify_all() only when a job finishes.
- don't hold the condition lock while calling notify_*(), as it is not
strictly needed, and because notify_*() can be very slow, there can
be a lot of contention on this lock as a result; blocking the whole
jobsystem thread pool.
- add a new version of run() that takes an opaque thread id that can
be retrieved from a job's execute function; this is especially
intended to be used by parallel_for(); it's just a more efficient
version of run() that avoids a hashmap lookup.
Overall these change yield a significant performance boost:
- running + waiting a job: +200%
- running many jobs: +150%
- running many jobs in parallel: +50%
- remove deprecated morphing APIs
- repair gltfio, samples and tests
The new API doesn't allow a MorphTargetBuffer per RenderPrimitive,
instead the MorphTargetBuffer is specified per Renderable.
gltfio separates RenderPrimitives from Renderables, in particular all
RenderPrimitives are created before their Renderable; this was
problematic for this change because all primitives must share
a single MorphTargetBuffer living in the Renderable.
To fix this, we're no longer initializing the morphing paramters
at RenderPrimitive creation, instead we store a reference to the
BufferSlot in the Primtive structure, so that later, when the Renderable
is created we can finally retrieve the BufferSlot and initialize its
morphing paramters, which are not available. The "morphing parameters"
are now expanded to contain the MorphTargetBuffer as before (except now
it's always the same for all the primitives of a Rendrable), as well
as the offset within the buffer and the vertex count.
The number of SH bands used for the indirect light irradiance
computations can be set to 1, 2 or 3 (default) in Material::Builder.
For e.g. in lower-end devices w/ non HDR content, it might be
beneficial to set this value to 2.
BUGS=[341971013]
