All of lore.kernel.org
 help / color / mirror / Atom feed
From: eric@anholt.net (Eric Anholt)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v4 1/5] drm/blend: Add a generic alpha property
Date: Wed, 04 Apr 2018 09:03:36 -0700	[thread overview]
Message-ID: <87a7ujc61z.fsf@anholt.net> (raw)
In-Reply-To: <e0c1e8af3c8e9a8283d78d4cdeb2fe92147c5945.camel@bootlin.com>

Paul Kocialkowski <paul.kocialkowski@bootlin.com> writes:

> [ Unknown signature status ]
> Hi,
>
> On Fri, 2018-03-30 at 13:37 -0700, Eric Anholt wrote:
>> Maxime Ripard <maxime.ripard@bootlin.com> writes:
>> 
>> > Some drivers duplicate the logic to create a property to store a
>> > per-plane
>> > alpha.
>> > 
>> > This is especially useful if we ever want to support extra protocols
>> > for
>> > Wayland like:
>> > https://lists.freedesktop.org/archives/wayland-devel/2017-August/034
>> > 741.html
>> > 
>> > Let's create a helper in order to move that to the core.
>> > diff --git a/drivers/gpu/drm/drm_blend.c
>> > b/drivers/gpu/drm/drm_blend.c
>> > index 5a81e1b4c076..05eda2d57c77 100644
>> > --- a/drivers/gpu/drm/drm_blend.c
>> > +++ b/drivers/gpu/drm/drm_blend.c
>> > @@ -88,6 +88,12 @@
>> >   * On top of this basic transformation additional properties can be
>> > exposed by
>> >   * the driver:
>> >   *
>> > + * alpha:
>> > + * 	Alpha is setup with drm_plane_create_alpha_property().
>> > It controls the
>> 
>> s/setup/set up/
>> 
>> > + * 	plane-wide opacity, from transparent (0) to opaque
>> > (0xffff). It can be
>> > + * 	combined with pixel alpha.
>> > + * 	The alpha value is represented as premultiplied alpha.
>> 
>> I don't think this premultiplied comment makes any sense.  What are
>> you saying it's premultiplied with?  Maybe you mean that the output
>> pixels will have both their color and alpha channels multiplied by
>> this alpha?
>> I'd just drop it.
>
> I disagree here: since there are multiple ways to blend the pixel alpha
> value and the plane-wide alpha value, I think it's important to clearly
> specify which blending equation DRM expects here. Otherwise, the plane-
> wide alpha value just doesn't have a specified meaning and driver
> implementations can't figure whether the hardware uses the same equation
> or not and whether adaptation to this coefficient is needed.
>
> The equations for premultiplied alpha blending are at:
> https://en.wikipedia.org/wiki/Alpha_compositing#Alpha_blending
>
> What do you think?

"Premultiplied alpha" only has a meaning for an RGBA value, where RGB
have been already multiplied by their own A.  In this case, you're
saying that an alpha-only value is "premultiplied", but with what?  What
would an alternative to it being "premultiplied" do instead?  That's
what I'm confused about.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180404/506231ea/attachment.sig>

WARNING: multiple messages have this Message-ID (diff)
From: Eric Anholt <eric@anholt.net>
To: Paul Kocialkowski <paul.kocialkowski@bootlin.com>,
	Maxime Ripard <maxime.ripard@bootlin.com>,
	Gustavo Padovan <gustavo@padovan.org>,
	Daniel Vetter <daniel.vetter@intel.com>,
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Sean Paul <seanpaul@chromium.org>
Cc: dri-devel@lists.freedesktop.org,
	Stefan Schake <stschake@gmail.com>,
	Boris Brezillon <boris.brezillon@bootlin.com>,
	Chen-Yu Tsai <wens@csie.org>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v4 1/5] drm/blend: Add a generic alpha property
Date: Wed, 04 Apr 2018 09:03:36 -0700	[thread overview]
Message-ID: <87a7ujc61z.fsf@anholt.net> (raw)
In-Reply-To: <e0c1e8af3c8e9a8283d78d4cdeb2fe92147c5945.camel@bootlin.com>


[-- Attachment #1.1: Type: text/plain, Size: 2434 bytes --]

Paul Kocialkowski <paul.kocialkowski@bootlin.com> writes:

> [ Unknown signature status ]
> Hi,
>
> On Fri, 2018-03-30 at 13:37 -0700, Eric Anholt wrote:
>> Maxime Ripard <maxime.ripard@bootlin.com> writes:
>> 
>> > Some drivers duplicate the logic to create a property to store a
>> > per-plane
>> > alpha.
>> > 
>> > This is especially useful if we ever want to support extra protocols
>> > for
>> > Wayland like:
>> > https://lists.freedesktop.org/archives/wayland-devel/2017-August/034
>> > 741.html
>> > 
>> > Let's create a helper in order to move that to the core.
>> > diff --git a/drivers/gpu/drm/drm_blend.c
>> > b/drivers/gpu/drm/drm_blend.c
>> > index 5a81e1b4c076..05eda2d57c77 100644
>> > --- a/drivers/gpu/drm/drm_blend.c
>> > +++ b/drivers/gpu/drm/drm_blend.c
>> > @@ -88,6 +88,12 @@
>> >   * On top of this basic transformation additional properties can be
>> > exposed by
>> >   * the driver:
>> >   *
>> > + * alpha:
>> > + * 	Alpha is setup with drm_plane_create_alpha_property().
>> > It controls the
>> 
>> s/setup/set up/
>> 
>> > + * 	plane-wide opacity, from transparent (0) to opaque
>> > (0xffff). It can be
>> > + * 	combined with pixel alpha.
>> > + * 	The alpha value is represented as premultiplied alpha.
>> 
>> I don't think this premultiplied comment makes any sense.  What are
>> you saying it's premultiplied with?  Maybe you mean that the output
>> pixels will have both their color and alpha channels multiplied by
>> this alpha?
>> I'd just drop it.
>
> I disagree here: since there are multiple ways to blend the pixel alpha
> value and the plane-wide alpha value, I think it's important to clearly
> specify which blending equation DRM expects here. Otherwise, the plane-
> wide alpha value just doesn't have a specified meaning and driver
> implementations can't figure whether the hardware uses the same equation
> or not and whether adaptation to this coefficient is needed.
>
> The equations for premultiplied alpha blending are at:
> https://en.wikipedia.org/wiki/Alpha_compositing#Alpha_blending
>
> What do you think?

"Premultiplied alpha" only has a meaning for an RGBA value, where RGB
have been already multiplied by their own A.  In this case, you're
saying that an alpha-only value is "premultiplied", but with what?  What
would an alternative to it being "premultiplied" do instead?  That's
what I'm confused about.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

  parent reply	other threads:[~2018-04-04 16:03 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-13 20:54 [PATCH v4 0/5] drm/blend: Support generic plane-wide alpha Maxime Ripard
2018-03-13 20:54 ` Maxime Ripard
2018-03-13 20:54 ` [PATCH v4 1/5] drm/blend: Add a generic alpha property Maxime Ripard
2018-03-13 20:54   ` Maxime Ripard
2018-03-30 20:37   ` Eric Anholt
2018-03-30 20:37     ` Eric Anholt
2018-04-04  9:15     ` Paul Kocialkowski
2018-04-04  9:15       ` Paul Kocialkowski
2018-04-04  9:46       ` Laurent Pinchart
2018-04-04  9:46         ` Laurent Pinchart
2018-04-04 16:03       ` Eric Anholt [this message]
2018-04-04 16:03         ` Eric Anholt
2018-04-04  8:47   ` Paul Kocialkowski
2018-04-04  8:47     ` Paul Kocialkowski
2018-03-13 20:54 ` [PATCH v4 2/5] drm/atmel-hclcdc: Convert to the new " Maxime Ripard
2018-03-13 20:54   ` Maxime Ripard
2018-03-13 20:54 ` [PATCH v4 3/5] drm/rcar-du: " Maxime Ripard
2018-03-13 20:54   ` Maxime Ripard
2018-03-13 20:54 ` [PATCH v4 4/5] drm/sun4i: Add support for plane alpha Maxime Ripard
2018-03-13 20:54   ` Maxime Ripard
2018-04-04  8:49   ` Paul Kocialkowski
2018-04-04  8:49     ` Paul Kocialkowski
2018-03-13 20:54 ` [PATCH v4 5/5] drm/docs: Remove the rcar alpha from the csv file Maxime Ripard
2018-03-13 20:54   ` Maxime Ripard
2018-03-26  7:47 ` [PATCH v4 0/5] drm/blend: Support generic plane-wide alpha Maxime Ripard
2018-03-26  7:47   ` Maxime Ripard

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87a7ujc61z.fsf@anholt.net \
    --to=eric@anholt.net \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.