obsproject
Fix OnAcceleratedPaint2() not being called when there's no new texture
Description
This pull request fixes libcef_dll wrapped calls of OnAcceleratedPaint2() failing parameter checks if shared_handle is null even when new_texture is false (where this is expected). It's pretty much just adjusting the check to how it was in the initial on-accelerated-paint2 branch.
Motivation and Context
Without this change, OnAcceleratedPaint2() is only called when the texture changed. This currently does not appear to be a problem for obs-browser, admittedly, but since it silently fails in release builds and for the sake of correctness, I believe it's worth fixing. It might also help in the future when there's going to be some kind of compositing going on (like overlaying PET_POPUP elements as there's a todo-comment for).
And while I don't expect this to be intended or supported use in any way, I've been making use of this CEF fork in my own project. If possible I'd like to avoid maintaining a fork on top of a fork for a couple of changed lines, but I realize I'm on my own making use of your work here.
How Has This Been Tested?
I've tested the change on Windows 10 with obs-browser, as well as in my own project (Desktop+ Browser), by checking if the browser output still works while making sure hardware acceleration and OnAcceleratedPaint2() is being used.
As there are no functional changes for obs-browser, there's not much of an effect there right now, except that OnAcceleratedPaint2() gets called for every frame update again (but always just returns when new_texture is false right now).
Types of changes
- Bug fix (non-breaking change which fixes an issue)
Checklist:
- [ ] My code has been run through clang-format. (I assume that wouldn't apply to CEF's code?)
- [x] I have read the contributing document.
- [x] My code is not on the master branch.
- [x] The code has been tested.
- [x] All commit messages are properly formatted and commits squashed where appropriate.
- [x] I have included updates to all appropriate documentation. (not that there is any)
I'm personally not opposed to merging this if this'd definitely make the PET_POPUP composition easier to do, however it's worth noting that 5060 is likely the last build that'll support using this renderer, so this code will likely be short-lived.
If you don't mind me asking, what makes it a problem in your case?
Performance and frame pacing. In my case the application is a SteamVR overlay, typically running alongside a VR game. In this scenario I don't control the final output, but instead just submit a texture to the SteamVR compositor whenever I feel like it. Ideally that is only when there's a change in the browser output as there's performance overhead with each submission (it doesn't use the shared texture directly but copies it).
Then there's the frame pacing part. Chromium does a good job at not rendering when not necessary, so an idle page causes zero updates and playing a 24 fps video for example results in 24 updates per second. The VR headsets my applications runs with can have a variety of refresh rates too (60, 72, 80, 90, 120, 144 Hz are typical). Submitting the shared texture at fixed intervals works, but is wasteful and can miss its mark. To make matters worse, on the SteamVR-end there's no option to 100% sync frame timing with the compositor as an overlay app (the VR foreground apps/games have that of course). Using OnAcceleratedPaint2() as a signal for a new frame to process is ideal and seems like it'd also be intended. I'm fairly new to CEF dev but I don't think there's even another way to get that info either?
I'm not too familiar with OBS' internals, but from what I gathered glancing around it's fine taking the shared texture when it needs it when compositing the scene, so it doesn't need to care when updates to the texture happen compared to my app.
To clarify again, this worked fine in the initial OnAcceleratedPaint2() patch for CEF 4638, so this pull request merely restores the check to be the same as on that branch (I assume the adjustment of the generated library wrapper was overlooked this time). I just ran into this when trying to upgrade the CEF version on my end when 5060 was patched for OBS.