Not sure if noop is a good behavior. Placing a node above/below itself doesn't make sense in the first place, so I'd rather add a node != sibling assertion instead.

Not sure if noop is a good behavior. Placing a node above/below itself doesn't make sense in the first place, so I'd rather add a node != sibling assertion instead.

The motivation is to make this tinywl code from #3250 simpler and avoid this special case:

As the order of nodes is modified with "place above/below" operations, placing a node on the top is a special case anyway. IMO, it wouldn't hurt to be a bit explicit and have a check like "does this node need to be moved in the first place?", maybe with a scene_node_raise() function.

That's just my personal preference though. At the very least, your proposed behavior probably needs to be documented in wlr_scene.h.

Barely related, but wl_subsurface.place_{above,below} with the obejct itself as sibling is a protocol error.

Yeah, I think I'd agree with @vyivel here. We should introduce a way to place on top or at the bottom, because that's a pretty common thing to do for compositors. But in general compositors either want to place above/below or on top/at the bottom -- ie, having functions which do both doesn't make it easier to write compositors.

Ok, updated place above/below to assert that the nodes are distinct and added API to place a node on the top/bottom. I think you two are right and making the operation of placing a node above itself an assertion failure is a good idea.

Renamed the new functions to wlr_scene_node_place_on_top and wlr_scene_node_place_on_bottom. This should be ready to go now barring further feedback.