[PATCH] D29363: [XRAY] A Color Choosing helper for XRay Graph

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added inline comments.


================
Comment at: tools/llvm-xray/xray-color-helper.cc:22-35
+// Static Polynomials for the ColorHelper.
+//
+// In order to calculate these polynomials,
+//   1. Convert the color scheme samples from sRGB to LAB color space.
+//   2. Interpolate between the descrete colors in LAB space using a cubic
+//      spline interpolation.
+//   3. Sample this interpolation at 100 points and convert to sRGB.
----------------
varno wrote:
> chandlerc wrote:
> > varno wrote:
> > > chandlerc wrote:
> > > > I really think this is more complexity than is reasonable to add to LLVM for this use case.
> > > > 
> > > > IMO, we don't need perfectly accurate color interpolation here. We need roughly accurate, *simple* interpolation so that it is useful even where it isn't perfect.
> > > > 
> > > > I would strongly suggest switching to what I understand is the very common scheme of RGB -> HSV, linear interpolate, HSV -> RGB process for interpolating colors. That seems likely to be good enough for all realistic usages LLVM will have, and much easier to understand and maintain going forward.
> > > Hi Chandler.
> > > 
> > > Colors are hard. Really hard. Especially when you want perceptual uniformity, as we do here. We want our perception of color to present a sense of distance quality. This is an attempt to have good perceptual uniformity in the least complicated way possible.
> > > 
> > > I have not heard of anyone interpolating in HSV, can you show an example. Everyone who does this sort of thing interpolates either in RGB very, very carefully (with many had chosen intermediate pointe) or in LAB like I am doing (or using bisection in delta-E).
> > I understand that colors are hard. I just don't believe that it is actually important to have (perfect) perceptual uniformity for these particular use cases.
> > 
> > I think that for tools like this, being close or having a rough correspondence is more than enough. If someone wants to get precise, they should use the actual data and not colors. The colors are to help *at a glance*.
> > 
> > Interpolating using HSV is the first answer to the first SO question I find when I search on Google for "interpolating between two colors":
> > http://stackoverflow.com/questions/13488957/interpolate-from-one-color-to-another
> > 
> > So, I guess not everyone?
> > 
> > Still, my comment remains in essence: this is too complicated for the value it brings. We should use a simpler solution with lower quality because the qualitative difference won't matter here. SO suggests one approach that seems reasonably simple. I'm happy for you to suggest other approaches, but they need to be similarly simple.
> Hi Chandler 
> 
> This patch implements the interpolation in HSV as requested.
Thanks, this is already tremendously simpler.

I'd also suggest starting with a fairly small set of colors / schemes. I think 2 or 3 are fine. IT is easy to add more if and when we have a compelling use case for more color schemes. Until then, YAGNI -- we shouldn't add the code.


https://reviews.llvm.org/D29363