gamescope icon indicating copy to clipboard operation
gamescope copied to clipboard

Make KMS "rotation" prop optional for non-libliftoff codepath

Open emersion opened this issue 4 years ago • 3 comments

Commit https://github.com/Plagman/gamescope/commit/8782dc2cd110ef169e7d5abf5e40c94763bf7a05 made the KMS "rotation" property mandatory. However some drivers don't support it: https://drmdb.emersion.fr/properties/4008636142/rotation

libliftoff can deal with this already: https://github.com/emersion/libliftoff/blob/bc0d0617acdd48e69272771237cad2684c07c901/plane.c#L245

However the "basic" codepath doesn't have this fallback.

  • If the "rotation" prop exists, set it.
  • If gamescope is rotated but the "rotation" prop doesn't exist, error out.
  • If gamescope is not rotated and the "rotation" prop doesn't exist, ignore.

emersion avatar Sep 03 '21 06:09 emersion

Hi @emersion and Gamescope maintainers,

I'm interested in working on this issue from 2021. After examining the current codebase, I see that:

  1. The original src/drm.cpp mentioned in the issue has been refactored into src/Backends/DRMBackend.cpp
  2. The rotation handling logic now appears around line 161 in DRMBackend.cpp:
    if ( pPlane->GetProperties().rotation )
        pPlane->GetProperties().rotation->SetPendingValue( req, DRM_MODE_ROTATE_0, true );
    

How ever I need to understand:

  1. Where is the rotation requirement determined in the current codebase?
  2. Is this still an issue with modern DRM drivers?
  3. Should I update the fix for the current architecture? I'd like to work on updating this fix for the current codebase. Any guidance on whether this is still relevant and where to look for the rotation requirement logic would be helpful.

Julioccps avatar Dec 05 '25 20:12 Julioccps

CONFIRMED: This issue still exists in current codebase (December 2025).

I've verified that src/drm.cpp was renamed to src/Backends/DRMBackend.cpp in commit 1ec6f63e without any content changes (diff -u shows identical files).

The problematic code is at src/Backends/DRMBackend.cpp:161-165:

if ( pPlane->GetProperties().rotation )
    pPlane->GetProperties().rotation->SetPendingValue( req, DRM_MODE_ROTATE_0, true );
// Missing: Check if rotation is actually needed before failing

I'm working on a fix now. To implement correctly, I need to understand where the desired rotation value is determined for the non-libliftoff path. libliftoff uses ulOrientation, but where does the DRM backend get its rotation requirement?

If anyone can point me to the rotation state management, I'll implement the fallback logic. Otherwise, I'll proceed with finding it through code exploration.

Julioccps avatar Dec 05 '25 20:12 Julioccps

Please tell me how you are hitting an issue here, we check for "rotation"'s existence now.

misyltoad avatar Dec 05 '25 21:12 misyltoad