[subsurface] Prettier profile colors (#89)

Dirk Hohndel dirk at hohndel.org
Mon Nov 28 12:29:54 PST 2011


On Mon, 28 Nov 2011 12:01:22 -0800, Linus Torvalds <torvalds at linux-foundation.org> wrote:
> On Mon, Nov 28, 2011 at 5:00 AM, henrik242
> <reply+i-2367030-22df6d3716b3a7f680fb3a594ca46d284a94afd4-1024025 at reply.github.com>
> wrote:
> > The profile colors aren't very pretty, and the grid lines are too thick.
> > This commit tries to improve that.
> 
> [ For others.. the pull request is at
> 
>    git://github.com/henrik242/subsurface profile-colors
> 
>   so you can look at it ]
> 
> So looking at before-and-after side-by-side, I do think this may be an
> improvement, but I have a few objections, some of which are big enough
> that I won't be pulling this as-is.
> 
> First off, the new background color is some kind of very light beige,
> which is neutral and fairly pleasing, but the new thin white gridlines
> are really hard to see. I bet it depends on just how contrasty the
> monitor settings are, but and for me it's kind of borderline usable,
> but it's borderline enough that I worry. The yellow warning markers
> are also much less visible.

I tried this on three different systems with different monitors. Haven't
found one where the gridlines work. The warning markers on the other
hand are now a bit less obnoxious - that I'm fine with :-)

> I like the new water fill colorization, though.

Me too, but if we do the 'darker blue as we go deeper' thing, then that
should be on a fixed scale - i.e. the deepest blue that you get should
be proportional to the deepest depth that is displayed - a dive on the
default scale (down to 100ft / 30m) should get at most a medium blue,
yet a dive that goes down to say 200ft / 60m should get a really dark
blue.

Hmm - maybe I'm over-thinking this :-)
 
> The depth printout changes are horrible, though, and totally unusable.
> We used to have nice readable "black-on-white" depth markers, they
> have now turned into these monstrous bold-black text on top of gray.
> That's not just ugly, it's also totally unreadable on lots of
> real-life printers that do gray with various dithering schemes (think
> traditional laser printer), so now that text is just an unreadable
> blob when actually printed out.

Completely unreadable. That part gets a NAK from me, too.
 
> Finally, in the code itself, I kind of like having things in one
> place, but quite frankly, that one table is broken. It turns the
> random "color indexes" into random RGB values, which makes it not very
> readable at all.
> 
> I'd suggest having a mapping of "color names to RGB" (which may be
> just a list of #define's), and then having a mapping from color index
> to color name instead. That way the global color index array would be
> a lot more readable.

Yes. Also, you still do three lookup tables right now for no good
reason. Why don't you have one table that matches a color name into rgba
for screen and rgb for print. And then have one table with the color_t
enum as index into it that matches the different values to a color. And
then simply use the color_t values to index into that one table.

> I would also suggest that perhaps the color index array would map to
> *two* colors: the screen color and the printer color. That way we
> could automatically have some nice way of showing "don't print this"
> (by just making the printer color some invalid color), but it would
> also avoid the issue with random mappings of colors for printing. Now
> you hack around it with nasty "DEPTH_FILL_PRINTER" things.

I like that suggestion (it's an even better extension of what I just
wrote abive with printer colors). 

> Finally: maybe you should make the "rgb_t" a "rgba_t". Now you have
> that odd "we have indexes to the color table, and then we have a
> separate alpha channel value". But who knows. I don't have a very
> strong opinion on this part.

I think if we go this direction we really want just one table with rgba
for screen and then rgb for print. After all the goal is to make things
more simple, right?

/D


More information about the subsurface mailing list