Native implementation of Engine.setHardwareScalingRatio #566
Conversation
…/BabylonNative into add-engine-hardware-scaling-level
Drigax
requested review from
bghgary,
syntheticmagus,
CedricGuillemet and
ryantrem
…/BabylonNative into add-engine-hardware-scaling-level
Co-authored-by: Ryan Tremblay <ryantrem@msn.com>
| m_state.Resolution.width = width; | ||
| m_state.Resolution.height = height; | ||
| } | ||
| UpdateBgfxResolution(); |
There was a problem hiding this comment.
nit: In both places where this is called, the state mutex is locked immediately before calling it, then that lock is abandoned, then it's locked again inside the call. If the contract for this method became that it's unsafe (maybe by putting it on the struct and/or calling it UnsafeUpdate...()?) then we could just move the call inside the scope of the outer lock and not need to lock, unlock, and relock. Honestly, I'm not sure about this one; the two locks feel unnecessary, but making a helper method which exists exclusively to modify mutex-guarded state, then making that helper method explicitly unguarded, also feels kind of weird. Do we know if it's meaningfully worse perf-wise to just leave in the two locks?
There was a problem hiding this comment.
I would just replace mutex with recursive_mutex which allows for re-entrancy.
There was a problem hiding this comment.
You could also use lock_guard instead of scoped_lock here. scoped_lock is just a helper for locking on multiple mutexes in an raii fashion, but we’re only locking on one mutex.
There was a problem hiding this comment.
good suggestion, that definitely removes the need for awkward block scoping to prevent re-locking the same mutex in the caller.
I'm going to keep with scoped_lock, I'm not sure if there's anything to gain in using the older lock implementation, I was under the impression that this supplanted lock_guard
This PR implements and adds boilerplate to expose NativeEngine.getHardwareScalingRatio.
Render scaling is implemented via a scalar used to resize bgfx's rendering resolution.
Addresses #482