Core: Add gpu rendering support

[vaxerski]

Adds support (enabled by default) for gpu rendering

Bugs:

• dynamic wallpaper changes are black
• there is a leak somewhere in preload (tho its also there on main I believe)

[vaxerski]

@fufexan nix fix plx

[vaxerski]

@gulafaran you did some memory leak analysis, I can't get valgrind or lsan to run on this, any ideas to track down the leak?

[gulafaran]

il poke around tomorrow and see if i can get it to find something useful

[vaxerski]

thanks bae

[gulafaran]

here is the temp changes i make to get asan to produce something.

diff --git a/CMakeLists.txt b/CMakeLists.txt
index a398016..18271fd 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -121,7 +121,8 @@ set(CPACK_PROJECT_NAME ${PROJECT_NAME})
 set(CPACK_PROJECT_VERSION ${PROJECT_VERSION})
 include(CPack)
 
-target_link_libraries(hyprpaper PkgConfig::deps)
+target_link_libraries(hyprpaper -fsanitize=address,undefined PkgConfig::deps)
+target_compile_options(hyprpaper PUBLIC -fsanitize=address,undefined)
 
 target_link_libraries(
   hyprpaper
diff --git a/src/Hyprpaper.cpp b/src/Hyprpaper.cpp
index d0bddb6..ce7ac31 100644
--- a/src/Hyprpaper.cpp
+++ b/src/Hyprpaper.cpp
@@ -134,7 +134,8 @@ void CHyprpaper::init() {
 
     tick(true);
 
-    while (1) {
+    int count = 0;
+    while (count < 5) {
         int ret = poll(pollFDs, 2, 5000 /* 5 seconds, reasonable. Just in case we need to terminate and the signal fails */);
 
         if (ret < 0) {
@@ -174,6 +175,7 @@ void CHyprpaper::init() {
                 wl_display_flush(m_sDisplay);
             } while (ret2 > 0);
         }
+        count++;
     }
 
     unlockSingleInstance();

this PR has a new issue on destruction

/home/tom/dev/hyprpaper/src/render/Buffer.cpp:180:13: runtime error: member call on null pointer of type 'CEGL'
    #0 0x6b660e in CBuffer::destroyGpu() /home/tom/dev/hyprpaper/src/render/Buffer.cpp:180:13
    #1 0x6b6fd8 in CBuffer::~CBuffer() /home/tom/dev/hyprpaper/src/render/Buffer.cpp:199:9
    #2 0x70df21 in std::default_delete<CBuffer>::operator()(CBuffer*) const /usr/lib/gcc/x86_64-pc-linux-gnu/14/include/g++-v14/bits/unique_ptr.h:93:2
    #3 0x70dcd9 in Hyprutils::Memory::CSharedPointer_::impl<CBuffer>::_destroy() /usr/include/hyprutils/memory/./SharedPtr.hpp:69:21
    #4 0x70d4ed in Hyprutils::Memory::CSharedPointer_::impl<CBuffer>::destroy() /usr/include/hyprutils/memory/./SharedPtr.hpp:103:21
    #5 0x6183d5 in Hyprutils::Memory::CSharedPointer<CBuffer>::destroyImpl() /usr/include/hyprutils/memory/./SharedPtr.hpp:279:24
    #6 0x61822b in Hyprutils::Memory::CSharedPointer<CBuffer>::decrement() /usr/include/hyprutils/memory/./SharedPtr.hpp:265:21
    #7 0x617ee5 in Hyprutils::Memory::CSharedPointer<CBuffer>::~CSharedPointer() /usr/include/hyprutils/memory/./SharedPtr.hpp:177:17
    #8 0x617e89 in std::pair<SMonitor* const, Hyprutils::Memory::CSharedPointer<CBuffer>>::~pair() /usr/lib/gcc/x86_64-pc-linux-gnu/14/include/g++-v14/bits/stl_iterator.h:3001:12


SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /home/tom/dev/hyprpaper/src/render/Buffer.cpp:180:13 
/home/tom/dev/hyprpaper/src/render/Egl.cpp:160:12: runtime error: member call on null pointer of type 'CEGL *'
    #0 0x6c6358 in CEGL::destroyEglImage(void*) /home/tom/dev/hyprpaper/src/render/Egl.cpp
    #1 0x6b6674 in CBuffer::destroyGpu() /home/tom/dev/hyprpaper/src/render/Buffer.cpp:180:13
    #2 0x6b6fd8 in CBuffer::~CBuffer() /home/tom/dev/hyprpaper/src/render/Buffer.cpp:199:9
    #3 0x70df21 in std::default_delete<CBuffer>::operator()(CBuffer*) const /usr/lib/gcc/x86_64-pc-linux-gnu/14/include/g++-v14/bits/unique_ptr.h:93:2

im gonna have to hunt down a recent mesa shader cache fd leak before i dig any deeper but feel free to asan it you too :)

[gulafaran]

there is also

hyprpaper/src/render/LayerSurface.cpp

Line 38 in d82aec0

pLayerSurface->sendSetAnchor((zwlrLayerSurfaceV1Anchor)(ZWLR_LAYER_SURFACE_V1_ANCHOR_TOP | ZWLR_LAYER_SURFACE_V1_ANCHOR_RIGHT | ZWLR_LAYER_SURFACE_V1_ANCHOR_BOTTOM |

, casting the enum to the value 15.

enum zwlrLayerSurfaceV1Anchor : uint32_t {
    ZWLR_LAYER_SURFACE_V1_ANCHOR_TOP = 1,
    ZWLR_LAYER_SURFACE_V1_ANCHOR_BOTTOM = 2,
    ZWLR_LAYER_SURFACE_V1_ANCHOR_LEFT = 4,
    ZWLR_LAYER_SURFACE_V1_ANCHOR_RIGHT = 8,
};

[vaxerski]

enum casting out of range is a classic in wayland I don't think we gon' escape that one.

Is it still UB even though we define it as uint32_t?

[gulafaran]

enum casting out of range is a classic in wayland I don't think we gon' escape that one.

Is it still UB even though we define it as uint32_t?

well ZWLR_LAYER_SURFACE_V1_ANCHOR_TOP | ZWLR_LAYER_SURFACE_V1_ANCHOR_RIGHT | ZWLR_LAYER_SURFACE_V1_ANCHOR_BOTTOM | ZWLR_LAYER_SURFACE_V1_ANCHOR_LEFT as in 1 + 8 + 2 + 4 = 15, cast uint32_t value 15 to (zwlrLayerSurfaceV1Anchor) enum but it has no member with value 15, i would assume its all sorts of UB. i wonder how kwin deals with things like this since its also c++ based hm. does it just send and deal with uint32 directly and parse the values to either single members depending on value or multiple if no member has the value? are the protocols really sending enums or uint32_t's? i suspect uint32_t without reading on it which mean its an issue on "our" side in parsing things and not in the protocol

[gulafaran]

for example in wlroots,

struct wlr_layer_surface_v1_state {
	uint32_t committed; // enum wlr_layer_surface_v1_state_field

	uint32_t anchor;

static void layer_surface_exclusive_zone(
		struct wlr_layer_surface_v1_state *state,
		struct wlr_box *usable_area) {
	switch (state->anchor) {

static void layer_surface_handle_set_anchor(struct wl_client *client,
		struct wl_resource *resource, uint32_t anchor) {
	const uint32_t max_anchor =
		ZWLR_LAYER_SURFACE_V1_ANCHOR_TOP |
		ZWLR_LAYER_SURFACE_V1_ANCHOR_BOTTOM |
		ZWLR_LAYER_SURFACE_V1_ANCHOR_LEFT |
		ZWLR_LAYER_SURFACE_V1_ANCHOR_RIGHT;

    if (anchor > max_anchor) {

it seems to be dealing with uint32_t all over and just using the enum values for references to what is what. it never casts it out of range that is

[gulafaran]

in short, im not fully read up how things are built up to a working source but i suspect hyprwayland-scanner builds the protocol things from the .xml files. perhaps it should just use the datatype of the protocol as parameters to the functions.

<request name="set_anchor">
      <description summary="configures the anchor point of the surface">
        Requests that the compositor anchor the surface to the specified edges
        and corners. If two orthoginal edges are specified (e.g. 'top' and
        'left'), then the anchor point will be the intersection of the edges
        (e.g. the top left corner of the output); otherwise the anchor point
        will be centered on that edge, or in the center if none is specified.

        Anchor is double-buffered, see wl_surface.commit.
      </description>
      <arg name="anchor" type="uint" enum="anchor"/>
    </request>

and on the sending and recieving side you still use uint32_t and just check its values if they are within range of the enum or exact values of the enum. and the enum decides what values does what or not. instead of trying to cast the enum to a value and send that as a parameter and as a consequence UB and out of range casting when all it does is technically send uint32_t.

just an idea from a random newb. :D , god i suck at trying to explain what im thinking too.

[vaxerski]

hw-s puts the enums there so that you know what it is. In actuality, it's just used as an uint32_t.

[gulafaran]

hw-s puts the enums there so that you know what it is. In actuality, it's just used as an uint32_t.

but what is creating sendSetAnchor and its parameters?

[gulafaran]

hw-s puts the enums there so that you know what it is. In actuality, it's just used as an uint32_t.

but what is creating sendSetAnchor and its parameters?

because thats what we should be changing to uint32_t instead.

like

wlr-layer-shell-unstable.hpp

-void sendSetAnchor(zwlrLayerSurfaceV1Anchor);
+void sendSetAnchor(uint32_t);

wlr-layer-shell-unstable.cpp

-void CCZwlrLayerSurfaceV1::sendSetAnchor(zwlrLayerSurfaceV1Anchor anchor) {
+void CCZwlrLayerSurfaceV1::sendSetAnchor(uint32_t anchor) {
    if (!pResource)
        return;

    auto proxy = wl_proxy_marshal_flags((wl_proxy*)pResource, 1, nullptr, wl_proxy_get_version((wl_proxy*)pResource), 0, anchor);
    proxy;
}

wl_proxy_marshal_flags already takes an uint32_t as its flags.

meaning in layersurface.cpp it can use

pLayerSurface->sendSetAnchor((ZWLR_LAYER_SURFACE_V1_ANCHOR_TOP | ZWLR_LAYER_SURFACE_V1_ANCHOR_RIGHT | ZWLR_LAYER_SURFACE_V1_ANCHOR_BOTTOM | ZWLR_LAYER_SURFACE_V1_ANCHOR_LEFT));

and then there is no casting out of range. same deal with all the out of casting enums in hyprland and the other tools.

[vaxerski]

but then you could mistakenly take the wrong enum

[gulafaran]

but then you could mistakenly take the wrong enum

well imo rather that then UB, one of the key issues with UB is it can show itself just as in recent conversations in hyprwm/Hyprland#6608 "working" in debug builds but as fast as higher optimisation levels are applied the compiler just strips out things that should be there. and crashes on release builds. or even cause really wonky memory issues. unless you got any other ideas i would go with the route of maybe wrong values instead of relying on chance the compiler gets it somewhat right ("it usually doesnt") , in this case who knows what it does its undefined after all but what if its doing anything like this famous horror example https://godbolt.org/z/3Pqojanqo eraseall is never called and yet it is.

implentation bugs versus language/compiler bugs sort of. one is solveable one isnt

[vaxerski]

lmao the linked one is indeed a disaster

I don't think the compiler is allowed to optimize here as we added : uint32_t, but I could just change the enum types to uint32_t I guess.

[vaxerski]

pushed to hw-s

[vaxerski]

hyprwm/hyprwayland-scanner@ce8dae8

[gulafaran]

hyprwm/hyprwayland-scanner@ce8dae8

cool :) now just a few minor changes in the hyprwm land then. heres hyperpaper., il see if i got the time to dig around the other tools tomorrow for out of range casts. trying to repaint the house and they are showing good weather tomorrow so a bit busy with that too heh

diff --git a/src/Hyprpaper.cpp b/src/Hyprpaper.cpp
index d0bddb6..c31ca82 100644
--- a/src/Hyprpaper.cpp
+++ b/src/Hyprpaper.cpp
@@ -258,7 +258,7 @@ void CHyprpaper::recheckAllMonitors() {
 void CHyprpaper::createSeat(SP<CCWlSeat> pSeat) {
     m_pSeat = pSeat;
 
-    pSeat->setCapabilities([this](CCWlSeat* r, wl_seat_capability caps) {
+    pSeat->setCapabilities([this](CCWlSeat* r, uint32_t caps) {
         if (caps & WL_SEAT_CAPABILITY_POINTER) {
             m_pSeatPointer = makeShared<CCWlPointer>(m_pSeat->sendGetPointer());
             if (!m_pCursorShape)
diff --git a/src/render/LayerSurface.cpp b/src/render/LayerSurface.cpp
index e4c969d..36f288a 100644
--- a/src/render/LayerSurface.cpp
+++ b/src/render/LayerSurface.cpp
@@ -35,7 +35,7 @@ CLayerSurface::CLayerSurface(SMonitor* pMonitor) {
     }
 
     pLayerSurface->sendSetSize(0, 0);
-    pLayerSurface->sendSetAnchor((zwlrLayerSurfaceV1Anchor)(ZWLR_LAYER_SURFACE_V1_ANCHOR_TOP | ZWLR_LAYER_SURFACE_V1_ANCHOR_RIGHT | ZWLR_LAYER_SURFACE_V1_ANCHOR_BOTTOM |
+    pLayerSurface->sendSetAnchor((ZWLR_LAYER_SURFACE_V1_ANCHOR_TOP | ZWLR_LAYER_SURFACE_V1_ANCHOR_RIGHT | ZWLR_LAYER_SURFACE_V1_ANCHOR_BOTTOM |
                                                             ZWLR_LAYER_SURFACE_V1_ANCHOR_LEFT));
     pLayerSurface->sendSetExclusiveZone(-1);
 

[vaxerski]

no hurries, everything should still compile.

If you could just make MRs whenever you have a moment

[vaxerski]

I've dropped the commit as its a breaking change... we gotta figure out a different way.

One thing of note: doesn't wayland itself do this all the time?

[vaxerski]

hm, no, it uses uint32_t... I don't know how to make this non-breaking, though. Any ideas?

[vaxerski]

Is this actually UB? The values are fixed and specified as uint32_t.

[vaxerski]

Seems like we are not doing UB.

[gulafaran]

Seems like we are not doing UB.

Had to bring out the big ctrl + f in the c++ std

(...) If the enumeration type has a fixed underlying type, the value is first converted to that type by integral conversion, if necessary, and then to the enumeration type. If the enumeration type does not have a fixed underlying type, the value is unchanged if the original value is within the range of the enumeration values (9.7.1), and otherwise, the behavior is undefined.

Indeed seems c++ allow this as long as the type is fixed
with a few notes as mentioned in that picture, one learns everyday. Atleast looks like so

[vaxerski]

:D yours truly

[vaxerski]

I've verified the leak is not there on main

also @gulafaran do you have an idea why gbm_device_create ups the VmRSS like 120MB???

[gulafaran]

I've verified the leak is not there on main

also @gulafaran do you have an idea why gbm_device_create ups the VmRSS like 120MB???

tried fiddling with asan/valgrind im not sure why something sneaky is going on asan runs it but valgrind segfaults

 6 ==8165== Syscall param ioctl(DRM_VERSION).version_major points to unaddressable byte(s)
  7 ==8165==    at 0x5547766: ioctl (in /usr/lib64/libc.so.6)
  8 ==8165==    by 0x4929250: drmGetVersion (in /usr/lib64/libdrm.so.2.4.0)
  9 ==8165==    by 0x4902192: ??? (in /usr/lib64/libgbm.so.1.0.0)
 10 ==8165==    by 0x4901398: ??? (in /usr/lib64/libgbm.so.1.0.0)
 11 ==8165==    by 0x48FF3A7: ??? (in /usr/lib64/libgbm.so.1.0.0)
 12 ==8165==    by 0x48FF52C: gbm_create_device (in /usr/lib64/libgbm.so.1.0.0)
 13 ==8165==    by 0x450BD8: CRenderer::CRenderer(bool) (src/render/Renderer.cpp:149)
 14 ==8165==    by 0x4113B2: std::__detail::_MakeUniq<CRenderer>::__single_object std::make_unique<CRenderer, bool>(bool&&) (unique_ptr.h:107    6)
 15 ==8165==    by 0x40AA3B: CHyprpaper::init() (src/Hyprpaper.cpp:91)
 16 ==8165==    by 0x43FFD8: main (src/main.cpp:36)
 17 ==8165==  Address 0x1008f42bb0 is 0 bytes inside a block of size 80 in arena "core"

either im doing something wrong or something very wonky is going on heh

[vaxerski]

thats not my code :)

[gulafaran]

thats not my code :)

yeah i might be hitting some mesa-git funzies but asan doesnt report anything nasty and the gl/shader code is beyond my knowledge are they somehow doing something? like missing glDeleteProgram or glDetachShader

[vaxerski]

would be odd cuz we leaking ram not vram