Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

platform: Add DRMFormat descriptor #2358

Merged
merged 9 commits into from
Mar 11, 2022
Merged

platform: Add DRMFormat descriptor #2358

merged 9 commits into from
Mar 11, 2022

Conversation

RAOF
Copy link
Contributor

@RAOF RAOF commented Mar 8, 2022

Various parts of Mir code want to know various things about
pixel formats. The DRM fourcc format specifiers are reasonably
well described, in common use through Linux subsystems,
both kernel and userspace, and we already need to use them
in various parts of the codebase.

This adds some infrastructure for describing DRM formats.

RAOF added 6 commits March 8, 2022 13:57
@RAOF
platform: Add DRMFormat descriptor
997cb27 
Various parts of Mir code want to know various things about
pixel formats. The DRM fourcc format specifiers are reasonably
well described, in common use through Linux subsystems,
both kernel and userspace, and we already need to use them
in various parts of the codebase.

This adds some infrastructure for describing DRM formats.
@RAOF
gbm-kms/Display: Use DRMFormat infrastructure
df52c9f
@RAOF
cmake: Fix the build with g++-12
2dcca47
@RAOF
platform/DRMFormat: Hold FormatInfo directly
1d9c0dd 
This switches to a single lookup on construction (which also
verifies that we understand the format) instead of a lookup
per query.
@RAOF
cmake: Fix add_object_library_to_target()
d560927 
This wasn't correctly adding library dependencies - it would
add them to things that *linked* to the target, but not the
target itself.
@RAOF
platform/DRMFormat: Actually use drmGetFormatModifierName when available
98e9cab
@RAOF
Copy link
Contributor Author

RAOF commented Mar 8, 2022

There are obvious additions to this, in a number of directions. Adding YUV buffer formats, conversions to/from MirPixelFormat, pulling some of the distributed GL format information in, using DRMFormat in more places, and so on. I've not done that because a bunch of the affected code is going to be changed in new-platform-API.

@Saviq
Copy link
Collaborator

Saviq commented Mar 8, 2022

Bunch of compilers no like:

/<<PKGBUILDDIR>>/src/platform/graphics/drm_formats.cpp:410:14: error: call to non-‘constexpr’ function ‘void std::sort(_RAIter, _RAIter, _Compare) [with _RAIter = mir::graphics::DRMFormat::FormatInfo*; _Compare = ***anonymous***::make_sorted_formats_list()::<lambda(const auto:3&, const auto:4&)>]’
  410 |     std::sort(
      |     ~~~~~~~~~^
  411 |         local_formats.begin(), local_formats.end(),
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  412 |         [](auto const& a, auto const& b)
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  413 |         ***
      |         ~     
  414 |             return a.format < b.format;
      |             ~~~~~~~~~~~~~~~~~~~~~~~~~~~
  415 |         ***);
      |         ~~    
/<<PKGBUILDDIR>>/src/platform/graphics/drm_formats.cpp: At global scope:
/<<PKGBUILDDIR>>/src/platform/graphics/drm_formats.cpp:417:2: error: extra ‘;’ [-Werror=pedantic]
  417 | ***;
      |  ^

@AlanGriffiths
Copy link
Contributor

/<>/src/platform/graphics/drm_formats.cpp:410:14: error: call to non-‘constexpr’ function ‘void std::sort(_RAIter, _RAIter, _Compare) [with _RAIter = mir::graphics::DRMFormat::FormatInfo*; _Compare = anonymous::make_sorted_formats_list()::<lambda(const auto:3&, const auto:4&)>]’
410 | std::sort(
| ~~~~~~~~~^

Sadly, 20.04's (among others it seems) libstdc++ headers are not completely c++20 compatible.

@Saviq
Copy link
Collaborator

Saviq commented Mar 9, 2022

Still unhappy:

/<<PKGBUILDDIR>>/src/platform/graphics/drm_formats.cpp:544: error: "STRINGIFY" redefined [-Werror]
  544 | #define STRINGIFY(val) \
      | 
/<<PKGBUILDDIR>>/src/platform/graphics/drm_formats.cpp:436: note: this is the location of the previous definition
  436 | #define STRINGIFY(format) \
      | 
/<<PKGBUILDDIR>>/src/platform/graphics/drm_formats.cpp: In function ‘constexpr const mir::graphics::DRMFormat::FormatInfo& ***anonymous***::info_for_format(uint32_t)’:
/<<PKGBUILDDIR>>/src/platform/graphics/drm_formats.cpp:433:38: error: uninitialized variable ‘info’ in ‘constexpr’ function
  433 |     mg::DRMFormat::FormatInfo const* info;
      |                                      ^~~~
[ 26%] Building CXX object tests/mir_test_doubles/CMakeFiles/mir-public-test-doubles-platform.dir/mock_xkb.cpp.o
cc1plus: all warnings being treated as errors

@RAOF
Copy link
Contributor Author

RAOF commented Mar 9, 2022

🔥

AlanGriffiths
AlanGriffiths approved these changes Mar 10, 2022
View reviewed changes
Copy link
Contributor

@AlanGriffiths AlanGriffiths left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks mostly sensible.

And I'm not against immutable types (they have lots of advantages).

bors delegate

include/platform/mir/graphics/drm_formats.h Outdated Show resolved Hide resolved
@RAOF
platform/DRMFormat: Switch to value semantics.
bb376f5 
Storing a `FormatInfo const&` is communicates that we're non-null, but
means DRMFormat can't have an assignment operator.

Switch to `FormatInfo const*` to get CopyAssign
@RAOF
Copy link
Contributor Author

RAOF commented Mar 11, 2022

Pretty sure that's a transient failure

bors try

bors bot added a commit that referenced this pull request Mar 11, 2022
@bors
Try #2358:
adec941
@bors
Copy link
Contributor

bors bot commented Mar 11, 2022

try

Build succeeded:

@AlanGriffiths
Copy link
Contributor

bors merge

@bors
Copy link
Contributor

bors bot commented Mar 11, 2022

Build succeeded:

@bors bors bot merged commit e494f7f into main Mar 11, 2022
@bors bors bot deleted the add-drm-format branch March 11, 2022 09:24
@Saviq Saviq mentioned this pull request May 12, 2022
Merged
bors bot added a commit that referenced this pull request May 24, 2022
@bors @Saviq
Merge #2423
a42bf5c 
2423: Release 2.8.0 r=AlanGriffiths,graysonguarino,RAOF,wmww,Saviq a=Saviq

---
> - ABI summary:
>   - miral ABI unchanged at 4
>   - mircommon ABI bumped to 9
>   - mircookie ABI unchanged at 2
>   - mircore ABI unchanged at 1
>   - miroil ABI unchanged at 1
>   - mirplatform ABI unchanged at 23
>   - mirserver ABI bumped to 58
>   - mirwayland ABI unchanged at 3
>   - mirplatformgraphics ABI bumped to 20
>   - mirinputplatform ABI unchanged at 8
> - Enhancements:
>   - Move generated protocol code to build directory (#2300)
>   - Allow --app-env-amend to be supplied multiple times (#2333)
>   - Make window title a configuration option (#2349)
>   - Add fatal_error if unable to bind Wayland socket (#2350)
>   - Add `mold` to the list of supported linkers (#2353)
>   - Platform refactoring towards hybrid GPU support (#2358, #2378, #2407)
>   - Implement wlr_screencopy_unstable_v1 for screenshots (#2383)
>   - Refactor out mf::MirDisplay (#2406)
> - Bugs fixed:
>   - Synchronize buffer swaps to video frame in egl spinner (Fixes #2154)
>   - Do not give menus keyboard focus (Fixes #2324)
>   - Refactor Wayland keyboard input (Fixes #2331)
>   - Further simplify and correct keyboard focus setting (Fixes #2338)
>   - wl_pointer: do not send events when not compatible (Fixes #2341)
>   - Kill clients with error instead of sending unsupported (Fixes #2343)
>   - Initialize sig_handler_desc.sa_mask (#2386)
>   - Fix ThreadedDispatcherSignalTest.keeps_dispatching... (Fixes #2377)

Co-authored-by: Michał Sawicz <michal@sawicz.net>
Co-authored-by: bors[bot] <26634292+bors[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AlanGriffiths AlanGriffiths AlanGriffiths approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@RAOF @Saviq @AlanGriffiths