All of lore.kernel.org
 help / color / mirror / Atom feed
From: Luca Ceresoli <luca.ceresoli@bootlin.com>
To: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
Cc: kernel@collabora.com, linux-sound@vger.kernel.org,
	linux-kernel@vger.kernel.org, Mark Brown <broonie@kernel.org>
Subject: Re: [PATCH] ASoC: dapm-graph: set fill colour of turned on nodes
Date: Mon, 24 Feb 2025 16:33:11 +0100	[thread overview]
Message-ID: <20250224163311.0611da3f@booty> (raw)
In-Reply-To: <20250221-dapm-graph-node-colour-v1-1-514ed0aa7069@collabora.com>

Hello Nicolas,

+Cc: Mark Brown

On Fri, 21 Feb 2025 21:39:32 +0100
Nicolas Frattaroli <nicolas.frattaroli@collabora.com> wrote:

> Some tools like KGraphViewer interpret the "ON" nodes not having an
> explicitly set fill colour as them being entirely black, which obscures
> the text on them and looks funny. In fact, I thought they were off for
> the longest time. Comparing to the output of the `dot` tool, I assume
> they are supposed to be white.
> 
> Instead of speclawyering over who's in the wrong and must immediately
> atone for their wickedness at the altar of RFC2119, just be explicit
> about it, set the fillcolor to white, and nobody gets confused.
> 
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> ---
> This is somewhat "just thrown out there"; I noticed that not setting the
> fill colour breaks KGraphViewer only *after* I thought this was just how
> they were for several days. With this change, both dot and KGraphViewer
> render it correctly, but I have no clue as to whether it's in the spirit
> of the file format at all. I figure that if this saves some other poor
> souls a bit of time and confusion, then it's worth it.

I confirm the issue with a plain installation of KGraphViewer (which I
didn't know at all before -- interesting tool).

So, let's have mercy on the poor souls:

Tested-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

  reply	other threads:[~2025-02-24 15:33 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-21 20:39 [PATCH] ASoC: dapm-graph: set fill colour of turned on nodes Nicolas Frattaroli
2025-02-24 15:33 ` Luca Ceresoli [this message]
2025-02-24 15:42   ` Mark Brown
2025-02-24 16:09     ` Nicolas Frattaroli
2025-02-24 16:21       ` Mark Brown
2025-02-25 13:38 ` Mark Brown

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=20250224163311.0611da3f@booty \
    --to=luca.ceresoli@bootlin.com \
    --cc=broonie@kernel.org \
    --cc=kernel@collabora.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sound@vger.kernel.org \
    --cc=nicolas.frattaroli@collabora.com \
    /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.