[PATCH v16 13/23] compositor-drm: Add modifiers to GBM dmabuf import

[Pekka Paalanen]

On Mon, 9 Jul 2018 13:25:50 +0100
Daniel Stone <daniel at fooishbar.org> wrote:

> Hi Pekka,
> 
> On Mon, 9 Jul 2018 at 11:22, Pekka Paalanen <ppaalanen at gmail.com> wrote:
> > On Mon, 9 Jul 2018 12:39:30 +0300 Pekka Paalanen <ppaalanen at gmail.com> wrote:  
> > > On Thu,  5 Jul 2018 18:16:40 +0100 Daniel Stone <daniels at collabora.com> wrote:  
> > > > +static struct drm_fb *
> > > > +drm_fb_get_from_dmabuf(struct linux_dmabuf_buffer *dmabuf,
> > > > +                  struct drm_backend *backend, bool is_opaque)
> > > > +{  
> >
> > Hi,
> >
> > one more complication:
> >
> > Because is_opaque state is now baked in to drm_fb, if
> > drm_fb_get_from_bo() returns an existing drm_fb for a bo, we need to
> > make sure is_opaque still matches. I think is_opaque might change if
> > the application is recycling dmabuf wl_buffers without destroying them
> > (as intended), but changes the wl_surface opaque region according to its
> > rendering.  
> 
> Right you are, but on the other hand it always has been, since
> drm_check_output_format() would turn ARGB8888 into XRGB8888 if the
> surface was opaque.
> 
> I filed an issue for this long ago:
> https://gitlab.freedesktop.org/wayland/weston/issues/20
> 
> I don't know of any good or easy answers. If an alpha <-> opaque
> transition occurs whilst the FB is being displayed, calling
> drmModeRmFB may forcibly remove it, including disabling the whole CRTC
> (!) if we're doing fullscreen direct scanout. The way to properly
> support and fix this would be to break the 1:1 buffer -> FB link,
> allowing a buffer to have multiple FBs with separate properties
> created from it. Given that this isn't a new problem and no-one has
> complained yet, I'm inclined to leave this on the backburner.

Hi,

I did not mean that a client changes the opaqueness *while* the FB is
up. I agree that that would be nonsense.

I meant a sequence more like this from a client:

- create buffer A
- draw buffer A
- set opaque region and commit buffer A
- create buffer B
- draw buffer B
- commit buffer B
- get release for buffer A
- draw buffer A with something transparent
- unset opaque region and commit buffer A

In the last step, if the compositor re-uses the FB it originally
created for buffer A, it will remain opaque contrary to what the client
intended.

A good client will wait for the release, so doesn't that prevent the
possibility of destroying the FB while on screen? If the compositor
checked the FB is not on screen, it could honour the legit opaqueness
change by destroying and re-creating the FB, while nonsensical
opaqueness changes (i.e. without content update or drawing into a
reserved buffer) would remain equally broken as they used to.

Anyway, it's good that the bug report exists, and I think that's
sufficient. I wanted to make sure this issue is known.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20180709/91c1792b/attachment-0001.sig>