Skip to content
Merged
Show file tree
Hide file tree
Changes from 51 commits
Commits
Show all changes
99 commits
Select commit Hold shift + click to select a range
b96836c
Readonly HSL view
tznind Jul 11, 2024
266f698
Make it possible to move between bars by moving to subview
tznind Jul 11, 2024
013252e
Basically working and with mouse support
tznind Jul 11, 2024
894d4df
Fix HSL to work properly with double values instead of color matching
tznind Jul 11, 2024
0ea13cf
Fix Value on ColorPicker to match HSL values
tznind Jul 11, 2024
602fa21
Fix color spectrum
tznind Jul 11, 2024
eb569f4
Add Swatch and better sync with text box
tznind Jul 11, 2024
c02cb8c
Work on jitter
tznind Jul 11, 2024
2f844fb
ColorPicker HSL working
tznind Jul 11, 2024
d2deb14
More keybindings
tznind Jul 11, 2024
257d97c
Add ColorModel
tznind Jul 11, 2024
49cebf0
Support both HSL and HSV
tznind Jul 11, 2024
d3b09f5
Add RGB
tznind Jul 11, 2024
97e4a6e
Better mouse handling
tznind Jul 11, 2024
11a8f0d
Merge branch 'v2_develop' into color-picker-hsv
tznind Jul 12, 2024
82aa2d2
WIP: AttributeView and integrate into LineDrawing
tznind Jul 12, 2024
f3a90ba
Merge branch 'color-picker-hsv' of https://github.com/tznind/gui.cs i…
tznind Jul 12, 2024
cc4feed
Fix color picking
tznind Jul 12, 2024
29a13ca
Add concept of an ITool
tznind Jul 12, 2024
0558b2d
Add ColorPickerStyle
tznind Jul 13, 2024
4241fb0
Fix selected cell rendering
tznind Jul 13, 2024
206a025
Add first test for ColorPicker2
tznind Jul 13, 2024
14d9b3d
Add more RGB tests
tznind Jul 13, 2024
df9fb5a
Improve ColorPicker2 setup process
tznind Jul 13, 2024
e73408c
Tests and fixes for keyboard changing value R
tznind Jul 13, 2024
f9fac50
Fix margin on bars when no textfields
tznind Jul 13, 2024
7fecfea
Add mouse test
tznind Jul 13, 2024
7fbc5cb
Add tests for with text field
tznind Jul 13, 2024
32c4357
Add more tests and fix bug sync component text field change with hex …
tznind Jul 13, 2024
8a1abfa
Fix tests and fix clicking in a bar label area possibly not selecting
tznind Jul 13, 2024
e13389d
Move AttributeView to LineDrawing and adjust to have a 'transparent p…
tznind Jul 13, 2024
8d8de6d
Merge branch 'v2_develop' into color-picker-hsv
tznind Jul 13, 2024
c718c15
Render triangle in dark gray if background is black
tznind Jul 13, 2024
68fe0d6
Merge branch 'color-picker-hsv' of https://github.com/tznind/gui.cs i…
tznind Jul 13, 2024
be83712
Add ColorChanged event
tznind Jul 13, 2024
034cbc1
Resharper Cleanup
tznind Jul 13, 2024
011f2a6
Merge branch 'v2_develop' into color-picker-hsv
tznind Jul 14, 2024
ce63697
Xml comments and public/private adjustments
tznind Jul 14, 2024
bd1a00e
Merge branch 'v2_develop' into color-picker-hsv
tznind Jul 14, 2024
d4697bd
Explore replacing diagram test with fragile Subview diving
tznind Jul 14, 2024
7ad915e
Migrate ColorPicker_DefaultBoot to sub asserts
tznind Jul 14, 2024
5c2bb8a
Port other tests
tznind Jul 14, 2024
046c2fd
Replace ColorPicker with new view
tznind Jul 14, 2024
0c099e0
Fix ColorPicker size to match scenarios size assumptions
tznind Jul 14, 2024
4dc4862
Split to separate files and ignore invalid test for ColorPicker
tznind Jul 14, 2024
04645dc
Ignore also in mouse version of AllViews_Enter_Leave_Events
tznind Jul 14, 2024
d6eb6c3
Remove bool _updating from ColorPicker
tznind Jul 14, 2024
6eecec3
Typo fix
tznind Jul 14, 2024
b4e32fd
Fix ReSharper bad renames in comments for "Value"
tznind Jul 14, 2024
e3e90ef
Refactor to single implementation of 'prompt for color' logic
tznind Jul 14, 2024
f9f62a5
Sum runes instead of Length
tznind Jul 14, 2024
21897b5
Hide ColorBar and SetValueWithoutRaisingEvent from public API
tznind Jul 16, 2024
4db6f40
Merge branch 'v2_develop' into color-picker-hsv
tig Jul 21, 2024
d99996a
Merge branch 'v2_develop' into color-picker-hsv
tznind Aug 3, 2024
9c58889
Move ColorEventArgs to Drawing folder
tznind Aug 3, 2024
e462ece
Merge branch 'color-picker-hsv' of https://github.com/tznind/gui.cs i…
tznind Aug 3, 2024
6f81f3e
Move ColorModel to Drawing folder
tznind Aug 3, 2024
769c37e
First try at Dim.Auto for ColorPicker
tznind Aug 3, 2024
182d631
Merge branch 'v2_develop' into color-picker-hsv
tig Aug 3, 2024
113d513
Remove explicit width/height setting in most scenarios
tznind Aug 4, 2024
fa9ff49
Merge branch 'color-picker-hsv' of https://github.com/tznind/gui.cs i…
tznind Aug 4, 2024
93f0efc
Remove explicit heights
tznind Aug 4, 2024
33422b4
Merge branch 'v2_develop' into color-picker-hsv
tig Aug 5, 2024
6b5e3a0
Fixed build/test issues.
tig Aug 5, 2024
885cfa8
Merge pull request #166 from tig/tznind-color-picker-hsv
tznind Aug 5, 2024
9a5d713
WIP: Start working on test changes and add new options to ColorPicker…
tznind Aug 8, 2024
39efad8
Merge branch 'v2_develop' into color-picker-hsv
tznind Aug 8, 2024
337390a
Fix for R indicator arrow sometimes 'falling off' the drawn area.
tznind Aug 8, 2024
3a1203d
Add nullable enable
tznind Aug 8, 2024
6e2f612
Test fixes and refactor for avoiding Begin
tznind Aug 8, 2024
4be9476
Merge branch 'v2_develop' into color-picker-hsv
tznind Aug 11, 2024
2519eda
Merge branch 'v2_develop' into color-picker-hsv
tig Aug 12, 2024
ea61495
Make ColorEventArgs inherit from EventArgs<Color>
tznind Aug 12, 2024
045c7a2
Merge branch 'color-picker-hsv' of https://github.com/tznind/gui.cs i…
tznind Aug 12, 2024
e371e99
Fix Dispose not being called on bars when switching color models
tznind Aug 12, 2024
b41a566
Remove 'oldColor' from test now it is not supported
tznind Aug 12, 2024
2efd89f
Add initial stab at ColorPickerStyle.ShowName
tznind Aug 12, 2024
ceeb54d
Merge branch 'v2_develop' into color-picker-hsv
tznind Aug 14, 2024
7bcaac3
Use AppendAutocomplete for color names
tznind Aug 14, 2024
4f41730
Merge branch 'color-picker-hsv' of https://github.com/tznind/gui.cs i…
tznind Aug 14, 2024
b4b37ef
Merge branch 'v2_develop' into color-picker-hsv
tznind Aug 14, 2024
2df0981
Implemented resoruce based colorname resolver
tig Aug 14, 2024
3978a2b
Merge pull request #167 from tig/tznind-color-picker-hsv
tznind Aug 17, 2024
a97096a
Update GetTextField to support getting the color names field
tznind Aug 17, 2024
9e79569
Color name updates when navigating away from the named color
tznind Aug 17, 2024
3312cbc
Merge branch 'v2_develop' into color-picker-hsv
tznind Aug 17, 2024
d1862d9
Restore old color picker as ColorPicker16
tznind Aug 17, 2024
6c17a6a
Merge branch 'color-picker-hsv' of https://github.com/tznind/gui.cs i…
tznind Aug 17, 2024
0bbe95e
Add test that shows 'Save as' is currently considered a named color ><
tznind Aug 17, 2024
0df0ffa
Fix GetW3CColorNames
tznind Aug 18, 2024
bdaa262
Merge branch 'v2_develop' into color-picker-hsv
tig Aug 19, 2024
77bd8ac
Removed dupe colors
tig Aug 19, 2024
98d5d7c
Merged correctly. Code Cleanup
tig Aug 19, 2024
37dc439
Merge pull request #168 from tig/tznind-color-picker-hsv
tznind Aug 19, 2024
b36a9ba
Merge branch 'v2_develop' into color-picker-hsv
tig Aug 19, 2024
a83ea4b
Revert to old color pickers
tznind Aug 19, 2024
0d91d68
Merge branch 'color-picker-hsv' of https://github.com/tznind/gui.cs i…
tznind Aug 19, 2024
461168b
Nullability question marks for everyone!
tznind Aug 19, 2024
e275eb6
Merge branch 'v2_develop' into color-picker-hsv
tznind Aug 22, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions Terminal.Gui/Views/BBar.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
using ColorHelper;

namespace Terminal.Gui;

internal class BBar : ColorBar
{
public RBar RBar { get; set; }
public GBar GBar { get; set; }

/// <inheritdoc/>
protected override int MaxValue => 255;

/// <inheritdoc/>
protected override Color GetColor (double fraction)
{
var rgb = new RGB ((byte)RBar.Value, (byte)GBar.Value, (byte)(MaxValue * fraction));

return new (rgb.R, rgb.G, rgb.B);
}
}
226 changes: 226 additions & 0 deletions Terminal.Gui/Views/ColorBar.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,226 @@
namespace Terminal.Gui;

/// <summary>
/// A bar representing a single component of a <see cref="Color"/> e.g.
/// the Red portion of a <see cref="ColorModel.RGB"/>.
/// </summary>
public abstract class ColorBar : View, IColorBar
{
/// <summary>
/// X coordinate that the bar starts at excluding any label.
/// </summary>
private int _barStartsAt;

/// <summary>
/// 0-1 for how much of the color element is present currently (HSL)
/// </summary>
private int _value;

/// <summary>
/// The amount of <see cref="Value"/> represented by each cell width on the bar
/// Can be less than 1 e.g. if Saturation (0-100) and width > 100
/// </summary>
private double _cellValue = 1d;

/// <summary>
/// The maximum value allowed for this component e.g. Saturation allows up to 100 as it
/// is a percentage while Hue allows up to 360 as it is measured in degrees.
/// </summary>
protected abstract int MaxValue { get; }

/// <summary>
/// The currently selected amount of the color component stored by this class e.g.
/// the amount of Hue in a <see cref="ColorModel.HSL"/>.
/// </summary>
public int Value
{
get => _value;
set
{
int clampedValue = Math.Clamp (value, 0, MaxValue);

if (_value != clampedValue)
{
_value = clampedValue;
OnValueChanged ();
}
}
}

/// <inheritdoc />
public void SetValueWithoutRaisingEvent (int v)
Copy link
Collaborator

@dodexahedron dodexahedron Jul 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a fan of this.

If setting something without raising events is necessary, unsubscribe.

What if the user wants those events to be raised for every individual value for whatever reason?

As a heads up, too:

When I copy that code from ColorHelper, that HSL type and the others in that namespace are getting turned into structs, so keep the value type semantics in mind even though it's currently a reference type. That'll also let you use the with operator once that's done, but of course you can't use it right now.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this only because @tig didn't like bool updating approach in parent view - see #3604 (comment)

We need a way to sync all the sub bits of ColorPicker without getting circular events being triggered.

A compromise is perhaps that the method can be made internal?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another suggestion is add a bool property like RaiseEventForAllValues.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The point of the change was to remove variables and before/after operations.

We do want the event raised for clicks and keyboard changes just not when we are sync a change e.g. when user tupes new value in hex box

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I interacted on impulse due to @dodexahedron saying that the user might want to be notified of all changes. Really, if they are unnecessary, avoiding them from generating events is the best option.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The point of the change was to remove variables and before/after operations.

We do want the event raised for clicks and keyboard changes just not when we are sync a change e.g. when user tupes new value in hex box

Well... Why not?

Other color pickers I (unreliably) recall usually update in real time.

Paint.net definitely does. Observe:

pdnColorPicker.mp4

However, making that behavior work exactly like that, for all possible 1-8 digit hex strings, would probably require some expansion of the color parse switch block to handle the 1, 2, 4, 5, and 7 character cases, I think. ...Though it's been many moons since I wrote that, so I don't remember all of the cases it handles off the top of my head. I'm pretty sure those are the cases not covered.

Alternatively/more simply, we could have the parse code only raise the event for strings that are meaningful, like 1 (interpret as that nibble repeated 6 times), 2 (that byte repeated 3 times), 3 (it already does that - each nibble is doubled, like on the web), 4 (also does this I'm pretty sure - same as 3 but with alpha), 6 (of course already does this), and 8 (again, already does this). So that only really means adding cases for 1 and 2, I guess? I suppose 5 and 7 could be handled, assuming there's just an omitted leading zero for both, making them just use the 6 and 8 character cases... 🤷‍♂️

And of course that struct is natively ARGB, but that's only relevant for naming really because it's just a 32-bit binary value. So, following some of my original thoughts farther down the rabbit hole, I think the way I might end up dealing with that is pretty much how .net 8 and the System.Numerics pseudo-recursive interfaces with static virtuals work: A couple of common interfaces with things like the named colors, and then ColorBlahBlah structs for each pixel format/color space, so they're largely interchangeable, the way things like INumber<TSelf> are, regardless of the underlying struct1.

That's where the things like int.Zero come from - they're defined on the static virtual interface member, and then each implementing type brings its own implementation of that member, but it is static, so it is available on the type argument of the interface (so, the color types themselves), meaning it's a largely non-breaking change, insofar as Color itself and its visible API surface goes.

Anyway... There's what occurred to me while thinking about it right now. Plenty of options available other than those, of course, but I think those would be very low-code and highly idiomatic C#12+ ways to go about it, at least, with details up to the implementation and ultimately irrelevant to the consumer (yay for interfaces!).

Footnotes

  1. Static virtuals on self-referencing generic interfaces are honestly something we should do to Dim/Pos to polish them up and make them seamless, as originally intended when they were nested types hidden from the consumer. They're exactly the solution to it all. When that stuff was happening, I hadn't yet had a chance to get terribly intimate with that language feature yet. I've come to love it because it really does deal with that exact scenario so well. Sure do wish I were familiar with it months ago, when we were trying to figure that all out. It is a bit tricky to initially get the hang of writing because it's VERY particular about the syntax, but it's all strongly-typed static analysis, so you pretty much can't write it wrong and manage to compile it. Score another point for Roslyn.

This comment was marked as resolved.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It already behaves as paint.net one. I.e. real-time sync.

The code here is just to avoid infinite sync looping

Copy link
Collaborator

@dodexahedron dodexahedron Jul 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A fix should be pretty quick I think but I'm on my way out the door so I'll look later. It's not a huge deal as a way to, at the end of the day, achieve the same result. But I would request that method not be public, please, so nobody uses it before it gets polished up. Or drop an obsolete attribute on it. Or whatev. 🤷‍♂️ Or just @mention me in a todo in the code if you don't wanna mess with it and I'll include it in one of my ADHD nitpick cleanup passes. 😅

A big one of which will be coming at some point, because turning a modicum of badic analysis on generates like 1500 warnings in TG alone right now. 😅

Copy link
Collaborator Author

@tznind tznind Jul 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I would request that method not be public

I already made it not internal in 21897b5

{
_value = v;
SetNeedsDisplay ();
}

/// <summary>
/// The last drawn location in View's viewport where the Triangle appeared.
/// Used exclusively for tests.
/// </summary>
internal int TrianglePosition { get; private set; }

/// <summary>
/// Event fired when <see cref="Value"/> is changed to a new value
/// </summary>
public event EventHandler<EventArgs<int>> ValueChanged;

/// <summary>
/// Creates a new instance of the <see cref="ColorBar"/> class.
/// </summary>
protected ColorBar ()
{
Height = 1;
Width = Dim.Fill ();
CanFocus = true;

AddCommand (Command.Left, _ => Adjust (-1));
AddCommand (Command.Right, _ => Adjust (1));

AddCommand (Command.LeftExtend, _ => Adjust (-MaxValue / 20));
AddCommand (Command.RightExtend, _ => Adjust (MaxValue / 20));

AddCommand (Command.LeftHome, _ => SetZero ());
AddCommand (Command.RightEnd, _ => SetMax ());

KeyBindings.Add (Key.CursorLeft, Command.Left);
KeyBindings.Add (Key.CursorRight, Command.Right);
KeyBindings.Add (Key.CursorLeft.WithShift, Command.LeftExtend);
KeyBindings.Add (Key.CursorRight.WithShift, Command.RightExtend);
KeyBindings.Add (Key.Home, Command.LeftHome);
KeyBindings.Add (Key.End, Command.RightEnd);
}

private bool? SetMax ()
{
Value = MaxValue;

return true;
}

private bool? SetZero ()
{
Value = 0;

return true;
}

private bool? Adjust (int delta)
{
var change = (int)(delta * _cellValue);

// Ensure that the change is at least 1 or -1 if delta is non-zero
if (change == 0 && delta != 0)
{
change = delta > 0 ? 1 : -1;
}

Value += change;

return true;
}

/// <summary>
/// Last known width of the bar as passed to <see cref="DrawBar"/>.
/// </summary>
private int _barWidth;


/// <inheritdoc/>
protected internal override bool OnMouseEvent (MouseEvent mouseEvent)
{
if (mouseEvent.Flags.HasFlag (MouseFlags.Button1Pressed))
{
if (mouseEvent.Position.X >= _barStartsAt)
{
double v = MaxValue * ((double)mouseEvent.Position.X - _barStartsAt) / (_barWidth - 1);
Value = Math.Clamp ((int)v, 0, MaxValue);
}

mouseEvent.Handled = true;
FocusFirst ();

return true;
}

return base.OnMouseEvent (mouseEvent);
}

/// <inheritdoc/>
public override void OnDrawContent (Rectangle viewport)
{
base.OnDrawContent (viewport);

var xOffset = 0;

if (!string.IsNullOrWhiteSpace (Text))
{
Move (0, 0);
Driver.SetAttribute (HasFocus ? GetFocusColor () : GetNormalColor ());
Driver.AddStr (Text);

// TODO: is there a better method than this? this is what it is in TableView
xOffset = Text.EnumerateRunes ().Sum (c => c.GetColumns ());
}

_barWidth = viewport.Width - xOffset;
_barStartsAt = xOffset;

DrawBar (xOffset, 0, _barWidth);
}

private void DrawBar (int xOffset, int yOffset, int width)
{
// Each 1 unit of X in the bar corresponds to this much of Value
_cellValue = (double)MaxValue / (width - 1);

for (var x = 0; x < width; x++)
{
double fraction = (double)x / (width - 1);
Color color = GetColor (fraction);

// Adjusted isSelectedCell calculation
bool isSelectedCell = Value > (x - 1) * _cellValue && Value <= x * _cellValue;

// Check the brightness of the background color
double brightness = (0.299 * color.R + 0.587 * color.G + 0.114 * color.B) / 255;

Color triangleColor = Color.Black;

if (brightness < 0.15) // Threshold to determine if the color is too close to black
{
triangleColor = Color.DarkGray;
}

if (isSelectedCell)
{
// Draw the triangle at the closest position
Application.Driver.SetAttribute (new (triangleColor, color));
AddRune (x + xOffset, yOffset, new ('▲'));

// Record for tests
TrianglePosition = x + xOffset;
}
else
{
Application.Driver.SetAttribute (new (color, color));
AddRune (x + xOffset, yOffset, new ('█'));
}
}
}

/// <summary>
/// When overriden in a derived class, returns the <see cref="Color"/> to
/// render at <paramref name="fraction"/> proportion of the full bars width.
/// e.g. 0.5 fraction of Saturation is 50% because Saturation goes from 0-100.
/// </summary>
/// <param name="fraction"></param>
/// <returns></returns>
protected abstract Color GetColor (double fraction);

private void OnValueChanged ()
{
ValueChanged?.Invoke (this, new (in _value));
SetNeedsDisplay ();
}
}
14 changes: 14 additions & 0 deletions Terminal.Gui/Views/ColorEventArgs.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
namespace Terminal.Gui;

/// <summary>Event arguments for the <see cref="Color"/> events.</summary>
public class ColorEventArgs : EventArgs
{
/// <summary>Initializes a new instance of <see cref="ColorEventArgs"/></summary>
public ColorEventArgs () { }

/// <summary>The new Color.</summary>
public Color Color { get; set; }

/// <summary>The previous Color.</summary>
public Color PreviousColor { get; set; }
}
23 changes: 23 additions & 0 deletions Terminal.Gui/Views/ColorModel.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
namespace Terminal.Gui;

/// <summary>
/// Describes away of modelling color e.g. Hue
/// Saturation Lightness.
/// </summary>
public enum ColorModel
{
/// <summary>
/// Color modelled by storing Red, Green and Blue as (0-255) ints
/// </summary>
RGB,

/// <summary>
/// Color modelled by storing Hue (360 degrees), Saturation (100%) and Value (100%)
/// </summary>
HSV,

/// <summary>
/// Color modelled by storing Hue (360 degrees), Saturation (100%) and Lightness (100%)
/// </summary>
HSL
}
Loading