Cef upgrade#3682
Conversation
…ew names in CEF, update function params, temporarily add additional debugging.
github-actions
Bot
added
the
triage
|
Lol sorry, I meant to keep this as a draft in my repository before submitting it to the main repository upon completion, my bad. Current issues:
And just to note, there's a bit of Git weirdness here. For example, I didn't change line 224-326 at |
|
@oracularhades I see that you've used CEF binaries from spotify, FiveM is using custom build of CEF, you should check these repositories first and merge these changes into new version: |
Yes I know, thanks 🙂 |
|
Please also compare the result of https://www.vsynctester.com/ for the current cef version and your upgrade and upload both videos here. |
|
@oracularhades do you have plan on finishing this PR? |
@Kr3mu Yes, definitely. I've just been looking for help with a specific CEF API. |
Thanks, You are the man who is going to save us |
|
@Kr3mu Oh hey! You made ESX! Great work :) |
thanks |
|
@oracularhades What's your discord I can't find it anywhere |
|
@Kr3mu "oracularhades" on Discord :) |
|
Any update |
Still debugging 🙂 - |
|
Any updates on this PR, would love to see it. |
|
@WolfsCoding Definitely still happening, just juggling a bit IRL right now 🙂. If anyone wants to contribute to the PR, I'd be happy to answer any questions about the PR. |
I’m not sure it’s worth spending time on this PR, as Ehbw mentioned that his upgrade is mostly complete. |
|
@ToffyMTA My PR uses a different method and I disagree with how the other PR is structured (respectfully, not wanting to disparage anyone 🙂). In Ehbw's PR merges are manually applied and changes are manually diverged, rather than being based on a fresh official CEF release, potentially creating stability/security gaps. I think my PR is better because it starts from the latest CEF build and will re-apply FiveM's changes. In a perfect world, I'd also like to compartmentalise FiveM's changes into specific areas to make upgrades a little easier for future maintainers. Ehbw's PR would require significantly more time to maintain in the future and I worry we'll have the same outdated CEF problem in the future.
(At the time of this PR, I will update the CEF release before merging) |
|
CFX depends on every single custom patch used. Without them we lose backwards compatability and fxdk functionality. Porting the patches from m103 -> m139 (which had over 2 years worth of patches and made all the patches unable to be automatically applied) took about an hour, with every subsequent update (m139 -> m140, m140 -> m144) taking less then 15 minutes to patch and 30 minutes to be in game on with my branch. However this is absolutely not the issue. CFX depended on OBS's shared texture patch which only works on m103 as m104 chromium removed a large part of the rendering code that OBS patched and relied on for OSR. CEF implemented a new OSR rendering system in M124, however this is simply a massive downgrade to the OBS one in every single way and is significantly slower and harder to work with in FiveM (and RedM + FxDK), it has massive performance issues with vSync and general rendering which will massively impact the end users in a very negative way. This is the main reason why my branch hasn't seen the light of day (I've had an experimental branch + experimental thread post in a draft since September) as without heavily patching CEF + Chromium which will make future upgrades significantly harder and more time consuming there is no fix to this problem. |
|
@Ehbw My concerns are how diverged the patches in your PR are from the official CEF repo. It's not based on an official CEF release but rather patches manually applied onto a release that is years out-of-date.
Yeah, I mentioned NUI's reliance on custom CEF implementations in my last comment. I think we're both saying very similar things. I have been facing the same shared textures issue, so we're currently both stuck in the same boat with regards to "This is the main reason why my branch hasn't seen the light of day" 😅.
I respectfully disagree with this. Headless Chromium has been re-done in CEF. My PR is updating FiveM's NUI to use the new headless textures rather than trying to recycle the older CEF versions. Heavily patching CEF/Chromium would inevitably unnecessary unmaintainable code with security flaws that go well outside the sanity of CEF or CFX's maintainers. |
|
I feel like you've misunderstood what I meant with the new implementation. I have fully adapted the fivem OSR logic to work with the new OSR system (both DUI and normal NUI). However the general performance and data sent (both frequency and content of the data) to the callback is much slower and inconsistent (e.g. frames being skipped especially on some heavy animated transitions where m103 fork had no issue with it) under the new OSR, along with just the new implementation for OnAcceleratedPaint either forcing the handle to be duplicated and passed to the render thread or OnAcceleratedPaint to be blocked until the texture is copied over to a resource owned by the games d3d11 device (which has to be done on the render thread) otherwise once the OnAcceleratedPaint callback returns the handle can be freed back to the pool immediately. These issues get worse under vsync with cef having issues about it in the cef repo. From testing with various users on m140 they faced performance issues or artifacting exclusive to having off screen rendering enabled. This can even be repro successfully outside of fivem entirely by using cefclient and passing it OSR flags and navigate to https://www.vsynctester.com/ and you can see the performance issue clearly compared to not using OSR. The git branches from my fork are temporary and are created from the upstream fork rather then off of the old fivem cef branches. Also something that isn't really an issue |
Thank you for clarifying. By "upstream fork" do you mean the latest CEF release or the m103 fork? Your insight into OSR performance is super interesting, thanks. I will continue this PR and keep debugging to see what can be done. I'm not sure heavily patching CEF/Chromium is the right approach. 2 competing PRs won't hurt anyone 🙂. I'd be interested to hear more about what you've learnt and see if there's anything we could come up with to get a CEF patch merged. My email is hi@oracularhades.com if you'd like to chat. |
|
@Ehbw I reread this thread after doing yet another week of digging and I see more of your point. I'm still very skeptical about heavily patching Chrome/CEF (which was my main point) as FiveM would essentially be maintaining it's own browser. Applying future chromium patches would be this problem on steroids. Maybe another browser could work? We could try firefox? It would be possible to add a flag in the resource manifest to either load CEF M103 or the new browser implementation, which would ensure backwards compatibility. |
|
Firefox doesn't have a CEF equivalent like Chromium but I think it's worth the contemplation if the discussion is at the point of maintaining a chromium fork. |
Not including the custom patches used in fivem and redm is an non-negotiable point as otherwise it will break how NUI and NUI resources work as a whole. The idea of heavily patching cef/chromium to improve OSR/rework OSR was more of highlighting how painful (and unlikely) a CEF upgrade would be. |
@Ehbw Sorry i misspoke, I do still mean including the custom FiveM code that already exists in the Fivem CEF repo. That is a lot easier to maintain than a Chromium fork. |
|
Superseded by another changeset targeting CEF. Thanks for your work here! |
8 participants
No description provided.