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.

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

Introduce wlr_format_set #1642

Merged
merged 2 commits into from Apr 8, 2019

Conversation

Projects
None yet
4 participants
@emersion
Copy link
Member

commented Mar 31, 2019

This type adds a container for formats + modifiers.

This is cherry-picked from #1355. wlr_format_set is only used in one place for now, but it'll be useful in other places too in #1641.

@emersion emersion requested a review from ascent12 Mar 31, 2019

@agx

This comment has been minimized.

Copy link
Contributor

commented Apr 1, 2019

Look good to me. I'd like wlr_dmabuf_format_set better but that's maybe a little long.

@ascent12

This comment has been minimized.

Copy link
Member

commented Apr 1, 2019

@agx

I'd like wlr_dmabuf_format_set better but that's maybe a little long.

The idea is that we would standardise everything in the codebase to "speak" these formats, and the modifier would just go unused or possibly be set to DRM_FORMAT_MOD_LINEAR where modifiers didn't make sense. I don't really think any extra qualification to the name is needed, but wlr_drm_format_set would be a slightly shorter compromise.

@emersion
This code is largely written by me, so can you preserve this in the commit author or possibly with a Co-authored-by.

Not really related to this PR, but it was/is intended for a intersection function to exist for this. I didn't get around to writing it, because I didn't get to the point where I needed it yet. It could be added later.

@agx

This comment has been minimized.

Copy link
Contributor

commented Apr 1, 2019

wlr_drm_format_set sounds good to me (but this is mostly bikeshedding)

@atomnuker

This comment has been minimized.

Copy link
Contributor

commented Apr 1, 2019

No need for an array of modifiers in the struct, just make it a single uint64_t. Modern kernels only use the first plane's modifier and very old kernels expect all plane modifiers to be identical.

@ascent12

This comment has been minimized.

Copy link
Member

commented Apr 1, 2019

It's done as a array of modifiers because that's how the GBM API works; it's the possible modifiers it can use, and the driver can pick the one it wants. It's not a separate modifier for each plane.

render: introduce wlr_drm_format_set
This types adds a container for formats + modifiers.

A list that is of [format [modifier]] was chosen instead of
[format modifer] because that is how GBM accepts them.

Co-Authored-By: emersion <contact@emersion.fr>

@emersion emersion force-pushed the emersion:format-set branch from 8503ee5 to 38e5eff Apr 1, 2019

@emersion emersion force-pushed the emersion:format-set branch from 38e5eff to e42178d Apr 1, 2019

@emersion

This comment has been minimized.

Copy link
Member Author

commented Apr 1, 2019

This code is largely written by me, so can you preserve this in the commit author or possibly with a Co-authored-by.

Err, you're right, thanks for pointing this out!

Updated to split it in two commits.

Changes I made to yours:

  • Renamed to wlr_drm_format_set
  • Fixed a realloc size bug (didn't include the fixed part)
@emersion

This comment has been minimized.

Copy link
Member Author

commented Apr 7, 2019

Ping

@agx

agx approved these changes Apr 8, 2019

@agx agx merged commit 9faea17 into swaywm:master Apr 8, 2019

3 checks passed

builds.sr.ht: alpine.yml builds.sr.ht job completed successfully
Details
builds.sr.ht: archlinux.yml builds.sr.ht job completed successfully
Details
builds.sr.ht: freebsd.yml builds.sr.ht job completed successfully
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.