cog: Fix wrong var in use to add the selected platforms #448
Conversation
|
Could you elaborate what component uses graphene causing the alignment fault? At least for GStreamer's gltransformation, this should be fixed by https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/1321. I'm running the WPE port from latest WebKit main and recent cog/libwpe/wpebackend-fdo on ARM 32 bit devices and I have not seen this since. |
The crash came out by using the We reached to thissolution through ebassi/graphene#215 (comment) |
|
Ah, I see. Thanks for the explanation. |
|
Is it possible to disable the NEON instructions only if the gtk platform is enabled? I think it's a bit unfortunate that all graphene users get their NEON instructions disabled, even if the affected gtk platform is not even built. |
I don't see a way of how an flag in one component ( |
Unfortunately, the distro features are more about the stack itself (wayland, x11, 3g ... ) than for a particular widget toolkit. I am going to do an explicit mention here to @kraj who time to time collaborated in this repo and it has by far much more experience in poky and meta-oe. For sure he can give a good suggestion. |
We do not have UI toolkits as distro level knobs, but loosely inferred so perhaps we could do a and then it could be tied to a but if we are needing it without X11 too then we might just keep it a packageconfig and turn it on and off from |
| @@ -0,0 +1,4 @@ | |||
| # Disable ARM neon support. Backported from Arch Linux: https://github.com/archlinuxarm/PKGBUILDs/pull/1977 | |||
| # Upstream-status: Reported [https://github.com/ebassi/graphene/issues/215] | |||
| EXTRA_OEMESON:append:arm = " -Darm_neon=false" | |||
There was a problem hiding this comment.
Does :arm applies both to Aarch64 and ARMv7 or only to ARMv7?
If the crash only happens on ARMv7 i think it will make sense to only disable it there
There was a problem hiding this comment.
Just checked it. It only seems to apply to ARM-32 bits. Good
There was a problem hiding this comment.
But this still means we disable graphene NEON instructions unconditionally for everybody on ARMv7, regardless of whether webkitgtk is built or not. I think that's bad style.
There was a problem hiding this comment.
I understand your point but see a clear path to follow right now. It sounds like distros like Arch and ReHat just disabled NEON on graphene package for ARM: archlinuxarm/PKGBUILDs#1977. If this is too much aggressive or not I don't have a strong enough argument to take position in one or another side.
There was a problem hiding this comment.
But this still means we disable graphene NEON instructions unconditionally for everybody on ARMv7, regardless of whether webkitgtk is built or not. I think that's bad style.
The issue not only affects WebKitGTK. It also affects WPE when using the GTK4 plugin of Cog.
Graphene seems to be used by the gstreamer-gl plugin. It is needed to build the gltransformation and glvideoflip elements.
So removing NEON support on Graphene may impact performance (would like to see some benchmarks) when using gltransformation, glvideoflip elements inside the gstreamer pipeline.
Other than that, I'm not aware of any usage of Graphene when using WPE that may be impacted by this change.
Do you have any example of use case that will be impacted?
We have to make a difficult choice here, and I will rather have every ARMv7 user of WPE+Cog/GTK4 and WebKitGTK/GTK4 with a working build than a hypothetical performance improvement when playing videos with WPE/WebKitGTK with some non-standard configuration.
Ideally I would like to fix this without having to disable NEON support on Graphene. If you have any suggestion on how to do that, then please share it.
There was a problem hiding this comment.
Ah, someone else posted the graphene approach already upstream: https://lore.kernel.org/openembedded-core/[email protected]/T/#u
Good. If this gets merged in oe-core then we don't have to do anything here.
But the end result will be the same: NEON support on Graphene will get disabled by default for all ARMv7 users
There was a problem hiding this comment.
The build instructions could then mention:
On ARMv7 WebKitGTK and cog GTK4 plugin users should disable NEON instructions in their local.conf (see bug: FIXME)
PACKAGECONFIG:remove:pn-graphene = "neon"
I agree with the PACKAGECONFIG option so it is possible to enable or disable it on local.conf
But I think things should work by default, so I would default it to be disabled and then add a note at https://github.com/Igalia/meta-webkit/wiki/PerformanceTips about enabling it for users that don't care about GTK
There was a problem hiding this comment.
Right. We could still add a notice that non-gtk users on ARMv7 might be able to add PACKAGECONFIG:append:pn-graphene = "neon". But OTOH that's not really meta-webkit exclusive.
There was a problem hiding this comment.
But OTOH that's not really meta-webkit exclusive.
Agreed. On top of that I'm a bit skeptical regarding whether enabling Neon support for Graphene has any noticeable performance improvement on WPE when playing videos with GStreamer. Have you tested it?
There was a problem hiding this comment.
Fixed on Yocto Poky mickledore: https://lists.openembedded.org/g/openembedded-devel/message/100996
|
It landed in poky this commit: yoctoproject/poky@491a323 It should be part of Yocto Mickledore (to be released in April 2023). In any case, I guess it is an option to add here a .bbappend with similar contents |
psaavedra
force-pushed
the
psaavedra-fixes-cog-gtk4
branch
from
e380845 to
99bd199
Compare
Let's keep this discussion just as reference anyone facing this problem could take the decision by their own. I've remove the append and I will merge the original commit what actually it is a bug in meta-webkit. |
Also: